Bug 410426 - VisibleWhen from contribution factories not handled in

toolbars

ToolbarContributionRecord did not handle the factory case in
anyVisibleWhen() and always return false for contributions through
org.eclipse.ui.menus.ExtensionContributionFactory.

This commit also correct a ClassCastException in
ToolBarManagerRenderer.toBeRenderedUpdater EventHandler: the
IContributionItem is not always a ContributionItem, it could be a
MenuManager too. 

Bug410426Test contains two tests which checks the corrected behaviors.

Change-Id: I5fd8b73ab8224fe9bd549549fac5eb0daf1b322f
Signed-off-by: Maxime PORHEL <maxime.porhel@obeo.fr>

6 files changed, 264 insertions, 5 deletions

diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarContributionRecord.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarContributionRecord.java
index 4e707e1851a..72b7c378286 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarContributionRecord.java
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarContributionRecord.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2013 IBM Corporation and others.
+ * Copyright (c) 2011, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Maxime Porhel <maxime.porhel@obeo.fr> Obeo - Bug 410426
  ******************************************************************************/
 
 package org.eclipse.e4.ui.workbench.renderers.swt;
@@ -144,7 +145,16 @@ public class ToolBarContributionRecord {
 		if (toolbarContribution.getVisibleWhen() != null) {
 			return true;
 		}
-		for (MToolBarElement child : toolbarContribution.getChildren()) {
+
+		List<MToolBarElement> childrenToInspect;
+		if (toolbarContribution.getTransientData().get(FACTORY) != null) {
+			// See mergeIntoModel
+			childrenToInspect = this.generatedElements;
+		} else {
+			childrenToInspect = toolbarContribution.getChildren();
+		}
+
+		for (MToolBarElement child : childrenToInspect) {
 			if (child.getVisibleWhen() != null
 					|| child.getPersistedState().get(
 							MenuManagerRenderer.VISIBILITY_IDENTIFIER) != null) {
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java
index cbfc1f3da56..06aa40f0730 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java
@@ -7,6 +7,7 @@
  * 
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Maxime Porhel <maxime.porhel@obeo.fr> Obeo - Bug 410426
  *******************************************************************************/
 package org.eclipse.e4.ui.workbench.renderers.swt;
 
@@ -56,6 +57,7 @@ import org.eclipse.jface.action.GroupMarker;
 import org.eclipse.jface.action.IContributionItem;
 import org.eclipse.jface.action.IContributionManagerOverrides;
 import org.eclipse.jface.action.IToolBarManager;
+import org.eclipse.jface.action.MenuManager;
 import org.eclipse.jface.action.Separator;
 import org.eclipse.jface.action.ToolBarManager;
 import org.eclipse.swt.SWT;
@@ -169,8 +171,15 @@ public class ToolBarManagerRenderer extends SWTPartRenderer {
 					return;
 				}
 				ici.setVisible(itemModel.isVisible());
-				ToolBarManager parent = (ToolBarManager) ((ContributionItem) ici)
-						.getParent();
+
+				ToolBarManager parent = null;
+				if (ici instanceof MenuManager) {
+					parent = (ToolBarManager) ((MenuManager) ici).getParent();
+				} else if (ici instanceof ContributionItem) {
+					parent = (ToolBarManager) ((ContributionItem) ici)
+							.getParent();
+				}
+
 				if (parent != null) {
 					parent.markDirty();
 					parent.update(true);
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/Bug410426Test.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/Bug410426Test.java
new file mode 100644
index 00000000000..861f1200233
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/Bug410426Test.java
@@ -0,0 +1,127 @@
+/*******************************************************************************
+ * Copyright (c) 2014 Obeo and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Obeo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.ui.tests.menus;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.equinox.log.ExtendedLogReaderService;
+import org.eclipse.equinox.log.LogFilter;
+import org.eclipse.equinox.log.SynchronousLogListener;
+import org.eclipse.jface.action.IContributionItem;
+import org.eclipse.jface.action.ToolBarManager;
+import org.eclipse.swt.widgets.ToolBar;
+import org.eclipse.swt.widgets.ToolItem;
+import org.eclipse.ui.IWorkbenchWindow;
+import org.eclipse.ui.menus.IMenuService;
+import org.eclipse.ui.tests.harness.util.UITestCase;
+import org.osgi.framework.Bundle;
+import org.osgi.service.log.LogEntry;
+import org.osgi.service.log.LogListener;
+import org.osgi.service.log.LogService;
+
+/**
+ * @author Maxime Porhel
+ */
+public class Bug410426Test extends UITestCase {
+
+    public Bug410426Test(String testName) {
+        super(testName);
+    }
+
+    public void testToolbarContributionFromFactoryVisibility() throws Exception {
+        IWorkbenchWindow window = openTestWindow();
+        IMenuService menus = (IMenuService) window.getService(IMenuService.class);
+        ToolBarManager manager = new ToolBarManager();
+
+        try {
+            // populate contribution
+            populateTestToolbar(menus, manager);
+
+            // check the contributions and their visibility
+            IContributionItem[] items = manager.getItems();
+            assertEquals(6, items.length);
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_ITEM_WITHOUT_VISIBLE_WHEN, items, true);
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_ITEM_WITH_ALWAYS_TRUE_VISIBLE_WHEN, items, true);
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_ITEM_WITH_ALWAYS_FALSE_VISIBLE_WHEN, items, false);
+
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_MENU_MANAGER_WITHOUT_VISIBLE_WHEN, items, true);
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_MENU_MANAGER_WITH_ALWAYS_TRUE_VISIBLE_WHEN, items, true);
+            checkItem(DeclaredProgrammaticFactoryForToolbarVisibilityTest.TEST_ITEM_WITH_ALWAYS_FALSE_VISIBLE_WHEN, items, false);
+
+            // now get the tool items
+            ToolBar toolBar = manager.createControl(window.getShell());
+            manager.update(true);
+            ToolItem[] toolItems = toolBar.getItems();
+            assertEquals("Only four tool items should be created as there are four visible contributions on the six contributions:", 4, toolItems.length); //$NON-NLS-N$
+        } finally {
+            menus.releaseContributions(manager);
+        }
+    }
+
+    private void populateTestToolbar(IMenuService menus, ToolBarManager manager) {
+        menus.populateContributionManager(manager, "toolbar:org.eclipse.ui.tests.toolbarContributionFromFactoryVisibilityTest"); //$NON-NLS-N$
+    }
+
+    private void checkItem(String id, IContributionItem[] items, boolean expectedVisibility) {
+        IContributionItem item = getItemWithId(id, items);
+
+        assertNotNull(item);
+        assertEquals("The contribution item with id '" + id + "' has not the expected vibility:", expectedVisibility, item.isVisible()); //$NON-NLS-N$
+    }
+
+    private IContributionItem getItemWithId(String id, IContributionItem[] items) {
+        for (IContributionItem item : items) {
+            if (id.equals(item.getId())) {
+                return item;
+            }
+        }
+        return null;
+    }
+
+    public void testNoClassCastExceptionForMenuManagerToolbarContribution() throws Exception {
+        IWorkbenchWindow window = openTestWindow();
+        IMenuService menus = (IMenuService) window.getService(IMenuService.class);
+        ToolBarManager manager = new ToolBarManager();
+
+        //Add a log listener to detect the corrected ClassCastException in bug 410426.
+        final List<ClassCastException> cces = new ArrayList<ClassCastException>();
+        ExtendedLogReaderService log = (ExtendedLogReaderService) window.getService(ExtendedLogReaderService.class);
+        LogListener logListener = new SynchronousLogListener() {
+            public void logged(LogEntry entry) {
+                if (entry.getLevel() == LogService.LOG_ERROR && entry.getException() instanceof ClassCastException
+                        && entry.getException().getMessage().contains("MenuManager cannot be cast to org.eclipse.jface.action.ContributionItem")) { //$NON-NLS-N$
+                    cces.add((ClassCastException) entry.getException());
+                }
+            }
+        };
+        LogFilter logFilter = new LogFilter() {
+            public boolean isLoggable(Bundle bundle, String loggerName, int logLevel) {
+                return logLevel == LogService.LOG_ERROR && loggerName == null && "org.eclipse.equinox.event".equals(bundle.getSymbolicName()); //$NON-NLS-N$
+            }
+        };
+        log.addLogListener(logListener, logFilter);
+
+        try {
+            populateTestToolbar(menus, manager);
+
+            assertTrue("We should not get these 'MenuManager cannot be cast to org.eclipse.jface.action.ContributionItem' ClassCastException.", cces.isEmpty()); //$NON-NLS-N$
+
+            // check the contributions count.
+            IContributionItem[] items = manager.getItems();
+            assertEquals(6, items.length);
+        } finally {
+            menus.releaseContributions(manager);
+            log.removeLogListener(logListener);
+            cces.clear();
+        }
+    }
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/DeclaredProgrammaticFactoryForToolbarVisibilityTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/DeclaredProgrammaticFactoryForToolbarVisibilityTest.java
new file mode 100644
index 00000000000..35da80202d7
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/DeclaredProgrammaticFactoryForToolbarVisibilityTest.java
@@ -0,0 +1,107 @@
+/*******************************************************************************
+ * Copyright (c) 2014 Obeo and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Obeo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.ui.tests.menus;
+
+import org.eclipse.core.expressions.EvaluationResult;
+import org.eclipse.core.expressions.Expression;
+import org.eclipse.core.expressions.IEvaluationContext;
+import org.eclipse.jface.action.Action;
+import org.eclipse.jface.action.ActionContributionItem;
+import org.eclipse.jface.action.MenuManager;
+import org.eclipse.swt.widgets.ToolBar;
+import org.eclipse.ui.internal.expressions.AlwaysEnabledExpression;
+import org.eclipse.ui.menus.ExtensionContributionFactory;
+import org.eclipse.ui.menus.IContributionRoot;
+import org.eclipse.ui.services.IServiceLocator;
+
+public class DeclaredProgrammaticFactoryForToolbarVisibilityTest extends ExtensionContributionFactory {
+    public static final String TEST_ITEM_WITHOUT_VISIBLE_WHEN = "TestItemWithoutVisibleWhen";//$NON-NLS-1$
+
+    public static final String TEST_ITEM_WITH_ALWAYS_FALSE_VISIBLE_WHEN = "TestItemWithAlwaysFalseVisibleWhen"; //$NON-NLS-1$
+
+    public static final String TEST_ITEM_WITH_ALWAYS_TRUE_VISIBLE_WHEN = "TestItemWithAlwaysTrueVisibleWhen"; //$NON-NLS-1$
+
+    public static final String TEST_MENU_MANAGER_WITHOUT_VISIBLE_WHEN = "TestMenuManagerWithoutVisibleWhen"; //$NON-NLS-1$
+
+    public static final String TEST_MENU_MANAGER_WITH_ALWAYS_FALSE_VISIBLE_WHEN = "TestMenuManagerWithAlwaysFalseVisibleWhen"; //$NON-NLS-1$
+
+    public static final String TEST_MENU_MANAGER_WITH_ALWAYS_TRUE_VISIBLE_WHEN = "TestMenuManagerWithAlwaysTrueVisibleWhen"; //$NON-NLS-1$
+
+    public void createContributionItems(IServiceLocator serviceLocator, IContributionRoot additions) {
+        addContribution(additions, new TestAction(TEST_ITEM_WITHOUT_VISIBLE_WHEN), null);
+        addContribution(additions, new TestAction(TEST_ITEM_WITH_ALWAYS_TRUE_VISIBLE_WHEN), AlwaysEnabledExpression.INSTANCE);
+        addContribution(additions, new TestAction(TEST_ITEM_WITH_ALWAYS_FALSE_VISIBLE_WHEN), new AlwaysDisabledExpression());
+
+        additions.addContributionItem(new TestActionMenuManager(TEST_MENU_MANAGER_WITHOUT_VISIBLE_WHEN), null);
+        additions.addContributionItem(new TestActionMenuManager(TEST_MENU_MANAGER_WITH_ALWAYS_TRUE_VISIBLE_WHEN), AlwaysEnabledExpression.INSTANCE);
+        additions.addContributionItem(new TestActionMenuManager(TEST_MENU_MANAGER_WITH_ALWAYS_FALSE_VISIBLE_WHEN), new AlwaysDisabledExpression());
+    }
+
+    private void addContribution(IContributionRoot additions, Action action, Expression visibleWhen) {
+        additions.addContributionItem(new ActionContributionItem(action), visibleWhen);
+    }
+
+    private static class TestAction extends Action {
+        public TestAction(String id) {
+            super(id);
+            setId(id);
+        }
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.eclipse.gmf.runtime.common.ui.action.ActionMenuManager which is
+     * an implementation of an <code>IMenuManager</code> that inherits its UI
+     * (text + icon + hints) from a given action. When filled in a toolbar, the
+     * menu is rendered as a tool item.
+     *
+     * See Bug 410426 which corrects a ClassCastException in
+     * ToolbarManagerRenderer while looking for a IContributionItem parent ToolBarManager.
+     */
+    private static class TestActionMenuManager extends MenuManager {
+        private ActionContributionItem actionContributionItem;
+
+        public TestActionMenuManager(String id) {
+            super(id, id);
+            actionContributionItem = new ActionContributionItem(new TestAction(id));
+            add(new TestAction(TEST_ITEM_WITHOUT_VISIBLE_WHEN));
+        }
+
+        /*
+         * (non-Javadoc)
+         *
+         * @see
+         * org.eclipse.jface.action.IContributionItem#fill(org.eclipse.swt.widgets
+         * .ToolBar, int)
+         */
+        public void fill(ToolBar parent, int index) {
+            actionContributionItem.fill(parent, index);
+        }
+
+        /*
+         * (non-Javadoc)
+         *
+         * @see org.eclipse.jface.action.MenuManager#dispose()
+         */
+        @Override
+        public void dispose() {
+            super.dispose();
+            actionContributionItem.dispose();
+        }
+    }
+
+    private static class AlwaysDisabledExpression extends Expression {
+        public EvaluationResult evaluate(IEvaluationContext context) {
+            return EvaluationResult.FALSE;
+        }
+    }
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
index b1c573d7d7b..27096638f8e 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
@@ -40,6 +40,7 @@ public class MenusTestSuite extends TestSuite {
         addTest(new TestSuite(Bug231304Test.class));
         addTest(new TestSuite(ShowViewMenuTest.class));
         addTest(new TestSuite(Bug264804Test.class));
-		addTest(new TestSuite(MenuHelperTest.class));
+        addTest(new TestSuite(MenuHelperTest.class));
+        addTest(new TestSuite(Bug410426Test.class));
     }
 }
diff --git a/tests/org.eclipse.ui.tests/plugin.xml b/tests/org.eclipse.ui.tests/plugin.xml
index d3ddc14f571..e5ca34e76ff 100644
--- a/tests/org.eclipse.ui.tests/plugin.xml
+++ b/tests/org.eclipse.ui.tests/plugin.xml
@@ -4188,6 +4188,11 @@
          </command>
       </menuContribution>
       <menuContribution
+            allPopups="false"
+            class="org.eclipse.ui.tests.menus.DeclaredProgrammaticFactoryForToolbarVisibilityTest"
+            locationURI="toolbar:org.eclipse.ui.tests.toolbarContributionFromFactoryVisibilityTest">
+      </menuContribution>
+      <menuContribution
             locationURI="popup:org.eclipse.ui.tests.Bug264804?after=additions">
          <command
                commandId="org.eclipse.ui.file.close"