Changeset 10819 in josm for trunk

Timestamp:

2016-08-15T23:33:32+02:00 (8 years ago)

Author:

Don-vip

Message:

fix #13352 - new bug report queue (patch by michael2402) - gsoc-core

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/MapView.java (modified) (7 diffs)
• src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java (modified) (8 diffs)
• src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java (modified) (4 diffs)
• src/org/openstreetmap/josm/tools/bugreport/BugReportQueue.java (added)
• src/org/openstreetmap/josm/tools/bugreport/ReportedException.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/gui/MapView.java

@@ -77 +77 @@
 import org.openstreetmap.josm.tools.Utils;
 import org.openstreetmap.josm.tools.bugreport.BugReport;
-import org.openstreetmap.josm.tools.bugreport.BugReportExceptionHandler;
 
 /**
@@ -845 +844 @@
             g.setPaintMode();
         } catch (RuntimeException t) {
-            //TODO: only display.
-            throw BugReport.intercept(t).put("layer", layer).put("bounds", box);
+            BugReport.intercept(t).put("layer", layer).put("bounds", box).warn();
         }
     }
@@ -855 +853 @@
     @Override
     public void paint(Graphics g) {
-        if (!prepareToDraw()) {
+        try {
+            if (!prepareToDraw()) {
+                return;
+            }
+        } catch (RuntimeException e) {
+            BugReport.intercept(e).put("center", () -> getCenter()).warn();
             return;
         }
@@ -933 +936 @@
         }
 
-        synchronized (temporaryLayers) {
-            for (MapViewPaintable mvp : temporaryLayers) {
-                try {
-                    mvp.paint(tempG, this, box);
-                } catch (RuntimeException e) {
-                    throw BugReport.intercept(e).put("mvp", mvp);
-                }
-            }
+        try {
+            drawTemporaryLayers(tempG, box);
+        } catch (RuntimeException e) {
+            BugReport.intercept(e).put("temporaryLayers", temporaryLayers).warn();
         }
 
@@ -947 +946 @@
             drawWorldBorders(tempG);
         } catch (RuntimeException e) {
-            throw BugReport.intercept(e).put("bounds", getProjection()::getWorldBoundsLatLon);
+            // getProjection() needs to be inside lambda to catch errors.
+            BugReport.intercept(e).put("bounds", () -> getProjection().getWorldBoundsLatLon()).warn();
         }
 
@@ -989 +989 @@
     }
 
+    private void drawTemporaryLayers(Graphics2D tempG, Bounds box) {
+        synchronized (temporaryLayers) {
+            for (MapViewPaintable mvp : temporaryLayers) {
+                try {
+                    mvp.paint(tempG, this, box);
+                } catch (RuntimeException e) {
+                    throw BugReport.intercept(e).put("mvp", mvp);
+                }
+            }
+        }
+    }
+
     private void drawWorldBorders(Graphics2D tempG) {
         tempG.setColor(Color.WHITE);
@@ -1015 +1027 @@
             initialViewport = null;
         }
-        if (BugReportExceptionHandler.exceptionHandlingInProgress())
-            return false;
 
         if (getCenter() == null)

trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java

@@ -17 +17 @@
 import javax.swing.JDialog;
 import javax.swing.JLabel;
+import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 import javax.swing.UIManager;
@@ -22 +23 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.actions.ExpertToggleAction;
+import org.openstreetmap.josm.gui.preferences.plugin.PluginPreference;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.widgets.JMultilineLabel;
 import org.openstreetmap.josm.gui.widgets.UrlLabel;
+import org.openstreetmap.josm.plugins.PluginDownloadTask;
+import org.openstreetmap.josm.plugins.PluginHandler;
 import org.openstreetmap.josm.tools.GBC;
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.InputMapUtils;
+import org.openstreetmap.josm.tools.bugreport.BugReportQueue.SuppressionMode;
 
 /**
@@ -41 +47 @@
     private final BugReport report;
     private final DebugTextDisplay textPanel;
-    private JCheckBox cbSuppress;
+    private JCheckBox cbSuppressSingle;
+    private JCheckBox cbSuppressAll;
 
     /**
@@ -56 +63 @@
 
         addUpToDateSection();
-        // TODO: Notify user about plugin updates
+        // TODO: Notify user about plugin updates, then remove that notification that is displayed before this dialog is displayed.
 
         addCreateTicketSection();
@@ -146 +153 @@
     private void addIgnoreButton() {
         JPanel panel = new JPanel(new GridBagLayout());
-        cbSuppress = new JCheckBox(tr("Suppress further error dialogs for this session."));
-        cbSuppress.setVisible(false);
-        panel.add(cbSuppress, GBC.std().fill(GBC.HORIZONTAL));
+        cbSuppressSingle = new JCheckBox(tr("Suppress this error for this session."));
+        cbSuppressSingle.setVisible(false);
+        panel.add(cbSuppressSingle, GBC.std(0, 0).fill(GBC.HORIZONTAL));
+        cbSuppressAll = new JCheckBox(tr("Suppress further error dialogs for this session."));
+        cbSuppressAll.setVisible(false);
+        panel.add(cbSuppressAll, GBC.std(0, 1).fill(GBC.HORIZONTAL));
         JButton ignore = new JButton(tr("Ignore this error."));
         ignore.addActionListener(e -> closeDialog());
-        panel.add(ignore, GBC.eol());
+        panel.add(ignore, GBC.std(1, 0).span(1, 2).anchor(GBC.CENTER));
         content.add(panel, GBC.eol().fill(GBC.HORIZONTAL).insets(20));
     }
@@ -160 +170 @@
      */
     public void setShowSuppress(boolean showSuppress) {
-        cbSuppress.setVisible(showSuppress);
+        cbSuppressSingle.setVisible(showSuppress);
+        pack();
+    }
+
+    /**
+     * Shows or hides the suppress all errors button
+     * @param showSuppress <code>true</code> to show the suppress errors checkbox.
+     * @since 10819
+     */
+    public void setShowSuppressAll(boolean showSuppress) {
+        cbSuppressAll.setVisible(showSuppress);
         pack();
     }
