Bug 487606 - Detached windows should take UI decorations from parent window

Ensure shell image of detached windows are updated on changes to the parent window

Change-Id: If19b02017b1f2ca488ffb7f5d8ee0f8b07ff1672

2 files changed, 136 insertions, 2 deletions

diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/WBWRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/WBWRenderer.java
index efdeb6d836a..190e923f8a1 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/WBWRenderer.java
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/WBWRenderer.java
@@ -40,6 +40,7 @@ import org.eclipse.e4.ui.model.application.MApplication;
 import org.eclipse.e4.ui.model.application.ui.MContext;
 import org.eclipse.e4.ui.model.application.ui.MElementContainer;
 import org.eclipse.e4.ui.model.application.ui.MUIElement;
+import org.eclipse.e4.ui.model.application.ui.MUILabel;
 import org.eclipse.e4.ui.model.application.ui.advanced.MPerspective;
 import org.eclipse.e4.ui.model.application.ui.basic.MPart;
 import org.eclipse.e4.ui.model.application.ui.basic.MPartStack;
@@ -73,6 +74,7 @@ import org.eclipse.swt.events.TraverseEvent;
 import org.eclipse.swt.events.TraverseListener;
 import org.eclipse.swt.graphics.Color;
 import org.eclipse.swt.graphics.Font;
+import org.eclipse.swt.graphics.Image;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.graphics.Rectangle;
 import org.eclipse.swt.graphics.Resource;
