Bug 375826 - No SWT.Show event on selecting a menu from the RCPv20120814-132626

workbench

Move the pre and post processing of show and hide into the MenuManager
calls themselves.  Then we don't interfere with the SWT Show or Hide
events.

4 files changed, 251 insertions, 168 deletions

diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerHideProcessor.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerHideProcessor.java
new file mode 100644
index 00000000000..f066a6df04a
--- /dev/null
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerHideProcessor.java
@@ -0,0 +1,88 @@
+/*******************************************************************************
+ * Copyright (c) 2010, 2012 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.e4.ui.workbench.renderers.swt;
+
+import javax.inject.Inject;
+import org.eclipse.e4.core.contexts.IEclipseContext;
+import org.eclipse.e4.ui.model.application.ui.menu.MMenu;
+import org.eclipse.e4.ui.model.application.ui.menu.MPopupMenu;
+import org.eclipse.e4.ui.workbench.modeling.EModelService;
+import org.eclipse.e4.ui.workbench.swt.factories.IRendererFactory;
+import org.eclipse.jface.action.IMenuListener2;
+import org.eclipse.jface.action.IMenuManager;
+import org.eclipse.jface.action.MenuManager;
+import org.eclipse.swt.widgets.Menu;
+
+/**
+ * <code>MenuManagerHideProcessor</code> provides hooks for renderer processing
+ * before and after the <code>MenuManager</code> calls out to its
+ * <code>IMenuManagerListener2</code> for the <code>menuAboutToHide</code>
+ * events.
+ */
+public class MenuManagerHideProcessor implements IMenuListener2 {
+
+	@Inject
+	private EModelService modelService;
+
+	@Inject
+	private IRendererFactory rendererFactory;
+
+	@Inject
+	private MenuManagerRenderer renderer;
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see
+	 * org.eclipse.jface.action.IMenuListener#menuAboutToShow(org.eclipse.jface
+	 * .action.IMenuManager)
+	 */
+	public void menuAboutToShow(IMenuManager manager) {
+		if (!(manager instanceof MenuManager)) {
+			return;
+		}
+		MenuManager menuManager = (MenuManager) manager;
+		final MMenu menuModel = renderer.getMenuModel(menuManager);
+		final Menu menu = menuManager.getMenu();
+		if (menuModel instanceof MPopupMenu) {
+			hidePopup(menu, (MPopupMenu) menuModel, menuManager);
+		}
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see
+	 * org.eclipse.jface.action.IMenuListener2#menuAboutToHide(org.eclipse.jface
+	 * .action.IMenuManager)
+	 */
+	public void menuAboutToHide(IMenuManager manager) {
+	}
+
+	private void hidePopup(Menu menu, MPopupMenu menuModel,
+			MenuManager menuManager) {
+		final IEclipseContext popupContext = menuModel.getContext();
+		final IEclipseContext originalChild = (IEclipseContext) popupContext
+				.get(MenuManagerRendererFilter.TMP_ORIGINAL_CONTEXT);
+		popupContext.remove(MenuManagerRendererFilter.TMP_ORIGINAL_CONTEXT);
+		if (!menu.isDisposed()) {
+			menu.getDisplay().asyncExec(new Runnable() {
+				public void run() {
+					if (originalChild == null) {
+						popupContext.deactivate();
+					} else {
+						originalChild.activate();
+					}
+				}
+			});
+		}
+	}
+}
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRenderer.java
index ac990eff679..a439075a581 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRenderer.java
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRenderer.java
@@ -58,6 +58,7 @@ import org.eclipse.jface.action.GroupMarker;
 import org.eclipse.jface.action.IContributionItem;
 import org.eclipse.jface.action.MenuManager;
 import org.eclipse.jface.action.Separator;
+import org.eclipse.jface.internal.MenuManagerEventHelper;
 import org.eclipse.jface.resource.ImageDescriptor;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.DisposeEvent;
@@ -221,6 +222,10 @@ public class MenuManagerRenderer extends SWTPartRenderer {
 		display.addFilter(SWT.Hide, rendererFilter);
 		display.addFilter(SWT.Dispose, rendererFilter);
 		context.set(MenuManagerRendererFilter.class, rendererFilter);
+		MenuManagerEventHelper.showHelper = ContextInjectionFactory.make(
+				MenuManagerShowProcessor.class, context);
+		MenuManagerEventHelper.hideHelper = ContextInjectionFactory.make(
+				MenuManagerHideProcessor.class, context);
 
 	}
 
