Avoid flashing shutdown dialog when save is fast

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

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;

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.

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
 	 *