@@ -168 +188 @@
      * @return <code>true</code> if the user wishes to suppress errors.
      */
-    public boolean shouldSuppressFurtherErrors() {
-        return cbSuppress.isSelected();
+    public SuppressionMode shouldSuppressFurtherErrors() {
+        if (cbSuppressAll.isSelected()) {
+            return SuppressionMode.ALL;
+        } else if (cbSuppressSingle.isSelected()) {
+            return SuppressionMode.SAME;
+        } else {
+            return SuppressionMode.NONE;
+        }
     }
 
@@ -199 +225 @@
         return null;
     }
+
+    /**
+     * Show the bug report for a given exception
+     * @param e The exception to display
+     * @param exceptionCounter A counter of how many exceptions have already been worked on
+     * @return The new suppression status
+     * @since 10819
+     */
+    public static SuppressionMode showFor(ReportedException e, int exceptionCounter) {
+        if (e.isOutOfMemory()) {
+            // do not translate the string, as translation may raise an exception
+            JOptionPane.showMessageDialog(Main.parent, "JOSM is out of memory. " +
+                    "Strange things may happen.\nPlease restart JOSM with the -Xmx###M option,\n" +
+                    "where ### is the number of MB assigned to JOSM (e.g. 256).\n" +
+                    "Currently, " + Runtime.getRuntime().maxMemory()/1024/1024 + " MB are available to JOSM.",
+                    "Error",
+                    JOptionPane.ERROR_MESSAGE
+                    );
+            return SuppressionMode.NONE;
+        } else {
+            return GuiHelper.runInEDTAndWaitAndReturn(() -> {
+                PluginDownloadTask downloadTask = PluginHandler.updateOrdisablePluginAfterException(e);
+                if (downloadTask != null) {
+                    // Ask for restart to install new plugin
+                    PluginPreference.notifyDownloadResults(
+                            Main.parent, downloadTask, !downloadTask.getDownloadedPlugins().isEmpty());
+                    return SuppressionMode.NONE;
+                }
+
+                BugReport report = new BugReport(e);
+                BugReportDialog dialog = new BugReportDialog(report);
+                dialog.setShowSuppress(exceptionCounter > 0);
+                dialog.setShowSuppressAll(exceptionCounter > 1);
+                dialog.setVisible(true);
+                return dialog.shouldSuppressFurtherErrors();
+            });
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.tools.bugreport;
-
-import java.awt.GraphicsEnvironment;
-
-import javax.swing.JOptionPane;
-import javax.swing.SwingUtilities;
-
-import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.gui.preferences.plugin.PluginPreference;
-import org.openstreetmap.josm.plugins.PluginDownloadTask;
-import org.openstreetmap.josm.plugins.PluginHandler;
 
 /**
@@ -19 +9 @@
  */
 public final class BugReportExceptionHandler implements Thread.UncaughtExceptionHandler {
-
-    private static boolean handlingInProgress;
-    private static volatile BugReporterThread bugReporterThread;
-    private static int exceptionCounter;
-    private static boolean suppressExceptionDialogs;
-
-    static final class BugReporterThread extends Thread {
-
-        private final class BugReporterWorker implements Runnable {
-            private final PluginDownloadTask pluginDownloadTask;
-
-            private BugReporterWorker(PluginDownloadTask pluginDownloadTask) {
-                this.pluginDownloadTask = pluginDownloadTask;
-            }
-
-            @Override
-            public void run() {
-                // Then ask for submitting a bug report, for exceptions thrown from a plugin too, unless updated to a new version
-                if (pluginDownloadTask == null) {
-                    askForBugReport(e);
-                } else {
-                    // Ask for restart to install new plugin
-                    PluginPreference.notifyDownloadResults(
-                            Main.parent, pluginDownloadTask, !pluginDownloadTask.getDownloadedPlugins().isEmpty());
-                }
-            }
-        }
-
-        private final Throwable e;
-
-        /**
-         * Constructs a new {@code BugReporterThread}.
-         * @param t the exception
-         */
-        private BugReporterThread(Throwable t) {
-            super("Bug Reporter");
-            this.e = t;
-        }
-
-        static void askForBugReport(final Throwable e) {
-            if (GraphicsEnvironment.isHeadless()) {
-                return;
-            }
-            BugReport report = new BugReport(BugReport.intercept(e));
-            BugReportDialog dialog = new BugReportDialog(report);
-            dialog.setShowSuppress(exceptionCounter > 1);
-            dialog.setVisible(true);
-            suppressExceptionDialogs = dialog.shouldSuppressFurtherErrors();
-        }
-
-        @Override
-        public void run() {
-            // Give the user a chance to deactivate the plugin which threw the exception (if it was thrown from a plugin)
-            SwingUtilities.invokeLater(new BugReporterWorker(PluginHandler.updateOrdisablePluginAfterException(e)));
-        }
-    }
 
     @Override
@@ -86 +20 @@
      */
     public static synchronized void handleException(final Throwable e) {
-        if (handlingInProgress || suppressExceptionDialogs)
-            return;                  // we do not handle secondary exceptions, this gets too messy
-        if (bugReporterThread != null && bugReporterThread.isAlive())
-            return;
-        handlingInProgress = true;
-        exceptionCounter++;
-        try {
-            Main.error(e);
-            if (Main.parent != null) {
-                if (e instanceof OutOfMemoryError) {
-                    // do not translate the string, as translation may raise an exception
-                    JOptionPane.showMessageDialog(Main.parent, "JOSM is out of memory. " +
-                            "Strange things may happen.\nPlease restart JOSM with the -Xmx###M option,\n" +
-                            "where ### is the number of MB assigned to JOSM (e.g. 256).\n" +
-                            "Currently, " + Runtime.getRuntime().maxMemory()/1024/1024 + " MB are available to JOSM.",
-                            "Error",
-                            JOptionPane.ERROR_MESSAGE
-                            );
-                    return;
-                }
-
-                bugReporterThread = new BugReporterThread(e);
-                bugReporterThread.start();
-            }
-        } finally {
-            handlingInProgress = false;
-        }
+        BugReport.intercept(e).warn();
     }
 
@@ -120 +28 @@
      */
     public static boolean exceptionHandlingInProgress() {
-        return handlingInProgress;
+        return BugReportQueue.getInstance().exceptionHandlingInProgress();
     }
 }

trunk/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java

@@ -66 +66 @@
     public void warn() {
         methodWarningFrom = BugReport.getCallingMethod(2);
-        // TODO: Open the dialog.
+        try {
+            BugReportQueue.getInstance().submit(this);
+        } catch (RuntimeException e) {
+            e.printStackTrace();
+        }
     }
 
@@ -243 +247 @@
     @Override
     public String toString() {
-        return new StringBuilder(48)
-            .append("CrashReportedException [on thread ")
-            .append(caughtOnThread)
-            .append(']')
-            .toString();
-    }
-
+        return "ReportedException [thread=" + caughtOnThread + ", exception=" + exception
+                + ", methodWarningFrom=" + methodWarningFrom + "]";
+    }
 
     /**
@@ -259 +259 @@
         return StreamUtils.toStream(CauseTraceIterator::new)
                 .anyMatch(t -> t instanceof ConcurrentModificationException || t instanceof InvocationTargetException);
+    }
+
+    /**
+     * Check if this is caused by an out of memory situaition
+     * @return <code>true</code> if it is.
+     * @since 10819
+     */
+    public boolean isOutOfMemory() {
+        return StreamUtils.toStream(CauseTraceIterator::new).anyMatch(t -> t instanceof OutOfMemoryError);
     }
 

trunk/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java

@@ -22 +22 @@
 
     /**
-     * Unit test for {@link BugReportExceptionHandler.BugReporterThread#askForBugReport} method.
-     */
-    @Test
-    public void testAskForBugReport() {
-        BugReportExceptionHandler.BugReporterThread.askForBugReport(new Exception("testAskForBugReport"));
-    }
-
-    /**
      * Unit test for {@link BugReportExceptionHandler#handleException} method.
      */