Skip to content

Commit ec2a668

Browse files
committed
Revert "Fix race condition in CleanupAddon.subscribeRenderingChanged"
This reverts commit 54b8aa2.
1 parent 2772712 commit ec2a668

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

bundles/org.eclipse.e4.ui.workbench.addons.swt/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddon.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,16 @@ private void subscribeRenderingChanged(
346346
int visCount = modelService.countRenderableChildren(container);
347347

348348
// Remove stacks with no visible children from the display (but not the
349-
// model). Since event delivery is synchronous (EventBroker.send via
350-
// EventAdmin.sendEvent and Display.syncExec), the visCount reflects
351-
// the actual state at the time of this event and can be acted upon
352-
// immediately without deferring via asyncExec.
353-
if (visCount == 0 && !isLastEditorStack(container)) {
354-
container.setToBeRendered(false);
355-
transferPrimaryDataStackIfRemoved(container);
349+
// model)
350+
final MElementContainer<MUIElement> theContainer = container;
351+
if (visCount == 0) {
352+
Display.getCurrent().asyncExec(() -> {
353+
int visCount1 = modelService.countRenderableChildren(theContainer);
354+
if (!isLastEditorStack(theContainer) && visCount1 == 0) {
355+
theContainer.setToBeRendered(false);
356+
transferPrimaryDataStackIfRemoved(theContainer);
357+
}
358+
});
356359
} else if (container.getParent() != null) { // omit detached windows
357360
// if there are rendered elements but none are 'visible' we should
358361
// make the container invisible as well

tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import org.eclipse.swt.widgets.Shell;
6565
import org.eclipse.swt.widgets.ToolBar;
6666
import org.eclipse.swt.widgets.Widget;
67+
import org.eclipse.ui.tests.harness.util.DisplayHelper;
6768
import org.junit.jupiter.api.AfterEach;
6869
import org.junit.jupiter.api.BeforeEach;
6970
import org.junit.jupiter.api.Disabled;
@@ -2446,9 +2447,12 @@ public void ensureCleanUpAddonCleansUp() {
24462447
assertTrue(partStackForPartBPartC.isToBeRendered(), " PartStack with children should be rendered");
24472448
partService.hidePart(partB);
24482449
partService.hidePart(partC);
2449-
contextRule.spinEventLoop();
2450-
assertFalse(partStackForPartBPartC.isToBeRendered(),
2451-
"CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed");
2450+
// DisplayHelper.waitForCondition() handles event processing via Display.sleep()
2451+
// and retries. Calling spinEventLoop() here creates a race condition where
2452+
// events may be processed before CleanupAddon's asyncExec() is queued (line 352).
2453+
assertTrue(
2454+
DisplayHelper.waitForCondition(Display.getDefault(), 30_000,
2455+
() -> !partStackForPartBPartC.isToBeRendered()), "CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed");
24522456
// PartStack with IPresentationEngine.NO_AUTO_COLLAPSE should not be removed
24532457
// even if children are removed
24542458
partService.hidePart(editor, true);

0 commit comments

Comments
 (0)