@@ -231,6 +236,13 @@ public class MenuManagerRenderer extends SWTPartRenderer {
 		eventBroker.unsubscribe(enabledUpdater);
 		eventBroker.unsubscribe(toBeRenderedUpdater);
 
+		ContextInjectionFactory.uninject(MenuManagerEventHelper.showHelper,
+				context);
+		MenuManagerEventHelper.showHelper = null;
+		ContextInjectionFactory.uninject(MenuManagerEventHelper.hideHelper,
+				context);
+		MenuManagerEventHelper.hideHelper = null;
+
 		context.remove(MenuManagerRendererFilter.class);
 		Display display = context.get(Display.class);
 		if (display != null && !display.isDisposed() && rendererFilter != null) {
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRendererFilter.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRendererFilter.java
index 2c0a32ff5fa..6243a852084 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRendererFilter.java
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerRendererFilter.java
@@ -10,8 +10,6 @@
  *******************************************************************************/
 package org.eclipse.e4.ui.workbench.renderers.swt;
 
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.HashSet;
 import javax.inject.Inject;
@@ -29,7 +27,6 @@ import org.eclipse.e4.ui.internal.workbench.ContributionsAnalyzer;
 import org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer;
 import org.eclipse.e4.ui.internal.workbench.swt.Policy;
 import org.eclipse.e4.ui.internal.workbench.swt.WorkbenchSWTActivator;
-import org.eclipse.e4.ui.model.application.ui.MContext;
 import org.eclipse.e4.ui.model.application.ui.MUIElement;
 import org.eclipse.e4.ui.model.application.ui.basic.MPart;
 import org.eclipse.e4.ui.model.application.ui.menu.MDirectMenuItem;
@@ -53,51 +50,13 @@ public class MenuManagerRendererFilter implements Listener {
 
 	public static final String NUL_MENU_ITEM = "(None Applicable)"; //$NON-NLS-1$
 
-	private static final String TMP_ORIGINAL_CONTEXT = "MenuServiceFilter.original.context"; //$NON-NLS-1$
+	static final String TMP_ORIGINAL_CONTEXT = "MenuServiceFilter.original.context"; //$NON-NLS-1$
 
 	private static void trace(String msg, Widget menu, MMenu menuModel) {
 		WorkbenchSWTActivator.trace(Policy.MENUS, msg + ": " + menu + ": " //$NON-NLS-1$ //$NON-NLS-2$
 				+ menuModel, null);
 	}
 
-	private static Method aboutToShow;
-
-	private static Method aboutToHide;
-
-	public static Method getAboutToShow() {
-		if (aboutToShow == null) {
-			try {
-				aboutToShow = MenuManager.class
-						.getDeclaredMethod("handleAboutToShow"); //$NON-NLS-1$
-				aboutToShow.setAccessible(true);
-			} catch (SecurityException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			} catch (NoSuchMethodException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			}
-		}
-		return aboutToShow;
-	}
-
-	public static Method getAboutToHide() {
-		if (aboutToHide == null) {
-			try {
-				aboutToHide = MenuManager.class
-						.getDeclaredMethod("handleAboutToHide"); //$NON-NLS-1$
-				aboutToHide.setAccessible(true);
-			} catch (SecurityException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			} catch (NoSuchMethodException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			}
-		}
-		return aboutToHide;
-	}
-
 	@Inject
 	private Logger logger;
 
@@ -156,7 +115,6 @@ public class MenuManagerRendererFilter implements Listener {
 		}
 
 		// fill in all of the pieces
-		MMenu menuModel = null;
 		MenuManager menuManager = null;
 		Object obj = menu.getData(AbstractPartRenderer.OWNING_ME);
 		if (obj == null) {
@@ -180,103 +138,15 @@ public class MenuManagerRendererFilter implements Listener {
 				}
 			}
 		}
-		if (obj == null && menu.getParentItem() != null) {
-			obj = menu.getParentItem().getData(AbstractPartRenderer.OWNING_ME);
-			if (obj == null) {
-				// hack because MenuManager doesn't do a setData()
-				Object tmp = menu.getParentItem().getData();
-				if (tmp instanceof MenuManager) {
-					MenuManager tmpManager = (MenuManager) tmp;
-					if (menu == tmpManager.getMenu()) {
-						// Eureka! We found a match, by golly!
-						menuManager = tmpManager;
-						obj = renderer.getMenuModel(tmpManager);
-					}
-				}
-			}
-		}
-		if (obj instanceof MMenu) {
-			menuModel = (MMenu) obj;
-		}
-		if (menuManager == null && menuModel != null) {
-			menuManager = renderer.getManager(menuModel);
-		}
-
-		switch (event.type) {
-		case SWT.Show:
-			handleShow(event, menu, menuModel, menuManager);
-			break;
-		case SWT.Hide:
-			handleHide(event, menu, menuModel, menuManager);
-			break;
-		}
-
-	}
-
-	private void handleShow(final Event event, final Menu menu,
-			MMenu menuModel, MenuManager menuManager) {
-		if (menuModel != null && menuManager != null) {
-			cleanUp(menu, menuModel, menuManager);
-		}
-		if (menuModel instanceof MPopupMenu) {
-			showPopup(event, menu, (MPopupMenu) menuModel, menuManager);
-		}
-		if (menuModel != null) {
-			showMenu(event, menu, menuModel, menuManager);
-		} else {
-			trace("Incorrect menu model to work with " + menuManager, menu, menuModel); //$NON-NLS-1$
-		}
-	}
-
-	private void handleHide(final Event event, final Menu menu,
-			MMenu menuModel, MenuManager menuManager) {
-		if (menuModel instanceof MPopupMenu) {
-			hidePopup(event, menu, (MPopupMenu) menuModel, menuManager);
-		}
-	}
-
-	public void showMenu(final Event event, final Menu menu,
-			final MMenu menuModel, MenuManager menuManager) {
-		AbstractPartRenderer obj = rendererFactory.getRenderer(menuModel,
-				menu.getParent());
-		if (!(obj instanceof MenuManagerRenderer)) {
-			trace("Not the correct renderer: " + obj, menu, menuModel); //$NON-NLS-1$
-			return;
-		}
-		MenuManagerRenderer renderer = (MenuManagerRenderer) obj;
-		if (menuModel.getWidget() == null) {
-			renderer.bindWidget(menuModel, menuManager.getMenu());
-		}
-
-		Method handleAboutToShow = getAboutToShow();
-		try {
-			handleAboutToShow.invoke(menuManager);
-		} catch (IllegalArgumentException e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
-		} catch (IllegalAccessException e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
-		} catch (InvocationTargetException e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
-		}
-
-		final IEclipseContext evalContext;
-		if (menuModel instanceof MContext) {
-			evalContext = ((MContext) menuModel).getContext();
-		} else {
-			evalContext = modelService.getContainingContext(menuModel);
-		}
-		updateElementVisibility(menuModel, renderer, menuManager, evalContext,
-				2, true);
-
-		// last thing to do, kill the event and update the menu manager
-		event.type = SWT.None;
-		event.doit = false;
-		menuManager.update(false);
 	}
 
+	/**
+	 * @param info
+	 * @param menuModel
+	 * @param renderer
+	 * @param evalContext
+	 * @param recurse
+	 */
 	public static void collectInfo(ExpressionInfo info, final MMenu menuModel,
 			final MenuManagerRenderer renderer,
 			final IEclipseContext evalContext, boolean recurse) {
@@ -371,36 +241,6 @@ public class MenuManagerRendererFilter implements Listener {
 		}
 	}
 
-	public void hidePopup(Event event, Menu menu, MPopupMenu menuModel,
-			MenuManager menuManager) {
-		final IEclipseContext popupContext = menuModel.getContext();
-		final IEclipseContext originalChild = (IEclipseContext) popupContext
-				.get(TMP_ORIGINAL_CONTEXT);
-		popupContext.remove(TMP_ORIGINAL_CONTEXT);
-		if (!menu.isDisposed()) {
-			menu.getDisplay().asyncExec(new Runnable() {
-				public void run() {
-					if (originalChild == null) {
-						popupContext.deactivate();
-					} else {
-						originalChild.activate();
-					}
-				}
-			});
-		}
-	}
-
-	public void showPopup(final Event event, final Menu menu,
-			final MPopupMenu menuModel, MenuManager menuManager) {
-		// System.err.println("showPopup: " + menuModel + "\n\t" + menu);
-		// we need some context foolery here
-		final IEclipseContext popupContext = menuModel.getContext();
-		final IEclipseContext parentContext = popupContext.getParent();
-		final IEclipseContext originalChild = parentContext.getActiveChild();
-		popupContext.activate();
-		popupContext.set(TMP_ORIGINAL_CONTEXT, originalChild);
-	}
-
 	void setEnabled(MHandledMenuItem item) {
 		if (!item.isToBeRendered() || !item.isVisible()
 				|| item.getWidget() == null) {
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerShowProcessor.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerShowProcessor.java
new file mode 100644
index 00000000000..e10afdd4bab
--- /dev/null
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/MenuManagerShowProcessor.java
@@ -0,0 +1,143 @@
+/*******************************************************************************
+ * Copyright (c) 2010, 2012 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.e4.ui.workbench.renderers.swt;
+
+import java.util.HashMap;
+import javax.inject.Inject;
+import org.eclipse.e4.core.contexts.IEclipseContext;
+import org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer;
+import org.eclipse.e4.ui.internal.workbench.swt.Policy;
+import org.eclipse.e4.ui.internal.workbench.swt.WorkbenchSWTActivator;
+import org.eclipse.e4.ui.model.application.ui.MContext;
+import org.eclipse.e4.ui.model.application.ui.menu.MMenu;
+import org.eclipse.e4.ui.model.application.ui.menu.MPopupMenu;
+import org.eclipse.e4.ui.workbench.modeling.EModelService;
+import org.eclipse.e4.ui.workbench.swt.factories.IRendererFactory;
+import org.eclipse.jface.action.IMenuListener2;
+import org.eclipse.jface.action.IMenuManager;
+import org.eclipse.jface.action.MenuManager;
+import org.eclipse.swt.widgets.Menu;
+import org.eclipse.swt.widgets.Widget;
+
+/**
+ * <code>MenuManagerShowProcessor</code> provides hooks for renderer processing
+ * before and after the <code>MenuManager</code> calls out to its
+ * <code>IMenuManagerListener2</code> for the <code>menuAboutToShow</code>
+ * events.
+ */
+public class MenuManagerShowProcessor implements IMenuListener2 {
+
+	private static void trace(String msg, Widget menu, MMenu menuModel) {
+		WorkbenchSWTActivator.trace(Policy.MENUS, msg + ": " + menu + ": " //$NON-NLS-1$ //$NON-NLS-2$
+				+ menuModel, null);
+	}
+
+	@Inject
+	private EModelService modelService;
+
+	@Inject
+	private IRendererFactory rendererFactory;
+
+	@Inject
+	private MenuManagerRenderer renderer;
+
+	private HashMap<Menu, Runnable> pendingCleanup = new HashMap<Menu, Runnable>();
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see
+	 * org.eclipse.jface.action.IMenuListener#menuAboutToShow(org.eclipse.jface
+	 * .action.IMenuManager)
+	 */
+	public void menuAboutToShow(IMenuManager manager) {
+		if (!(manager instanceof MenuManager)) {
+			return;
+		}
+		MenuManager menuManager = (MenuManager) manager;
+		final MMenu menuModel = renderer.getMenuModel(menuManager);
+		final Menu menu = menuManager.getMenu();
+		if (menuModel != null && menuManager != null) {
+			cleanUp(menu, menuModel, menuManager);
+		}
+		if (menuModel instanceof MPopupMenu) {
+			showPopup(menu, (MPopupMenu) menuModel, menuManager);
+		}
+		AbstractPartRenderer obj = rendererFactory.getRenderer(menuModel,
+				menu.getParent());
+		if (!(obj instanceof MenuManagerRenderer)) {
+			trace("Not the correct renderer: " + obj, menu, menuModel); //$NON-NLS-1$
+			return;
+		}
+		MenuManagerRenderer renderer = (MenuManagerRenderer) obj;
+		if (menuModel.getWidget() == null) {
+			renderer.bindWidget(menuModel, menuManager.getMenu());
+		}
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see
+	 * org.eclipse.jface.action.IMenuListener2#menuAboutToHide(org.eclipse.jface
+	 * .action.IMenuManager)
+	 */
+	public void menuAboutToHide(IMenuManager manager) {
+		if (!(manager instanceof MenuManager)) {
+			return;
+		}
+		MenuManager menuManager = (MenuManager) manager;
+		final MMenu menuModel = renderer.getMenuModel(menuManager);
+		final Menu menu = menuManager.getMenu();
+		if (menuModel != null) {
+			showMenu(menu, menuModel, menuManager);
+		}
+	}
+
+	private void cleanUp(final Menu menu, MMenu menuModel,
+			MenuManager menuManager) {
+		trace("cleanUp", menu, null); //$NON-NLS-1$
+		if (pendingCleanup.isEmpty()) {
+			return;
+		}
+		Runnable cleanUp = pendingCleanup.remove(menu);
+		if (cleanUp != null) {
+			trace("cleanUp.run()", menu, null); //$NON-NLS-1$
+			cleanUp.run();
+		}
+	}
+
+	private void showPopup(final Menu menu, final MPopupMenu menuModel,
+			MenuManager menuManager) {
+		// System.err.println("showPopup: " + menuModel + "\n\t" + menu);
+		// we need some context foolery here
+		final IEclipseContext popupContext = menuModel.getContext();
+		final IEclipseContext parentContext = popupContext.getParent();
+		final IEclipseContext originalChild = parentContext.getActiveChild();
+		popupContext.activate();
+		popupContext.set(MenuManagerRendererFilter.TMP_ORIGINAL_CONTEXT,
+				originalChild);
+	}
+
+	private void showMenu(final Menu menu, final MMenu menuModel,
+			MenuManager menuManager) {
+
+		final IEclipseContext evalContext;
+		if (menuModel instanceof MContext) {
+			evalContext = ((MContext) menuModel).getContext();
+		} else {
+			evalContext = modelService.getContainingContext(menuModel);
+		}
+		MenuManagerRendererFilter.updateElementVisibility(menuModel, renderer,
+				menuManager, evalContext, 2, true);
+	}
+
+}