From ca52e6d4c032024e4953d19af6996cfdc68e0474 Mon Sep 17 00:00:00 2001 From: Federico Jeanne Date: Thu, 23 Jul 2026 12:51:48 +0200 Subject: [PATCH] Honor 'Always run in background' in IProgressService.run() #4202 Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/4202 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ui/internal/progress/ProgressManager.java | 29 +- .../org.eclipse.ui.examples.job/plugin.xml | 7 + .../jobs/views/ProgressServiceView.java | 289 ++++++++++++ .../tests/progress/ProgressServiceTest.java | 438 ++++++++++++++++++ 4 files changed, 750 insertions(+), 13 deletions(-) create mode 100644 examples/org.eclipse.ui.examples.job/src/org/eclipse/ui/examples/jobs/views/ProgressServiceView.java create mode 100644 tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressServiceTest.java diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/ProgressManager.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/ProgressManager.java index 6aba4aedc63..1b1cfb528a2 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/ProgressManager.java +++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/ProgressManager.java @@ -1075,22 +1075,25 @@ public void showInDialog(Shell shell, Job job) { @Override public void run(boolean fork, boolean cancelable, IRunnableWithProgress runnable) throws InvocationTargetException, InterruptedException { + final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog(ProgressManagerUtil.getDefaultParent()); + dialog.setOpenOnRun(false); + if (shouldRunInBackground()) { + // The "Always run in background" preference must suppress the modal + // progress dialog for every fork/cancelable combination, not just the + // backward compatible (fork==false || cancelable==false) path below. + dialog.run(fork, cancelable, runnable); + return; + } + if (!fork || !cancelable) { // Backward compatible code. - final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog( - ProgressManagerUtil.getDefaultParent()); - if (shouldRunInBackground()) { - dialog.setOpenOnRun(false); + Job showDialogJob = scheduleProgressMonitorJob(dialog); + try { dialog.run(fork, cancelable, runnable); - } else { - Job showDialogJob = scheduleProgressMonitorJob(dialog); - try { - dialog.run(fork, cancelable, runnable); - } finally { - // In case the dialog hasn't popped up yet, cancel it so it doesn't pop up after - // the operation finishes or unwinds with an exception. - showDialogJob.cancel(); - } + } finally { + // In case the dialog hasn't popped up yet, cancel it so it doesn't pop up after + // the operation finishes or unwinds with an exception. + showDialogJob.cancel(); } return; } diff --git a/examples/org.eclipse.ui.examples.job/plugin.xml b/examples/org.eclipse.ui.examples.job/plugin.xml index 229c50aaafc..17f8a142720 100644 --- a/examples/org.eclipse.ui.examples.job/plugin.xml +++ b/examples/org.eclipse.ui.examples.job/plugin.xml @@ -22,6 +22,13 @@ class="org.eclipse.ui.examples.jobs.views.JobsView" id="org.eclipse.ui.examples.jobs.views.JobsView"> + + diff --git a/examples/org.eclipse.ui.examples.job/src/org/eclipse/ui/examples/jobs/views/ProgressServiceView.java b/examples/org.eclipse.ui.examples.job/src/org/eclipse/ui/examples/jobs/views/ProgressServiceView.java new file mode 100644 index 00000000000..9b3b52d4dc5 --- /dev/null +++ b/examples/org.eclipse.ui.examples.job/src/org/eclipse/ui/examples/jobs/views/ProgressServiceView.java @@ -0,0 +1,289 @@ +/******************************************************************************* + * Copyright (c) 2026 Eclipse contributors and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.ui.examples.jobs.views; + +import java.lang.reflect.InvocationTargetException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import org.eclipse.core.runtime.Platform; +import org.eclipse.jface.operation.IRunnableWithProgress; +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.jface.util.IPropertyChangeListener; +import org.eclipse.swt.SWT; +import org.eclipse.swt.events.SelectionListener; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Text; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.internal.IPreferenceConstants; +import org.eclipse.ui.part.ViewPart; +import org.eclipse.ui.progress.IProgressService; + +/** + * A view that demonstrates (and lets you reproduce) the documented and + * undocumented behaviors of + * {@link IProgressService#run(boolean, boolean, IRunnableWithProgress)} + * across every combination of the fork/cancelable + * parameters, the "Always run in background" preference, and calling from a + * non-UI thread. + */ +@SuppressWarnings("restriction") +public class ProgressServiceView extends ViewPart { + + private static final int SLEEP_STEP_MS = 100; + + private final ExecutorService nonUiExecutor = Executors.newSingleThreadExecutor(r -> { + Thread t = new Thread(r, "ProgressServiceView-nonUI"); //$NON-NLS-1$ + t.setDaemon(true); + return t; + }); + + private Button backgroundPrefField; + private IPropertyChangeListener prefListener; + + private Button forkField; + private Button cancelableField; + private Text durationField; + + @Override + public void createPartControl(Composite parent) { + Composite body = new Composite(parent, SWT.NONE); + GridLayout layout = new GridLayout(); + layout.numColumns = 1; + body.setLayout(layout); + body.setLayoutData(new GridData(GridData.FILL_BOTH)); + + createPreferenceGroup(body); + createRunGroup(body); + createForkIllustrationGroup(body); + createNonUiGroup(body); + } + + private void createPreferenceGroup(Composite parent) { + Composite group = new Composite(parent, SWT.NONE); + GridLayout layout = new GridLayout(); + layout.numColumns = 1; + group.setLayout(layout); + group.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + backgroundPrefField = new Button(group, SWT.CHECK); + backgroundPrefField.setText("Always run in background"); //$NON-NLS-1$ + backgroundPrefField.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + backgroundPrefField.setSelection(getPreferenceStore().getBoolean(IPreferenceConstants.RUN_IN_BACKGROUND)); + backgroundPrefField.addSelectionListener(SelectionListener.widgetSelectedAdapter( + e -> getPreferenceStore().setValue(IPreferenceConstants.RUN_IN_BACKGROUND, + backgroundPrefField.getSelection()))); + + Label hint = new Label(group, SWT.WRAP); + hint.setText("This preference can also be set under Preferences > General."); //$NON-NLS-1$ + hint.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + prefListener = event -> { + if (IPreferenceConstants.RUN_IN_BACKGROUND.equals(event.getProperty())) { + if (backgroundPrefField.isDisposed()) { + return; + } + Display display = backgroundPrefField.getDisplay(); + if (display.isDisposed()) { + return; + } + display.asyncExec(() -> { + if (!backgroundPrefField.isDisposed()) { + backgroundPrefField + .setSelection(getPreferenceStore().getBoolean(IPreferenceConstants.RUN_IN_BACKGROUND)); + } + }); + } + }; + getPreferenceStore().addPropertyChangeListener(prefListener); + } + + private void createRunGroup(Composite parent) { + Composite group = new Composite(parent, SWT.NONE); + GridLayout layout = new GridLayout(); + layout.numColumns = 2; + group.setLayout(layout); + group.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + Label label = new Label(group, SWT.NONE); + label.setText("Duration (ms):"); //$NON-NLS-1$ + durationField = new Text(group, SWT.BORDER); + durationField.setText("3000"); //$NON-NLS-1$ + durationField.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + forkField = new Button(group, SWT.CHECK); + forkField.setText("fork"); //$NON-NLS-1$ + forkField.setSelection(true); + GridData forkData = new GridData(GridData.FILL_HORIZONTAL); + forkData.horizontalSpan = 2; + forkField.setLayoutData(forkData); + + cancelableField = new Button(group, SWT.CHECK); + cancelableField.setText("cancelable"); //$NON-NLS-1$ + cancelableField.setSelection(true); + GridData cancelableData = new GridData(GridData.FILL_HORIZONTAL); + cancelableData.horizontalSpan = 2; + cancelableField.setLayoutData(cancelableData); + + Button run = new Button(group, SWT.PUSH); + run.setText("Run via IProgressService.run(fork, cancelable, ...)"); //$NON-NLS-1$ + GridData runData = new GridData(GridData.FILL_HORIZONTAL); + runData.horizontalSpan = 2; + run.setLayoutData(runData); + run.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> runViaProgressService())); + } + + private void createForkIllustrationGroup(Composite parent) { + Composite group = new Composite(parent, SWT.NONE); + GridLayout layout = new GridLayout(); + layout.numColumns = 1; + group.setLayout(layout); + group.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + Button naive = new Button(group, SWT.PUSH); + naive.setText("fork=false (naive - will freeze)"); //$NON-NLS-1$ + naive.setToolTipText("Calls run(false, true, ...) with a plain sleep loop. Watch the heartbeat stop."); //$NON-NLS-1$ + naive.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + naive.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> runForkFalseNaive())); + + Button pumping = new Button(group, SWT.PUSH); + pumping.setText("fork=false (pumping events - correct)"); //$NON-NLS-1$ + pumping.setToolTipText( + "Calls run(false, true, ...) with a loop that calls Display.readAndDispatch(). The heartbeat keeps ticking."); //$NON-NLS-1$ + pumping.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + pumping.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> runForkFalsePumping())); + } + + private void createNonUiGroup(Composite parent) { + Composite group = new Composite(parent, SWT.NONE); + GridLayout layout = new GridLayout(); + layout.numColumns = 1; + group.setLayout(layout); + group.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + + Button nonUiButton = new Button(group, SWT.PUSH); + nonUiButton.setText("Run from non-UI thread (expect exception)"); //$NON-NLS-1$ + nonUiButton.setToolTipText( + "IProgressService.run() must be called from the UI thread; this documents the resulting exception."); //$NON-NLS-1$ + nonUiButton.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); + nonUiButton.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> runFromNonUIThread())); + } + + @SuppressWarnings("deprecation") + private IPreferenceStore getPreferenceStore() { + return PlatformUI.getWorkbench().getPreferenceStore(); + } + + private long getDuration() { + try { + return Long.parseLong(durationField.getText().trim()); + } catch (NumberFormatException e) { + Platform.getLog(ProgressServiceView.class).error(e.getMessage(), e); + return 3000; + } + } + + private IRunnableWithProgress createSleepRunnable(long durationMillis, boolean pumpEvents) { + return monitor -> { + int ticks = (int) Math.max(1, durationMillis / SLEEP_STEP_MS); + monitor.beginTask("Simulated long-running operation", ticks); //$NON-NLS-1$ + try { + for (int i = 0; i < ticks; i++) { + if (monitor.isCanceled()) { + return; + } + try { + Thread.sleep(SLEEP_STEP_MS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + if (pumpEvents) { + Display display = Display.getCurrent(); + if (display != null) { + while (display.readAndDispatch()) { + // drain pending UI events so the heartbeat keeps ticking + } + } + } + monitor.worked(1); + } + } finally { + monitor.done(); + } + }; + } + + private void runViaProgressService() { + boolean fork = forkField.getSelection(); + boolean cancelable = cancelableField.getSelection(); + long duration = getDuration(); + IProgressService service = PlatformUI.getWorkbench().getProgressService(); + try { + service.run(fork, cancelable, createSleepRunnable(duration, false)); + } catch (InvocationTargetException | InterruptedException e) { + Platform.getLog(ProgressServiceView.class).error(e.getMessage(), e); + } + } + + private void runForkFalseNaive() { + long duration = getDuration(); + boolean cancelable = cancelableField.getSelection(); + try { + PlatformUI.getWorkbench().getProgressService().run(false, cancelable, createSleepRunnable(duration, false)); + } catch (InvocationTargetException | InterruptedException e) { + Platform.getLog(ProgressServiceView.class).error(e.getMessage(), e); + } + } + + private void runForkFalsePumping() { + long duration = getDuration(); + boolean cancelable = cancelableField.getSelection(); + try { + PlatformUI.getWorkbench().getProgressService().run(false, cancelable, createSleepRunnable(duration, true)); + } catch (InvocationTargetException | InterruptedException e) { + Platform.getLog(ProgressServiceView.class).error(e.getMessage(), e); + } + } + + private void runFromNonUIThread() { + boolean fork = forkField.getSelection(); + boolean cancelable = cancelableField.getSelection(); + long duration = getDuration(); + nonUiExecutor.execute(() -> { + try { + PlatformUI.getWorkbench().getProgressService().run(fork, cancelable, + createSleepRunnable(duration, false)); + } catch (Throwable t) { + Platform.getLog(ProgressServiceView.class).error(t.getMessage(), t); + } + }); + } + + @Override + public void dispose() { + if (prefListener != null) { + getPreferenceStore().removePropertyChangeListener(prefListener); + } + nonUiExecutor.shutdownNow(); + super.dispose(); + } + + @Override + public void setFocus() { + // do nothing + } + +} diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressServiceTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressServiceTest.java new file mode 100644 index 00000000000..b31d4ba19b5 --- /dev/null +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressServiceTest.java @@ -0,0 +1,438 @@ +package org.eclipse.ui.tests.progress; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.eclipse.core.runtime.SubMonitor; +import org.eclipse.jface.operation.IRunnableWithProgress; +import org.eclipse.swt.SWT; +import org.eclipse.swt.SWTException; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Listener; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.internal.IPreferenceConstants; +import org.eclipse.ui.internal.WorkbenchPlugin; +import org.eclipse.ui.internal.progress.FinishedJobs; +import org.eclipse.ui.progress.IProgressService; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * @since 3.5 + * + */ +public class ProgressServiceTest extends ProgressTestCase { + + private IProgressService progressService; + + @Override + @Before + public void doSetUp() throws Exception { + window = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); + progressService = PlatformUI.getWorkbench().getProgressService(); + FinishedJobs.getInstance().clearAll(); + } + + @Override + @After + public void doTearDown() throws Exception { + FinishedJobs.getInstance().clearAll(); + WorkbenchPlugin.getDefault().getPreferenceStore().setToDefault(IPreferenceConstants.RUN_IN_BACKGROUND); + super.doTearDown(); + } + + /** + * See + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown()} + * for why a progress dialog is expected to never appear when {@code fork == + * false} and the runnable simply blocks the UI thread without pumping + * events, and for the caveat about why this alone does not fully prove the + * 800ms delay is honored. + */ + @Test + public void testRun_noFork_notCancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown() + throws Exception { + assertDialogShown(false, false, false, true, false, false); + } + + /** + * See + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown()} + * for why pumping UI events while "working" is required for {@code fork == + * false} to be able to show a dialog. + */ + @Test + public void testRun_noFork_notCancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown() + throws Exception { + assertDialogShown(false, false, false, true, true, true); + } + + /** + * See + * {@link #testRun_noFork_cancelable_runInBackground_callFromUIThread_blockingUiThread_noDialogShown()} + * for why this assertion passing does not, by itself, prove that the + * "Always run in background" preference is honored. + */ + @Test + public void testRun_noFork_notCancelable_runInBackground_callFromUIThread_blockingUiThread_noDialogShown() + throws Exception { + assertDialogShown(false, false, true, true, false, false); + } + + /** + * See + * {@link #testRun_noFork_cancelable_runInBackground_callFromUIThread_pumpingUiEventsWhileRunning_noDialogShown()}. + */ + @Test + public void testRun_noFork_notCancelable_runInBackground_callFromUIThread_pumpingUiEventsWhileRunning_noDialogShown() + throws Exception { + assertDialogShown(false, false, true, true, true, false); + } + + /** + * This is the direct counterpart of the naive assumption that + * "{@code fork == false} called from a non-UI thread is fine, since the + * calling (non-UI) thread just blocks while the UI thread stays + * responsive". That assumption is wrong: {@link IProgressService#run} + * cannot be called off the UI thread at all, independently of + * {@code fork} - see + * {@link #assertDialogShown(boolean, boolean, boolean, boolean, boolean, boolean)} + * for why, and + * {@link #testRun_fork_cancelable_runInForeground_callFromNonUIThread_notPumpingUiEvents_throwsInvalidThreadAccess()} + * for the {@code fork == true} counterpart of this test, showing the + * restriction is unconditional on {@code fork}. + *

