Skip to content

Commit 9007215

Browse files
fedejeanneCopilot
fedejeanne
and
Copilot
committed
Honor 'Always run in background' in IProgressService.run() #4202
Fixes #4202 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1044ad8 commit 9007215

3 files changed

Lines changed: 211 additions & 29 deletions

File tree

‎bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/ProgressManager.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,22 +1075,28 @@ public void showInDialog(Shell shell, Job job) {
10751075
@Override
10761076
public void run(boolean fork, boolean cancelable, IRunnableWithProgress runnable)
10771077
throws InvocationTargetException, InterruptedException {
1078+
if (shouldRunInBackground()) {
1079+
// The "Always run in background" preference must suppress the modal
1080+
// progress dialog for every fork/cancelable combination, not just the
1081+
// backward compatible (fork==false || cancelable==false) path below.
1082+
final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog(
1083+
ProgressManagerUtil.getDefaultParent());
1084+
dialog.setOpenOnRun(false);
1085+
dialog.run(fork, cancelable, runnable);
1086+
return;
1087+
}
1088+
10781089
if (!fork || !cancelable) {
10791090
// Backward compatible code.
10801091
final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog(
10811092
ProgressManagerUtil.getDefaultParent());
1082-
if (shouldRunInBackground()) {
1083-
dialog.setOpenOnRun(false);
1093+
Job showDialogJob = scheduleProgressMonitorJob(dialog);
1094+
try {
10841095
dialog.run(fork, cancelable, runnable);
1085-
} else {
1086-
Job showDialogJob = scheduleProgressMonitorJob(dialog);
1087-
try {
1088-
dialog.run(fork, cancelable, runnable);
1089-
} finally {
1090-
// In case the dialog hasn't popped up yet, cancel it so it doesn't pop up after
1091-
// the operation finishes or unwinds with an exception.
1092-
showDialogJob.cancel();
1093-
}
1096+
} finally {
1097+
// In case the dialog hasn't popped up yet, cancel it so it doesn't pop up after
1098+
// the operation finishes or unwinds with an exception.
1099+
showDialogJob.cancel();
10941100
}
10951101
return;
10961102
}

‎examples/org.eclipse.ui.examples.job/src/org/eclipse/ui/examples/jobs/views/ProgressServiceView.java‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ public class ProgressServiceView extends ViewPart {
4444

4545
private static final int SLEEP_STEP_MS = 100;
4646

47-
private final ExecutorService nonUiExecutor = Executors.newSingleThreadExecutor();
47+
private final ExecutorService nonUiExecutor = Executors.newSingleThreadExecutor(r -> {
48+
Thread t = new Thread(r, "ProgressServiceView-nonUI"); //$NON-NLS-1$
49+
t.setDaemon(true);
50+
return t;
51+
});
4852

4953
private Button backgroundPrefField;
5054
private IPropertyChangeListener prefListener;
@@ -88,7 +92,14 @@ private void createPreferenceGroup(Composite parent) {
8892

8993
prefListener = event -> {
9094
if (IPreferenceConstants.RUN_IN_BACKGROUND.equals(event.getProperty())) {
91-
Display.getDefault().asyncExec(() -> {
95+
if (backgroundPrefField.isDisposed()) {
96+
return;
97+
}
98+
Display display = backgroundPrefField.getDisplay();
99+
if (display.isDisposed()) {
100+
return;
101+
}
102+
display.asyncExec(() -> {
92103
if (!backgroundPrefField.isDisposed()) {
93104
backgroundPrefField
94105
.setSelection(getPreferenceStore().getBoolean(IPreferenceConstants.RUN_IN_BACKGROUND));
@@ -188,25 +199,29 @@ private IRunnableWithProgress createSleepRunnable(long durationMillis, boolean p
188199
return monitor -> {
189200
int ticks = (int) Math.max(1, durationMillis / SLEEP_STEP_MS);
190201
monitor.beginTask("Simulated long-running operation", ticks); //$NON-NLS-1$
191-
for (int i = 0; i < ticks; i++) {
192-
if (monitor.isCanceled()) {
193-
return;
194-
}
195-
try {
196-
Thread.sleep(SLEEP_STEP_MS);
197-
} catch (InterruptedException e) {
198-
Thread.currentThread().interrupt();
199-
return;
200-
}
201-
if (pumpEvents) {
202-
Display display = Display.getCurrent();
203-
if (display != null) {
204-
while (display.readAndDispatch()) {
205-
// drain pending UI events so the heartbeat keeps ticking
202+
try {
203+
for (int i = 0; i < ticks; i++) {
204+
if (monitor.isCanceled()) {
205+
return;
206+
}
207+
try {
208+
Thread.sleep(SLEEP_STEP_MS);
209+
} catch (InterruptedException e) {
210+
Thread.currentThread().interrupt();
211+
return;
212+
}
213+
if (pumpEvents) {
214+
Display display = Display.getCurrent();
215+
if (display != null) {
216+
while (display.readAndDispatch()) {
217+
// drain pending UI events so the heartbeat keeps ticking
218+
}
206219
}
207220
}
221+
monitor.worked(1);
208222
}
209-
monitor.worked(1);
223+
} finally {
224+
monitor.done();
210225
}
211226
};
212227
}

‎tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressServiceTest.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
package org.eclipse.ui.tests.progress;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
5+
import java.util.Arrays;
6+
import java.util.HashSet;
7+
import java.util.Set;
8+
import java.util.concurrent.atomic.AtomicBoolean;
9+
10+
import org.eclipse.core.runtime.SubMonitor;
11+
import org.eclipse.jface.operation.IRunnableWithProgress;
12+
import org.eclipse.swt.SWT;
13+
import org.eclipse.swt.widgets.Display;
14+
import org.eclipse.swt.widgets.Listener;
15+
import org.eclipse.swt.widgets.Shell;
16+
import org.eclipse.ui.PlatformUI;
17+
import org.eclipse.ui.internal.IPreferenceConstants;
18+
import org.eclipse.ui.internal.WorkbenchPlugin;
19+
import org.eclipse.ui.internal.progress.FinishedJobs;
20+
import org.eclipse.ui.progress.IProgressService;
21+
import org.junit.After;
22+
import org.junit.Before;
23+
import org.junit.Test;
24+
25+
/**
26+
* @since 3.5
27+
*
28+
*/
29+
public class ProgressServiceTest extends ProgressTestCase {
30+
31+
private IProgressService progressService;
32+
33+
@Override
34+
@Before
35+
public void doSetUp() throws Exception {
36+
window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
37+
progressService = PlatformUI.getWorkbench().getProgressService();
38+
FinishedJobs.getInstance().clearAll();
39+
}
40+
41+
@Override
42+
@After
43+
public void doTearDown() throws Exception {
44+
FinishedJobs.getInstance().clearAll();
45+
WorkbenchPlugin.getDefault().getPreferenceStore().setToDefault(IPreferenceConstants.RUN_IN_BACKGROUND);
46+
super.doTearDown();
47+
}
48+
49+
@Test
50+
public void testRun_fork_cancelable_runInBackground_noDialogShown() throws Exception {
51+
assertDialogShown(true, true, true, false);
52+
}
53+
54+
@Test
55+
public void testRun_fork_cancelable_runInForeground_dialogShown() throws Exception {
56+
assertDialogShown(true, true, false, true);
57+
}
58+
59+
@Test
60+
public void testRun_fork_notCancelable_runInBackground_noDialogShown() throws Exception {
61+
assertDialogShown(true, false, true, false);
62+
}
63+
64+
@Test
65+
public void testRun_fork_notCancelable_runInForeground_dialogShown() throws Exception {
66+
assertDialogShown(true, false, false, true);
67+
}
68+
69+
@Test
70+
public void testRun_noFork_cancelable_runInBackground_noDialogShown() throws Exception {
71+
assertDialogShown(false, true, true, false);
72+
}
73+
74+
@Test
75+
public void testRun_noFork_cancelable_runInForeground_dialogShown() throws Exception {
76+
assertDialogShown(false, true, false, true);
77+
}
78+
79+
@Test
80+
public void testRun_noFork_notCancelable_runInBackground_noDialogShown() throws Exception {
81+
assertDialogShown(false, false, true, false);
82+
}
83+
84+
@Test
85+
public void testRun_noFork_notCancelable_runInForeground_dialogShown() throws Exception {
86+
assertDialogShown(false, false, false, true);
87+
}
88+
89+
/**
90+
* Runs {@link IProgressService#run(boolean, boolean, IRunnableWithProgress)}
91+
* with the given {@code fork}/{@code cancelable} arguments and the given
92+
* "Always run in background" preference, detects whether the progress dialog
93+
* popped up while it was running, and asserts that against
94+
* {@code expectDialogShown}.
95+
*/
96+
private void assertDialogShown(boolean fork, boolean cancelable, boolean runInBackgroundPref,
97+
boolean expectDialogShown) throws Exception {
98+
runInBackground(runInBackgroundPref);
99+
100+
boolean dialogShown = runAndDetectProgressDialog(() -> progressService.run(fork, cancelable, monitor -> {
101+
try {
102+
int workUnits = 3;
103+
// Give it enough time so that the progress dialog can appear
104+
long workDurationMillis = (long) (progressService.getLongOperationTime() / workUnits * 1.5);
105+
SubMonitor sub = SubMonitor.convert(monitor, workUnits);
106+
for (int i = 0; i < workUnits; i++) {
107+
Thread.sleep(workDurationMillis);
108+
sub.worked(1);
109+
}
110+
} catch (InterruptedException e) {
111+
// do nothing
112+
}
113+
}));
114+
115+
assertEquals(expectDialogShown, dialogShown,
116+
String.format("Expected progress dialog shown=%s for fork=%s, cancelable=%s, runInBackground=%s",
117+
expectDialogShown, fork, cancelable, runInBackgroundPref));
118+
}
119+
120+
/**
121+
* Runs {@code action} while watching for any <b>newly shown</b> {@link Shell}
122+
* (i.e. one that did not already exist right before {@code action} started)
123+
* becoming visible on the display. Uses an {@link SWT#Show} display filter
124+
* rather than polling, because {@code Shell.setVisible(true)} (called by
125+
* {@code Window.open()}) fires that event synchronously as a direct consequence
126+
* of the call - it does not require the SWT event loop to be pumped. This
127+
* matters because with {@code fork == false} the calling (UI) thread never gets
128+
* to pump events again once the runnable starts (it just freezes), so a
129+
* poll-based approach would never see a dialog that was already opened
130+
* synchronously right before the freeze.
131+
*/
132+
private boolean runAndDetectProgressDialog(ThrowingRunnable action) throws Exception {
133+
Display display = PlatformUI.getWorkbench().getDisplay();
134+
Set<Shell> preexistingShells = new HashSet<>(Arrays.asList(display.getShells()));
135+
AtomicBoolean dialogShown = new AtomicBoolean(false);
136+
Listener showListener = event -> {
137+
if (event.widget instanceof Shell shell && !preexistingShells.contains(shell)) {
138+
dialogShown.set(true);
139+
}
140+
};
141+
142+
// SWT fires  Show  synchronously as part of  Shell.setVisible(true)  itself so
143+
// it will fire even during the frozen, non-pumping case (i.e. fork == false).
144+
display.addFilter(SWT.Show, showListener);
145+
try {
146+
action.run();
147+
} finally {
148+
display.removeFilter(SWT.Show, showListener);
149+
}
150+
return dialogShown.get();
151+
}
152+
153+
static void runInBackground(boolean value) {
154+
WorkbenchPlugin.getDefault().getPreferenceStore().setValue(IPreferenceConstants.RUN_IN_BACKGROUND, value);
155+
}
156+
157+
interface ThrowingRunnable {
158+
void run() throws Exception;
159+
}
160+
161+
}

0 commit comments

Comments
 (0)