@@ -174,6 +176,12 @@ public class WBWRenderer extends SWTPartRenderer {
 			theShell.setText(newTitle);
 		} else if (UIEvents.UILabel.ICONURI.equals(attName)) {
 			theShell.setImage(getImage(windowModel));
+			// child windows may take their shell icon from the parent
+			for (MWindow child : windowModel.getWindows()) {
+				if (child.getRenderer() instanceof WBWRenderer) {
+					((WBWRenderer) child.getRenderer()).handleParentChange(child);
+				}
+			}
 		} else if (UIEvents.UILabel.TOOLTIP.equals(attName) || UIEvents.UILabel.LOCALIZED_TOOLTIP.equals(attName)) {
 			String newTTip = (String) event.getProperty(UIEvents.EventTags.NEW_VALUE);
 			theShell.setToolTipText(newTTip);
@@ -252,6 +260,52 @@ public class WBWRenderer extends SWTPartRenderer {
 		themeDefinitionChanged.handleEvent(event);
 	}
 
+	@Inject
+	@Optional
+	private void subscribeTopicDetachedChanged(@UIEventTopic(UIEvents.Window.TOPIC_WINDOWS) Event event) {
+		/*
+		 * Handle any changes required for parent changes on detached windows.
+		 * This isn't quite straightforward as we don't see TOPIC_PARENT events
+		 * parent changes are only described as ADD and REMOVE on the
+		 * Window.TOPIC_WINDOWS and Application.TOPIC_CHILDREN.
+		 */
+		if (!(event.getProperty(UIEvents.EventTags.ELEMENT) instanceof MWindow))
+			return;
+
+		if (UIEvents.isREMOVE(event)) {
+			for (Object removed : UIEvents.asIterable(event, UIEvents.EventTags.OLD_VALUE)) {
+				if (removed instanceof MWindow && ((MWindow) removed).getRenderer() instanceof WBWRenderer) {
+					MWindow window = (MWindow) removed;
+					((WBWRenderer) window.getRenderer()).handleParentChange(window);
+				}
+			}
+		} else if (UIEvents.isADD(event)) {
+			for (Object removed : UIEvents.asIterable(event, UIEvents.EventTags.NEW_VALUE)) {
+				if (removed instanceof MWindow && ((MWindow) removed).getRenderer() instanceof WBWRenderer) {
+					MWindow window = (MWindow) removed;
+					((WBWRenderer) window.getRenderer()).handleParentChange(window);
+				}
+			}
+		}
+	}
+
+	/**
+	 * Update this child window with any values that may have been obtained from
+	 * the parent.
+	 *
+	 * @param child
+	 *            the child window (may now be orphaned)
+	 */
+	private void handleParentChange(MWindow child) {
+		// No widget == nothing to update
+		Shell theShell = (Shell) child.getWidget();
+		if (theShell == null)
+			return;
+
+		// Detached windows may take their shell icon from the parent window
+		theShell.setImage(getImage(child));
+	}
+
 	/**
 	 * Closes the provided detached window.
 	 *
@@ -398,8 +452,9 @@ public class WBWRenderer extends SWTPartRenderer {
 		if (wbwModel.getLabel() != null)
 			wbwShell.setText(wbwModel.getLocalizedLabel());
 
-		if (wbwModel.getIconURI() != null && wbwModel.getIconURI().length() > 0) {
-			wbwShell.setImage(getImage(wbwModel));
+		Image windowImage = getImage(wbwModel);
+		if (windowImage != null) {
+			wbwShell.setImage(windowImage);
 		} else {
 			// TODO: This should be added to the model, see bug 308494
 			// it allows for a range of icon sizes that the platform gets to
@@ -501,6 +556,19 @@ public class WBWRenderer extends SWTPartRenderer {
 	}
 
 	@Override
+	public Image getImage(MUILabel element) {
+		Image image = super.getImage(element);
+		if (image == null && element instanceof MWindow) {
+			// Detached windows should take their image from parent window
+			MWindow parent = modelService.getTopLevelWindowFor((MWindow) element);
+			if (parent != null && parent != element) {
+				image = getImage(parent);
+			}
+		}
+		return image;
+	}
+
+	@Override
 	public void hookControllerLogic(MUIElement me) {
 		super.hookControllerLogic(me);
 
diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/MWindowTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/MWindowTest.java
index ba9e79e47fc..edb7f08a763 100644
--- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/MWindowTest.java
+++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/MWindowTest.java
@@ -13,13 +13,16 @@ package org.eclipse.e4.ui.tests.workbench;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import org.eclipse.e4.core.commands.CommandServiceAddon;
 import org.eclipse.e4.core.contexts.ContextInjectionFactory;
 import org.eclipse.e4.core.contexts.IEclipseContext;
+import org.eclipse.e4.ui.di.UISynchronize;
 import org.eclipse.e4.ui.internal.workbench.E4Workbench;
 import org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer;
 import org.eclipse.e4.ui.internal.workbench.swt.E4Application;
@@ -63,6 +66,18 @@ public class MWindowTest {
 		appContext = E4Application.createDefaultContext();
 		ContextInjectionFactory.make(CommandServiceAddon.class, appContext);
 		appContext.set(E4Workbench.PRESENTATION_URI_ARG, PartRenderingEngine.engineURI);
+		appContext.set(UISynchronize.class, new UISynchronize() {
+			@Override
+			public void syncExec(Runnable runnable) {
+				runnable.run();
+			}
+
+			@Override
+			public void asyncExec(final Runnable runnable) {
+				runnable.run();
+			}
+		});
+		ContextInjectionFactory.setDefault(appContext);
 		ems = appContext.get(EModelService.class);
 	}
 
@@ -72,6 +87,7 @@ public class MWindowTest {
 			wb.close();
 		}
 		appContext.dispose();
+		ContextInjectionFactory.setDefault(null);
 	}
 
 	@Test
@@ -91,6 +107,7 @@ public class MWindowTest {
 		assertNotNull(topWidget);
 		assertTrue(topWidget instanceof Shell);
 		assertEquals("MyWindow", ((Shell) topWidget).getText());
+		// XXX Use of ACTIVE_SHELL fails when running standalone
 		assertEquals(topWidget, appContext.get(IServiceConstants.ACTIVE_SHELL));
 	}
 
@@ -428,6 +445,55 @@ public class MWindowTest {
 		assertEquals(300, shell.getBounds().height);
 	}
 
+	@Test
+	public void testDetachedWindow() {
+		final MWindow window = ems.createModelElement(MWindow.class);
+		window.setLabel("MyWindow");
+		final MWindow detachedWindow = ems.createModelElement(MWindow.class);
+		detachedWindow.setLabel("DetachedWindow");
+		window.getWindows().add(detachedWindow);
+
+		MApplication application = ems.createModelElement(MApplication.class);
+		application.getChildren().add(window);
+		application.setContext(appContext);
+		appContext.set(MApplication.class, application);
+
+		wb = new E4Workbench(application, appContext);
+		wb.createAndRunUI(window);
+
+		assertTrue(window.getWidget() instanceof Shell);
+		assertTrue(detachedWindow.getWidget() instanceof Shell);
+		Shell topShell = (Shell) window.getWidget();
+		Shell detachedShell = (Shell) detachedWindow.getWidget();
+		assertEquals(window, ems.getContainer(detachedWindow));
+		assertNull("Should have no shell image", topShell.getImage());
+		assertEquals("Detached should have same image", topShell.getImage(), detachedShell.getImage());
+
+		// now set icon on top-level window; detached window should inherit it
+		window.setIconURI("platform:/plugin/org.eclipse.e4.ui.tests/icons/filenav_nav.png");
+		while (topShell.getDisplay().readAndDispatch()) {
+		}
+		assertNotNull("Should have shell image", topShell.getImage());
+		assertEquals("Detached should have same image", topShell.getImage(), detachedShell.getImage());
+
+		// change top-level icon; detached window should inherit it
+		window.setIconURI(null);
+		while (topShell.getDisplay().readAndDispatch()) {
+		}
+		assertNull("Should have no shell image", topShell.getImage());
+		assertEquals("Detached should have same image", topShell.getImage(), detachedShell.getImage());
+
+		// turn detached into top-level window; inherited icon should be removed
+		window.setIconURI("platform:/plugin/org.eclipse.e4.ui.tests/icons/filenav_nav.png");
+		application.getChildren().add(detachedWindow);
+		while (topShell.getDisplay().readAndDispatch()) {
+		}
+		assertTrue(window.getWindows().isEmpty());
+		assertNotEquals(window, ems.getContainer(detachedWindow));
+		assertNotNull(topShell.getImage());
+		assertNull(detachedShell.getImage());
+	}
+
 	private MPart getContributedPart(MWindow window) {
 		MPartSashContainer psc = (MPartSashContainer) window.getChildren().get(0);
 		MPartStack stack = (MPartStack) psc.getChildren().get(0);