Index: trunk/src/org/openstreetmap/josm/data/preferences/JosmBaseDirectories.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/preferences/JosmBaseDirectories.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/data/preferences/JosmBaseDirectories.java (revision 14052) @@ -155,3 +155,14 @@ return cacheDir; } + + /** + * Clears any previously calculated values used for {@link #getPreferencesDirectory(boolean)}, + * {@link #getCacheDirectory(boolean)} or {@link #getUserDataDirectory(boolean)}. Useful for tests. + * @since 14052 + */ + public void clearMemos() { + this.preferencesDir = null; + this.cacheDir = null; + this.userdataDir = null; + } } Index: trunk/src/org/openstreetmap/josm/gui/ConditionalOptionPaneUtil.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/ConditionalOptionPaneUtil.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/ConditionalOptionPaneUtil.java (revision 14052) @@ -265,5 +265,5 @@ * */ - static class MessagePanel extends JPanel { + public static class MessagePanel extends JPanel { private final JRadioButton cbShowPermanentDialog = new JRadioButton(NotShowAgain.PERMANENT.getLabel()); private final JRadioButton cbShowSessionDialog = new JRadioButton(NotShowAgain.SESSION.getLabel()); Index: trunk/src/org/openstreetmap/josm/gui/MainApplication.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 14052) @@ -488,12 +488,11 @@ @Override protected void shutdown() { - if (!GraphicsEnvironment.isHeadless()) { - try { - worker.shutdown(); - } catch (SecurityException e) { - Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e); - } - JCSCacheManager.shutdown(); - } + try { + worker.shutdown(); + } catch (SecurityException e) { + Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e); + } + JCSCacheManager.shutdown(); + if (mainFrame != null) { mainFrame.storeState(); @@ -505,10 +504,10 @@ layerManager.resetState(); super.shutdown(); - if (!GraphicsEnvironment.isHeadless()) { - try { - worker.shutdownNow(); - } catch (SecurityException e) { - Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e); - } + + try { + // in case the current task still hasn't finished + worker.shutdownNow(); + } catch (SecurityException e) { + Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e); } } Index: trunk/src/org/openstreetmap/josm/gui/MapViewState.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/MapViewState.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/MapViewState.java (revision 14052) @@ -3,5 +3,4 @@ import java.awt.Container; -import java.awt.GraphicsEnvironment; import java.awt.Point; import java.awt.geom.AffineTransform; @@ -146,14 +145,8 @@ private static Point findTopLeftOnScreen(JComponent position) { - if (GraphicsEnvironment.isHeadless()) { - // in our imaginary universe the window is always (10, 10) from the top left of the screen - Point topLeftInWindow = findTopLeftInWindow(position); - return new Point(topLeftInWindow.x + 10, topLeftInWindow.y + 10); - } else { - try { - return position.getLocationOnScreen(); - } catch (JosmRuntimeException | IllegalArgumentException | IllegalStateException e) { - throw BugReport.intercept(e).put("position", position).put("parent", position::getParent); - } + try { + return position.getLocationOnScreen(); + } catch (JosmRuntimeException | IllegalArgumentException | IllegalStateException e) { + throw BugReport.intercept(e).put("position", position).put("parent", position::getParent); } } Index: trunk/src/org/openstreetmap/josm/gui/NavigatableComponent.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/NavigatableComponent.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/NavigatableComponent.java (revision 14052) @@ -3,5 +3,4 @@ import java.awt.Cursor; -import java.awt.GraphicsEnvironment; import java.awt.Point; import java.awt.Rectangle; @@ -326,7 +325,5 @@ protected boolean isVisibleOnScreen() { - return GraphicsEnvironment.isHeadless() || ( - SwingUtilities.getWindowAncestor(this) != null && isShowing() - ); + return SwingUtilities.getWindowAncestor(this) != null && isShowing(); } Index: trunk/src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java (revision 14052) @@ -4,5 +4,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.awt.GraphicsEnvironment; import java.util.Optional; @@ -86,10 +85,8 @@ synchronized (AsynchronousUploadPrimitivesTask.class) { if (asynchronousUploadPrimitivesTask != null) { - if (!GraphicsEnvironment.isHeadless()) { - GuiHelper.runInEDTAndWait(() -> - JOptionPane.showMessageDialog(MainApplication.parent, - tr("A background upload is already in progress. " + - "Kindly wait for it to finish before uploading new changes"))); - } + GuiHelper.runInEDTAndWait(() -> + JOptionPane.showMessageDialog(MainApplication.parent, + tr("A background upload is already in progress. " + + "Kindly wait for it to finish before uploading new changes"))); return Optional.empty(); } else { Index: trunk/src/org/openstreetmap/josm/gui/preferences/PreferenceTabbedPane.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/PreferenceTabbedPane.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/preferences/PreferenceTabbedPane.java (revision 14052) @@ -26,5 +26,4 @@ import javax.swing.JScrollPane; import javax.swing.JTabbedPane; -import javax.swing.SwingUtilities; import javax.swing.event.ChangeEvent; import javax.swing.event.ChangeListener; @@ -60,4 +59,5 @@ import org.openstreetmap.josm.gui.preferences.validator.ValidatorTagCheckerRulesPreference; import org.openstreetmap.josm.gui.preferences.validator.ValidatorTestsPreference; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.plugins.PluginDownloadTask; import org.openstreetmap.josm.plugins.PluginHandler; @@ -175,5 +175,7 @@ } - Main.parent.repaint(); + if (Main.parent != null) { + Main.parent.repaint(); + } } } @@ -421,5 +423,5 @@ // by the remaining "save preferences" activites run on the Swing EDT. MainApplication.worker.submit(task); - MainApplication.worker.submit(() -> SwingUtilities.invokeLater(continuation)); + MainApplication.worker.submit(() -> GuiHelper.runInEDT(continuation)); } else { // no need for asynchronous activities. Simply run the remaining "save preference" Index: trunk/src/org/openstreetmap/josm/gui/preferences/advanced/ExportProfileAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/advanced/ExportProfileAction.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/preferences/advanced/ExportProfileAction.java (revision 14052) @@ -56,8 +56,6 @@ } if (keys.isEmpty()) { - if (!GraphicsEnvironment.isHeadless()) { - JOptionPane.showMessageDialog(Main.parent, - tr("All the preferences of this group are default, nothing to save"), tr("Warning"), JOptionPane.WARNING_MESSAGE); - } + JOptionPane.showMessageDialog(Main.parent, + tr("All the preferences of this group are default, nothing to save"), tr("Warning"), JOptionPane.WARNING_MESSAGE); return; } Index: trunk/src/org/openstreetmap/josm/gui/preferences/advanced/PreferencesTable.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/advanced/PreferencesTable.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/preferences/advanced/PreferencesTable.java (revision 14052) @@ -8,5 +8,4 @@ import java.awt.Component; import java.awt.Font; -import java.awt.GraphicsEnvironment; import java.awt.GridBagLayout; import java.awt.event.MouseAdapter; @@ -96,12 +95,10 @@ public boolean editPreference(final JComponent gui) { if (getSelectedRowCount() != 1) { - if (!GraphicsEnvironment.isHeadless()) { - JOptionPane.showMessageDialog( - gui, - tr("Please select the row to edit."), - tr("Warning"), - JOptionPane.WARNING_MESSAGE - ); - } + JOptionPane.showMessageDialog( + gui, + tr("Please select the row to edit."), + tr("Warning"), + JOptionPane.WARNING_MESSAGE + ); return false; } @@ -197,5 +194,5 @@ PrefEntry pe = null; boolean ok = false; - if (!GraphicsEnvironment.isHeadless() && askAddSetting(gui, p)) { + if (askAddSetting(gui, p)) { if (rbString.isSelected()) { StringSetting sSetting = new StringSetting(null); @@ -282,12 +279,10 @@ public void resetPreferences(final JComponent gui) { if (getSelectedRowCount() == 0) { - if (!GraphicsEnvironment.isHeadless()) { - JOptionPane.showMessageDialog( - gui, - tr("Please select the row to delete."), - tr("Warning"), - JOptionPane.WARNING_MESSAGE - ); - } + JOptionPane.showMessageDialog( + gui, + tr("Please select the row to delete."), + tr("Warning"), + JOptionPane.WARNING_MESSAGE + ); return; } Index: trunk/src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java (revision 14052) @@ -7,5 +7,4 @@ import java.awt.BorderLayout; import java.awt.Component; -import java.awt.GraphicsEnvironment; import java.awt.GridBagConstraints; import java.awt.GridBagLayout; @@ -159,13 +158,11 @@ } sb.append(""); - if (!GraphicsEnvironment.isHeadless()) { - GuiHelper.runInEDTAndWait(() -> HelpAwareOptionPane.showOptionDialog( - parent, - sb.toString(), - tr("Update plugins"), - !failed.isEmpty() ? JOptionPane.WARNING_MESSAGE : JOptionPane.INFORMATION_MESSAGE, - HelpUtil.ht("/Preferences/Plugins") - )); - } + GuiHelper.runInEDTAndWait(() -> HelpAwareOptionPane.showOptionDialog( + parent, + sb.toString(), + tr("Update plugins"), + !failed.isEmpty() ? JOptionPane.WARNING_MESSAGE : JOptionPane.INFORMATION_MESSAGE, + HelpUtil.ht("/Preferences/Plugins") + )); } @@ -212,11 +209,17 @@ } + private static Component addButton(JPanel pnl, JButton button, String buttonName) { + button.setName(buttonName); + return pnl.add(button); + } + private JPanel buildActionPanel() { JPanel pnl = new JPanel(new GridLayout(1, 4)); - pnl.add(new JButton(new DownloadAvailablePluginsAction())); - pnl.add(new JButton(new UpdateSelectedPluginsAction())); - ExpertToggleAction.addVisibilitySwitcher(pnl.add(new JButton(new SelectByListAction()))); - ExpertToggleAction.addVisibilitySwitcher(pnl.add(new JButton(new ConfigureSitesAction()))); + // assign some component names to these as we go to aid testing + addButton(pnl, new JButton(new DownloadAvailablePluginsAction()), "downloadListButton"); + addButton(pnl, new JButton(new UpdateSelectedPluginsAction()), "updatePluginsButton"); + ExpertToggleAction.addVisibilitySwitcher(addButton(pnl, new JButton(new SelectByListAction()), "loadFromListButton")); + ExpertToggleAction.addVisibilitySwitcher(addButton(pnl, new JButton(new ConfigureSitesAction()), "configureSitesButton")); return pnl; } Index: trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java (revision 14052) @@ -2,8 +2,12 @@ package org.openstreetmap.josm.gui.progress.swing; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -37,3 +41,22 @@ } + @Override + public void afterExecute(final Runnable r, Throwable t) { + // largely as proposed by JDK8 docs + super.afterExecute(r, t); + if (t == null && r instanceof Future) { + try { + ((Future) r).get(); + } catch (CancellationException cancellationException) { + t = cancellationException; + } catch (ExecutionException executionException) { + t = executionException.getCause(); + } catch (InterruptedException interruptedException) { + Thread.currentThread().interrupt(); // ignore/reset + } + } + if (t != null) { + Logging.error("Thread {0} raised {1}", Thread.currentThread().getName(), t); + } + } } Index: trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java (revision 14052) @@ -202,4 +202,8 @@ } + private static void handleEDTException(Throwable t) { + Logging.logWithStackTrace(Logging.LEVEL_ERROR, t, "Exception raised in EDT"); + } + /** * Executes synchronously a runnable in @@ -215,5 +219,5 @@ SwingUtilities.invokeAndWait(task); } catch (InterruptedException | InvocationTargetException e) { - Logging.error(e); + handleEDTException(e); } } @@ -256,5 +260,5 @@ return callable.call(); } catch (Exception e) { // NOPMD - Logging.error(e); + handleEDTException(e); return null; } @@ -265,5 +269,5 @@ return task.get(); } catch (InterruptedException | ExecutionException e) { - Logging.error(e); + handleEDTException(e); return null; } Index: trunk/src/org/openstreetmap/josm/tools/Logging.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/Logging.java (revision 14051) +++ trunk/src/org/openstreetmap/josm/tools/Logging.java (revision 14052) @@ -51,4 +51,65 @@ private static final RememberWarningHandler WARNINGS = new RememberWarningHandler(); + /** + * A {@link ConsoleHandler} with a couple of extra features, allowing it to be targeted at an + * an arbitrary {@link OutputStream} which it can be asked to reaquire the reference for on demand + * through {@link #reacquireOutputStream()}. It can also prevent a LogRecord's output if a + * specified {@code prioritizedHandler} would have outputted it. + * @since 14052 + */ + public static class ReacquiringConsoleHandler extends ConsoleHandler { + private final Supplier outputStreamSupplier; + private final Handler prioritizedHandler; + private OutputStream outputStreamMemo; + + /** + * Construct a new {@link ReacquiringConsoleHandler}. + * @param outputStreamSupplier A {@link Supplier} which will return the desired + * {@link OutputStream} for this handler when called. Particularly useful if you happen to be + * using a test framework which will switch out references to the stderr/stdout streams with + * new dummy ones from time to time. + * @param prioritizedHandler If non-null, will suppress output of any log records which pass this + * handler's {@code Handler#isLoggable(LogRecord)} method. + */ + public ReacquiringConsoleHandler( + final Supplier outputStreamSupplier, + final Handler prioritizedHandler + ) { + this.outputStreamSupplier = outputStreamSupplier; + this.prioritizedHandler = prioritizedHandler; + + this.reacquireOutputStream(); + } + + /** + * Set output stream to one acquired from calling outputStreamSupplier + */ + public void reacquireOutputStream() { + final OutputStream reacquiredStream = this.outputStreamSupplier.get(); + + // only bother calling setOutputStream if it's actually different, as setOutputStream + // has the nasty side effect of closing any previous output stream, which is certainly not + // what we would want were the new stream the same one + if (reacquiredStream != this.outputStreamMemo) { + this.setOutputStream(reacquiredStream); + } + } + + @Override + public synchronized void setOutputStream(final OutputStream outputStream) { + // this wouldn't be necessary if StreamHandler made it possible to see what the current + // output stream is set to + this.outputStreamMemo = outputStream; + super.setOutputStream(outputStream); + } + + @Override + public synchronized void publish(LogRecord record) { + if (this.prioritizedHandler == null || !this.prioritizedHandler.isLoggable(record)) { + super.publish(record); + } + } + } + static { // We need to be sure java.locale.providers system property is initialized by JOSM, not by JRE @@ -62,5 +123,5 @@ Utils.updateSystemProperty("java.util.logging.SimpleFormatter.format", "%1$tF %1$tT.%1$tL %4$s: %5$s%6$s%n"); - ConsoleHandler stderr = new ConsoleHandler(); + ConsoleHandler stderr = new ReacquiringConsoleHandler(() -> System.err, null); LOGGER.addHandler(stderr); try { @@ -70,18 +131,5 @@ } - ConsoleHandler stdout = new ConsoleHandler() { - @Override - protected synchronized void setOutputStream(OutputStream out) { - // overwrite output stream. - super.setOutputStream(System.out); - } - - @Override - public synchronized void publish(LogRecord record) { - if (!stderr.isLoggable(record)) { - super.publish(record); - } - } - }; + ConsoleHandler stdout = new ReacquiringConsoleHandler(() -> System.out, stderr); LOGGER.addHandler(stdout); try {