+ * We deliberately do not repeat this for every {@code cancelable} / + * {@code runInBackground} combination: the failure happens before those + * parameters are ever read, so varying them cannot change the outcome. + */ + @Test + public void testRun_noFork_cancelable_runInForeground_callFromNonUIThread_notPumpingUiEvents_throwsInvalidThreadAccess() + throws Exception { + assertDialogShown(false, true, false, false, false, false); + } + + /** + * With {@code fork == false}, {@link IProgressService#run} executes the + * runnable synchronously on the calling thread, which here (like in + * the vast majority of real callers, e.g. drag-and-drop handlers) is the UI + * thread - {@code fork == false} can only ever be meaningfully invoked from + * the UI thread in the first place, since calling it from any other thread + * fails immediately (see + * {@link #testRun_noFork_cancelable_runInForeground_callFromNonUIThread_notPumpingUiEvents_throwsInvalidThreadAccess()}). + *

+ * The progress dialog is only opened by a {@code WorkbenchJob} that is + * scheduled to run on the UI thread after a delay + * ({@link IProgressService#getLongOperationTime()}) via + * {@code Display.asyncExec}; actually running that queued job requires the + * UI thread's event loop to be pumped (i.e. {@code Display.readAndDispatch()} + * to be called again). If the runnable just blocks (e.g. with + * {@code Thread.sleep}) without ever giving the UI thread a chance to pump + * events, that queued job never runs and the dialog is expected to + * never appear, no matter how long the operation takes or whether it + * is cancelable. + *

+ * Caveat: this assertion passing does not, by itself, prove + * that the 800ms long-operation delay is honored while the UI thread is + * blocked - it can also pass for the wrong reason. Before this PR, + * {@code ProgressManager} had a bug where, for this exact + * {@code fork == false} / not-running-in-background combination, it left + * the dialog's {@code openOnRun} flag at its default of {@code true} and + * therefore opened the dialog synchronously, immediately, as part of + * {@code run()} itself - completely ignoring the 800ms delay. Had that bug + * still been present, a dialog would have appeared here too (just far too + * early), and this test would have failed - which is how the fix is + * indirectly exercised. But if some other, hypothetical bug reintroduced an + * immediate/synchronous open while also leaving the UI thread + * blocked such that the {@code Show} event never gets a chance to be + * observed by this test's listener before the runnable finishes, this + * assertion could pass without truly proving the delay is honored. The + * unambiguous proof that the delay is honored - i.e. that the dialog is + * shown only after the delay elapses and only if the UI thread is kept + * responsive - is + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown()}, + * where events are pumped and the dialog is expected (and observed) to + * appear. + */ + @Test + public void testRun_noFork_cancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown() + throws Exception { + assertDialogShown(false, true, false, true, false, false); + } + + /** + * Unlike the {@code blockingUiThread} variants above, here the runnable + * keeps calling {@code Display.readAndDispatch()} while it "works" instead + * of just sleeping - i.e. it behaves like a real, responsive long-running + * operation that cooperates with the UI thread instead of freezing it. This + * gives the {@code WorkbenchJob} that opens the progress dialog after the + * "long operation" delay a chance to actually run, so the dialog is shown. + * This is the only way {@code fork == false} can show a dialog, since (see + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown()}) + * {@code fork == false} can only ever be invoked from the UI thread in the + * first place. + */ + @Test + public void testRun_noFork_cancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown() + throws Exception { + assertDialogShown(false, true, false, true, true, true); + } + + /** + * Caveat: with the runnable blocking the UI thread (i.e. not pumping + * events), the dialog never gets a chance to appear "the normal way" + * (i.e. via the delayed {@code WorkbenchJob}, see + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown()}) + * - regardless of the "Always run in background" preference. So this + * assertion passing here does not, by itself, prove that the + * preference is honored: it would equally pass if the preference were + * completely ignored, or even if {@code ProgressManager} had the bug this + * PR fixes, where the dialog was opened synchronously (ignoring the + * 800ms long-operation delay entirely) as soon as {@code run()} was called + * - because that buggy synchronous-open code path was itself skipped + * whenever the "Always run in background" preference was set, both before + * and after this PR. The real proof that the preference suppresses the + * dialog is + * {@link #testRun_noFork_cancelable_runInBackground_callFromUIThread_pumpingUiEventsWhileRunning_noDialogShown()}, + * where the runnable does pump events (giving the dialog a genuine + * opportunity to appear) and it still does not show up. This test is kept + * only to document/pin the (also correct, if weaker) blocked-UI-thread + * behavior. + */ + @Test + public void testRun_noFork_cancelable_runInBackground_callFromUIThread_blockingUiThread_noDialogShown() + throws Exception { + assertDialogShown(false, true, true, true, false, false); + } + + /** + * Even when the runnable pumps UI events while running (see + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown()}), + * the "Always run in background" preference must still suppress the modal + * progress dialog. + */ + @Test + public void testRun_noFork_cancelable_runInBackground_callFromUIThread_pumpingUiEventsWhileRunning_noDialogShown() + throws Exception { + assertDialogShown(false, true, true, true, true, false); + } + + @Test + public void testRun_fork_notCancelable_runInForeground_callFromUIThread_notPumpingUiEvents_dialogShown() + throws Exception { + assertDialogShown(true, false, false, true, false, true); + } + + @Test + public void testRun_fork_notCancelable_runInBackground_callFromUIThread_notPumpingUiEvents_noDialogShown() + throws Exception { + assertDialogShown(true, false, true, true, false, false); + } + + /** + * One might assume {@code fork == true} is always safe to call from any + * thread, since the actual runnable executes on a separate (forked) + * thread anyway. This is not the case: {@code ProgressManager.run} + * unconditionally computes the dialog's default parent shell via + * {@code Display.getShells()} as its very first statement - before it + * even looks at {@code fork} - so the call itself always requires the UI + * thread, regardless of {@code fork}. See + * {@link #assertDialogShown(boolean, boolean, boolean, boolean, boolean, boolean)} + * for details, and + * {@link #testRun_noFork_cancelable_runInForeground_callFromNonUIThread_notPumpingUiEvents_throwsInvalidThreadAccess()} + * for the {@code fork == false} counterpart of this test. + *

+ * We deliberately do not repeat this for every {@code cancelable} / + * {@code runInBackground} combination: the failure happens before those + * parameters are ever read, so varying them cannot change the outcome. + */ + @Test + public void testRun_fork_cancelable_runInForeground_callFromNonUIThread_notPumpingUiEvents_throwsInvalidThreadAccess() + throws Exception { + assertDialogShown(true, true, false, false, false, false); + } + + @Test + public void testRun_fork_cancelable_runInForeground_callFromUIThread_notPumpingUiEvents_dialogShown() + throws Exception { + assertDialogShown(true, true, false, true, false, true); + } + + @Test + public void testRun_fork_cancelable_runInBackground_callFromUIThread_notPumpingUiEvents_noDialogShown() + throws Exception { + assertDialogShown(true, true, true, true, false, false); + } + + /** + * Runs {@link IProgressService#run(boolean, boolean, IRunnableWithProgress)} + * with the given {@code fork}/{@code cancelable} arguments and the given + * "Always run in background" preference, detects whether the progress dialog + * popped up while it was running (or that the call failed with the expected + * {@link SWTException} when {@code callFromUIThread} is {@code false}), and + * asserts the outcome against {@code expectDialogShown}. + * + * @param callFromUIThread whether {@link IProgressService#run} itself is + * called from this (UI) thread or from a plain + * background thread. Regardless of + * {@code fork}: {@code ProgressManager.run} + * unconditionally computes the dialog's default + * parent shell via {@code Display.getShells()} as + * its very first statement - before it even looks + * at {@code fork}, {@code cancelable} or the + * "Always run in background" preference - and + * {@code Display.getShells()} throws an + * {@link SWTException} with + * {@link SWT#ERROR_THREAD_INVALID_ACCESS} when + * called from any thread other than the display's + * own thread. So calling + * {@link IProgressService#run} off the UI thread + * always fails immediately and the operation never + * even starts - this is not specific to + * {@code fork == false}. When {@code false}, + * {@code expectDialogShown} must be {@code false} + * and {@code pumpUiEventsWhileRunning} is + * meaningless (the runnable's body is never + * reached). + * @param pumpUiEventsWhileRunning whether the runnable itself keeps pumping + * SWT events (rather than just blocking, e.g. + * via {@code Thread.sleep}) while it + * "works". Only matters - and is only safe + * to set - when {@code fork == false} and + * {@code callFromUIThread == true}: see + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_blockingUiThread_noDialogShown()} + * and + * {@link #testRun_noFork_cancelable_runInForeground_callFromUIThread_pumpingUiEventsWhileRunning_dialogShown()}. + */ + private void assertDialogShown(boolean fork, boolean cancelable, boolean runInBackgroundPref, + boolean callFromUIThread, boolean pumpUiEventsWhileRunning, boolean expectDialogShown) throws Exception { + runInBackground(runInBackgroundPref); + + Display display = PlatformUI.getWorkbench().getDisplay(); + IRunnableWithProgress longRunningRunnable = monitor -> { + int workUnits = 3; + // Give it enough time so that the progress dialog can appear + long workDurationMillis = (long) (progressService.getLongOperationTime() / workUnits * 1.5); + SubMonitor sub = SubMonitor.convert(monitor, workUnits); + for (int i = 0; i < workUnits; i++) { + try { + if (pumpUiEventsWhileRunning) { + // Behave like a real, responsive long-running operation: keep + // dispatching UI events instead of freezing the UI thread, so + // that the job opening the progress dialog gets a chance to run. + long deadline = System.currentTimeMillis() + workDurationMillis; + while (System.currentTimeMillis() < deadline) { + if (!display.readAndDispatch()) { + Thread.sleep(5); + } + } + } else { + Thread.sleep(workDurationMillis); + } + } catch (InterruptedException e) { + // do nothing + } + sub.worked(1); + } + }; + + if (!callFromUIThread) { + assertFalse(expectDialogShown, + "callFromUIThread=false always fails before any dialog could ever show - pass expectDialogShown=false"); + Throwable thrown = runOnNonUiThreadAndCaptureThrowable( + () -> progressService.run(fork, cancelable, longRunningRunnable)); + SWTException swtException = assertInstanceOf(SWTException.class, thrown, + () -> String.format( + "Expected IProgressService#run(fork=%s, cancelable=%s) called off the UI thread to fail " + + "with an SWTException (it always does, regardless of fork/cancelable/" + + "runInBackground, because ProgressManager.run computes the dialog's default " + + "parent shell via Display.getShells() before doing anything else), but got: %s", + fork, cancelable, thrown)); + assertEquals(SWT.ERROR_THREAD_INVALID_ACCESS, swtException.code, + "Expected the invalid-thread-access SWTException, but got a different SWTException: " + + swtException); + return; + } + + boolean dialogShown = runAndDetectProgressDialog( + () -> progressService.run(fork, cancelable, longRunningRunnable)); + + assertEquals(expectDialogShown, dialogShown, + String.format( + "Expected progress dialog shown=%s for fork=%s, cancelable=%s, runInBackground=%s, pumpUiEventsWhileRunning=%s", + expectDialogShown, fork, cancelable, runInBackgroundPref, pumpUiEventsWhileRunning)); + } + + private static void runInBackground(boolean value) { + WorkbenchPlugin.getDefault().getPreferenceStore().setValue(IPreferenceConstants.RUN_IN_BACKGROUND, value); + } + + /** + * Runs {@code action} on a plain, non-UI thread (i.e. not the SWT display + * thread) and returns whatever {@link Throwable} it throws - or + * {@code null} if it completes without throwing - after joining that + * thread. Used to verify that {@link IProgressService#run} refuses to be + * called off the UI thread; see + * {@link #assertDialogShown(boolean, boolean, boolean, boolean, boolean, boolean)} + * for why. + */ + private Throwable runOnNonUiThreadAndCaptureThrowable(ThrowingRunnable action) throws InterruptedException { + Throwable[] thrown = new Throwable[1]; + Thread nonUiThread = new Thread(() -> { + try { + action.run(); + } catch (Throwable t) { + thrown[0] = t; + } + }, "ProgressServiceTest-non-UI-caller"); + nonUiThread.start(); + nonUiThread.join(); + return thrown[0]; + } + + /** + * Runs {@code action} while watching for any newly shown {@link Shell} + * (i.e. one that did not already exist right before {@code action} started) + * becoming visible on the display. Uses an {@link SWT#Show} display filter + * rather than polling, because {@code Shell.setVisible(true)} (called by + * {@code Window.open()}) fires that event synchronously as a direct consequence + * of the call - it does not require the SWT event loop to be pumped. This + * matters because with {@code fork == false} the calling (UI) thread never gets + * to pump events again once the runnable starts (it just freezes), so a + * poll-based approach would never see a dialog that was already opened + * synchronously right before the freeze. + */ + private boolean runAndDetectProgressDialog(ThrowingRunnable action) throws Exception { + Display display = PlatformUI.getWorkbench().getDisplay(); + Set preexistingShells = new HashSet<>(Arrays.asList(display.getShells())); + AtomicBoolean dialogShown = new AtomicBoolean(false); + Listener showListener = event -> { + if (event.widget instanceof Shell shell && !preexistingShells.contains(shell)) { + dialogShown.set(true); + } + }; + + // SWT fires Show synchronously as part of Shell.setVisible(true) itself so + // it will fire even during the frozen, non-pumping case (i.e. fork == false). + display.addFilter(SWT.Show, showListener); + try { + action.run(); + } finally { + display.removeFilter(SWT.Show, showListener); + } + return dialogShown.get(); + } + + interface ThrowingRunnable { + void run() throws Exception; + } + +}