diff --git a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java index 30c6d5ad70b..1cba5423e2d 100644 --- a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java +++ b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java @@ -42,7 +42,6 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.MultiStatus; import org.eclipse.core.runtime.Platform; -import org.eclipse.core.runtime.ProgressMonitorWrapper; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.ISchedulingRule; import org.eclipse.core.runtime.jobs.Job; @@ -58,10 +57,8 @@ import org.eclipse.jface.util.Policy; import org.eclipse.jface.window.Window; import org.eclipse.swt.SWT; -import org.eclipse.swt.events.SelectionAdapter; -import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.custom.BusyIndicator; import org.eclipse.swt.graphics.Resource; -import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Listener; import org.eclipse.swt.widgets.Shell; @@ -156,6 +153,13 @@ public class IDEWorkbenchAdvisor extends WorkbenchAdvisor { */ private final int workspaceWaitDelay; + /** + * Captured during {@link #initialize(IWorkbenchConfigurer)} so + * {@link #disconnectFromWorkspace()} does not have to reach through + * {@link PlatformUI#getWorkbench()} after the workbench has been shut down. + */ + private int savedLongOperationTime; + private final Listener closeListener = event -> { boolean doExit = IDEWorkbenchWindowAdvisor.promptOnExit(null); event.doit = doExit; @@ -194,6 +198,8 @@ public IDEWorkbenchAdvisor(DelayedEventsProcessor processor) { @Override public void initialize(IWorkbenchConfigurer configurer) { + savedLongOperationTime = configurer.getWorkbench().getProgressService().getLongOperationTime(); + PluginActionBuilder.setAllowIdeLogging(true); initResourceTracking(); @@ -464,120 +470,73 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException { } } - protected static class CancelableProgressMonitorWrapper extends - ProgressMonitorWrapper { - private double total = 0; - private final ProgressMonitorJobsDialog dialog; - - CancelableProgressMonitorWrapper(IProgressMonitor monitor, - ProgressMonitorJobsDialog dialog) { - super(monitor); - this.dialog = dialog; - } - - @Override - public void internalWorked(double work) { - super.internalWorked(work); - total += work; - updateProgressDetails(); - } - - @Override - public void worked(int work) { - super.worked(work); - total += work; - updateProgressDetails(); - } - - @Override - public void beginTask(String name, int totalWork) { - super.beginTask(name, totalWork); - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_preHistoryCompaction); - } - - private void updateProgressDetails() { - if (!isCanceled() && Math.abs(total - 4.0) < 0.0001 /* right before history compacting */) { - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_cancelHistoryPruning); - dialog.setCancelable(true); - } - if (Math.abs(total - 5.0) < 0.0001 /* history compacting finished */) { - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_postHistoryCompaction); - dialog.setCancelable(false); - } - } - } - - protected static class CancelableProgressMonitorJobsDialog extends - ProgressMonitorJobsDialog { - - public CancelableProgressMonitorJobsDialog(Shell parent) { - super(parent); - } - - @Override - protected void createButtonsForButtonBar(Composite parent) { - super.createButtonsForButtonBar(parent); - registerCancelButtonListener(); - } - - public void registerCancelButtonListener() { - cancel.addSelectionListener(new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent e) { - subTaskLabel.setText(""); //$NON-NLS-1$ - } - }); - } - } - /** * Disconnect from the core workspace. * + * Shows the progress dialog only if the save operation takes longer than + * the {@link IProgressService#getLongOperationTime() long operation time}; + * otherwise a busy cursor is shown while the save runs on a worker thread. + * This avoids a flashing dialog for fast shutdowns. + * * Locks workspace in a background thread, should not be called while * holding any workspace locks. */ protected void disconnectFromWorkspace() { - // save the workspace final MultiStatus status = new MultiStatus(IDEWorkbenchPlugin.IDE_WORKBENCH, 1, IDEWorkbenchMessages.ProblemSavingWorkbench); - try { - final ProgressMonitorJobsDialog p = new CancelableProgressMonitorJobsDialog( - null); - final boolean applyPolicy = ResourcesPlugin.getWorkspace() - .getDescription().isApplyFileStatePolicy(); + final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog(null); + dialog.setOpenOnRun(false); - IRunnableWithProgress runnable = monitor -> { - try { - if (applyPolicy) { - monitor = new CancelableProgressMonitorWrapper(monitor, p); - } + IRunnableWithProgress runnable = monitor -> { + try { + status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor)); + } catch (CoreException e) { + status.merge(e.getStatus()); + } + }; - status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor)); - } catch (CoreException e) { - status.merge(e.getStatus()); - } - }; + final Display display = Display.getCurrent(); + final int longOperationTime = savedLongOperationTime; + final Runnable openDialogLater = () -> { + if (!display.isDisposed() && !hasModalShell(display)) { + dialog.open(); + } + }; + display.timerExec(longOperationTime, openDialogLater); - p.run(true, false, runnable); - } catch (InvocationTargetException e) { - status - .merge(new Status(IStatus.ERROR, - IDEWorkbenchPlugin.IDE_WORKBENCH, 1, - IDEWorkbenchMessages.InternalError, e - .getTargetException())); - } catch (InterruptedException e) { - status.merge(new Status(IStatus.ERROR, - IDEWorkbenchPlugin.IDE_WORKBENCH, 1, - IDEWorkbenchMessages.InternalError, e)); + try { + BusyIndicator.showWhile(display, () -> { + try { + dialog.run(true, false, runnable); + } catch (InvocationTargetException e) { + status.merge(new Status(IStatus.ERROR, IDEWorkbenchPlugin.IDE_WORKBENCH, 1, + IDEWorkbenchMessages.InternalError, e.getTargetException())); + } catch (InterruptedException e) { + status.merge(new Status(IStatus.ERROR, IDEWorkbenchPlugin.IDE_WORKBENCH, 1, + IDEWorkbenchMessages.InternalError, e)); + } + }); + } finally { + // Cancel openDialogLater if it is still pending; no-op if it already ran. + display.timerExec(-1, openDialogLater); } + if (!status.isOK()) { - ErrorDialog.openError(null, - IDEWorkbenchMessages.ProblemsSavingWorkspace, null, status, + ErrorDialog.openError(null, IDEWorkbenchMessages.ProblemsSavingWorkspace, null, status, IStatus.ERROR | IStatus.WARNING); - IDEWorkbenchPlugin.log( - IDEWorkbenchMessages.ProblemsSavingWorkspace, status); + IDEWorkbenchPlugin.log(IDEWorkbenchMessages.ProblemsSavingWorkspace, status); + } + } + + private static boolean hasModalShell(Display display) { + final int modal = SWT.APPLICATION_MODAL | SWT.SYSTEM_MODAL | SWT.PRIMARY_MODAL; + for (Shell shell : display.getShells()) { + if (!shell.isDisposed() && shell.isVisible() && (shell.getStyle() & modal) != 0) { + return true; + } } + return false; } @Override diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java index bec664b18a5..09eb0bc9531 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java @@ -37,9 +37,6 @@ public class IDEWorkbenchMessages extends NLS { // package: org.eclipse.ui.ide public static String IDEWorkbenchAdvisor_noPerspective; - public static String IDEWorkbenchAdvisor_cancelHistoryPruning; - public static String IDEWorkbenchAdvisor_preHistoryCompaction; - public static String IDEWorkbenchAdvisor_postHistoryCompaction; public static String IDE_noFileEditorSelectedUserCanceled; public static String IDE_noFileEditorFound; diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties index 348c34e50b9..ae357abe00d 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties @@ -41,9 +41,6 @@ # package: org.eclipse.ui.ide IDEWorkbenchAdvisor_noPerspective=No perspectives are open. To open a perspective, press this button: -IDEWorkbenchAdvisor_cancelHistoryPruning=Cancel to skip compacting local history. -IDEWorkbenchAdvisor_preHistoryCompaction=Saving workbench state. -IDEWorkbenchAdvisor_postHistoryCompaction=Disconnecting from workspace. IDE_noFileEditorSelectedUserCanceled = No editor selected, operation canceled by user. IDE_noFileEditorFound = No editor found to edit the file resource. diff --git a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java index b62112f9c31..1990ee437e4 100644 --- a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java +++ b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java @@ -20,6 +20,7 @@ import java.io.Closeable; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import org.eclipse.core.resources.ISaveContext; import org.eclipse.core.resources.ISaveParticipant; @@ -41,6 +42,7 @@ public class IDEWorkbenchAdvisorTest { private static final String PLUGIN_ID = "org.eclipse.ui.ide.application.tests"; private Display display = null; private ISchedulingRule rule; + private final IWorkspace workspace = ResourcesPlugin.getWorkspace(); private static final class SaveHook implements ISaveParticipant, Closeable { public ISaveContext saving = null; @@ -200,6 +202,69 @@ public void postShutdown() { } } + /** + * Workbench shutdown should complete cleanly when the workspace save takes + * longer than the progress service's long operation time. In that case the + * progress dialog is expected to open after the delay, but the save must + * still run to completion and the save hooks must fire. + * + * Regression test for issue 1269 (flashing shutdown dialog). + */ + @Test + public void testShutdownWithSlowSave() throws CoreException { + try (SaveHook saveHook = new SaveHook()) { + // Sleep duration is derived from the live IProgressService#getLongOperationTime() + // value in postStartup(), so the test exercises the delayed-open path even if + // the platform default ever changes. + final long sleepMarginMillis = 500L; + final AtomicLong sleepMillis = new AtomicLong(); + workspace.addSaveParticipant(PLUGIN_ID + ".slow", new ISaveParticipant() { + @Override + public void saving(ISaveContext context) { + try { + Thread.sleep(sleepMillis.get()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + @Override + public void prepareToSave(ISaveContext context) { + } + + @Override + public void doneSaving(ISaveContext context) { + } + + @Override + public void rollback(ISaveContext context) { + } + }); + try { + IDEWorkbenchAdvisor advisor = new IDEWorkbenchAdvisor() { + @Override + public void postStartup() { + super.postStartup(); + long longOperationTime = PlatformUI.getWorkbench().getProgressService() + .getLongOperationTime(); + sleepMillis.set(longOperationTime + sleepMarginMillis); + display.asyncExec(() -> PlatformUI.getWorkbench().close()); + } + }; + int returnCode = PlatformUI.createAndRunWorkbench(display, advisor); + assertEquals(PlatformUI.RETURN_OK, returnCode); + dispatchDisplay(); + + assertNotNull(saveHook.prepareToSave); + assertNotNull(saveHook.saving); + assertNotNull(saveHook.doneSaving); + assertNull(saveHook.rollback); + } finally { + workspace.removeSaveParticipant(PLUGIN_ID + ".slow"); + } + } + } + /** * Workbench shutdown should disconnect workspace if it is not locked *