GP-1440 fixing issue global actions shared across windows getting the

wrong context

Ghidra/Framework/Docking/src/main/java/docking/ActionToGuiMapper.java

@@ -100,7 +100,7 @@ public class ActionToGuiMapper {
 
 	void update() {
 		menuAndToolBarManager.update();
-		contextChangedAll();
+		contextChanged();
 	}
 
 	void dispose() {
@@ -117,12 +117,8 @@ public class ActionToGuiMapper {
 		return menuBarMenuHandler;
 	}
 
-	void contextChangedAll() {
-		menuAndToolBarManager.contextChangedAll();
-	}
-
-	void contextChanged(ComponentPlaceholder placeHolder) {
-		menuAndToolBarManager.contextChanged(placeHolder);
+	void contextChanged() {
+		menuAndToolBarManager.contextChanged();
 	}
 
 	PopupActionManager getPopupActionManager() {

Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java

@@ -159,7 +159,7 @@ public class ComponentPlaceholder {
 		return group;
 	}
 
-	DetachedWindowNode getWindowNode() {
+	DetachedWindowNode getDetachedWindowNode() {
 		Node node = compNode.parent;
 		while (node != null) {
 			if (node instanceof DetachedWindowNode) {
@@ -170,6 +170,17 @@ public class ComponentPlaceholder {
 		return null;
 	}
 
+	WindowNode getWindowNode() {
+		Node node = compNode.parent;
+		while (node != null) {
+			if (node instanceof WindowNode) {
+				return (WindowNode) node;
+			}
+			node = node.parent;
+		}
+		return null;
+	}
+
 	WindowNode getTopLevelNode() {
 		if (compNode == null) {
 			return null;
@@ -279,9 +290,9 @@ public class ComponentPlaceholder {
 
 	// makes sure that the given window is not in an iconified state
 	private void activateWindow() {
-		DetachedWindowNode windowNode = getWindowNode();
+		DetachedWindowNode windowNode = getDetachedWindowNode();
 		if (windowNode != null) {
-			Window window = getWindowNode().getWindow();
+			Window window = getDetachedWindowNode().getWindow();
 			if (window instanceof Frame) {
 				Frame frame = (Frame) window;
 				frame.setState(Frame.NORMAL);

Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java

@@ -630,7 +630,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 		placeholder.update();
 		scheduleUpdate();
 
-		DetachedWindowNode wNode = placeholder.getWindowNode();
+		DetachedWindowNode wNode = placeholder.getDetachedWindowNode();
 		if (wNode != null) {
 			wNode.updateTitle();
 		}
@@ -2225,19 +2225,16 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 	}
 
 	public void contextChanged(ComponentProvider provider) {
-
-		if (provider == null) {
-			actionToGuiMapper.contextChangedAll(); // this updates the actions for all windows
-			return;
+		// if provider is specified, update its local menu and tool bar actions
+		if (provider != null) {
+			ComponentPlaceholder placeholder = getActivePlaceholder(provider);
+			if (placeholder != null) {
+				placeholder.contextChanged();
+			}
 		}
 
-		ComponentPlaceholder placeholder = getActivePlaceholder(provider);
-		if (placeholder == null) {
-			return;
-		}
-
-		placeholder.contextChanged();
-		actionToGuiMapper.contextChanged(placeholder);
+		// always update the global tool menu and tool bar actions
+		actionToGuiMapper.contextChanged();
 	}
 
 	/**
@@ -2326,12 +2323,16 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 
 	}
 
-	void notifyContextListeners(ComponentPlaceholder placeHolder, ActionContext actionContext) {
-
-		if (placeHolder == focusedPlaceholder) {
-			for (DockingContextListener listener : contextListeners) {
-				listener.contextChanged(actionContext);
-			}
+	/**
+	 * This call will notify any context listeners that the context has changed.
+	 * 
+	 * <p>Our {@link #contextChanged(ComponentProvider)} method will eventually call back into this
+	 * method after any buffering has taken place.
+	 * @param context the context
+	 */
+	void doContextChanged(ActionContext context) {
+		for (DockingContextListener listener : contextListeners) {
+			listener.contextChanged(context);
 		}
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/GlobalMenuAndToolBarManager.java

@@ -21,7 +21,12 @@ import docking.action.DockingActionIf;
 import docking.menu.MenuGroupMap;
 import docking.menu.MenuHandler;
 import ghidra.util.Swing;
+import ghidra.util.task.AbstractSwingUpdateManager;
+import ghidra.util.task.SwingUpdateManager;
 
+/**
+ * Class to manage all the global actions that show up on the main tool menubar or toolbar
+ */
 public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 
 	private Map<WindowNode, WindowActionManager> windowToActionManagerMap;
@@ -29,6 +34,12 @@ public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 	private final MenuGroupMap menuGroupMap;
 	private final DockingWindowManager windowManager;
 
+	// set the max delay low enough so that users can't interact with out-of-date actions before
+	// they get updated
+	private SwingUpdateManager updateManager =
+		new SwingUpdateManager(AbstractSwingUpdateManager.DEFAULT_MIN_DELAY, 500,
+			"Context Update Manager", () -> updateActions());
+
 	public GlobalMenuAndToolBarManager(DockingWindowManager windowManager, MenuHandler menuHandler,
 			MenuGroupMap menuGroupMap) {
 
@@ -77,11 +88,15 @@ public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 	}
 
 	public void dispose() {
-		for (WindowActionManager actionManager : windowToActionManagerMap.values()) {
-			actionManager.dispose();
-		}
-		windowToActionManagerMap.clear();
-
+		// make sure this is on the swing thread to avoid clearing stuff while the swing update
+		// manager is firing its processContextChanged() call
+		Swing.runIfSwingOrRunLater(() -> {
+			updateManager.dispose();
+			for (WindowActionManager actionManager : windowToActionManagerMap.values()) {
+				actionManager.dispose();
+			}
+			windowToActionManagerMap.clear();
+		});
 	}
 
 	@Override
@@ -113,13 +128,7 @@ public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 
 	@Override
 	public void dockingWindowFocusChanged(WindowNode windowNode) {
-		//update global menus and toolbars for this window
-		ComponentPlaceholder lastFocused = windowNode.getLastFocusedProviderInWindow();
-		WindowActionManager windowActionManager = windowToActionManagerMap.get(windowNode);
-		if (windowActionManager == null) {
-			return;
-		}
-		windowActionManager.contextChanged(lastFocused);
+		updateManager.updateLater();
 	}
 
 	private void removeWindowActionManager(WindowNode windowNode) {
@@ -135,8 +144,7 @@ public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 			new WindowActionManager(windowNode, menuHandler, windowManager, menuGroupMap);
 		windowToActionManagerMap.put(windowNode, newActionManager);
 		newActionManager.setActions(actionsForWindow);
-		ComponentPlaceholder lastFocused = windowNode.getLastFocusedProviderInWindow();
-		newActionManager.contextChanged(lastFocused);
+		updateManager.updateLater();
 	}
 
 	private List<DockingActionIf> getActionsForWindow(WindowNode windowNode) {
@@ -152,34 +160,80 @@ public class GlobalMenuAndToolBarManager implements DockingWindowListener {
 		return actionsForWindow;
 	}
 
-	public void contextChangedAll() {
-		Swing.runIfSwingOrRunLater(this::updateAllWindowActions);
+	public void contextChanged() {
+		// schedule an update for all the global actions
+		updateManager.updateLater();
 	}
 
-	private void updateAllWindowActions() {
+	private void updateActions() {
+
+		//
+		// The focused window's actions must be notified after all other windows in order to
+		// prevent incorrect context updates.   We will first update all non-focused windows,
+		// then the focused window and then finally tell the Docking Window Manager to update.
+		//
+		WindowNode focusedWindowNode = getFocusedWindowNode();
+		Set<DockingActionIf> focusedWindowActions = getWindowActions(focusedWindowNode);
+
+		ActionContext globalContext = windowManager.getDefaultToolContext();
 		for (WindowNode windowNode : windowToActionManagerMap.keySet()) {
-			ComponentPlaceholder lastFocused = windowNode.getLastFocusedProviderInWindow();
-			windowToActionManagerMap.get(windowNode).contextChanged(lastFocused);
+			if (windowNode == focusedWindowNode) {
+				continue; // the focused window will be called after this loop later
+			}
+
+			WindowActionManager actionManager = windowToActionManagerMap.get(windowNode);
+			ActionContext localContext = getContext(windowNode);
+			actionManager.contextChanged(globalContext, localContext, focusedWindowActions);
+		}
+
+		// now update the focused window's actions
+		WindowActionManager actionManager = windowToActionManagerMap.get(focusedWindowNode);
+		ActionContext focusedContext = getContext(focusedWindowNode);
+		if (actionManager != null) {
+			actionManager.contextChanged(globalContext, focusedContext, Collections.emptySet());
+		}
+
+		// update the docking window manager ; no focused context when no window is focused
+		if (focusedContext != null) {
+			windowManager.doContextChanged(focusedContext);
 		}
 	}
 
-	public void contextChanged(ComponentPlaceholder placeHolder) {
-		if (placeHolder == null) {
-			return;
+	private ActionContext getContext(WindowNode windowNode) {
+		if (windowNode == null) {
+			return null;
 		}
 
-		WindowNode topLevelNode = placeHolder.getTopLevelNode();
-		if (topLevelNode == null) {
-			return;
+		ActionContext context = null;
+		ComponentPlaceholder placeholder = windowNode.getLastFocusedProviderInWindow();
+		if (placeholder != null) {
+			ComponentProvider provider = placeholder.getProvider();
+			if (provider != null) {
+				context = provider.getActionContext(null);
+			}
 		}
-
-		if (topLevelNode.getLastFocusedProviderInWindow() != placeHolder) {
-			return; // actions in this window are not currently responding to this provider
-		}
-
-		WindowActionManager windowActionManager = windowToActionManagerMap.get(topLevelNode);
-		if (windowActionManager != null) {
-			windowActionManager.contextChanged(placeHolder);
+		if (context == null) {
+			context = new ActionContext();
 		}
+		return context;
 	}
+
+	private Set<DockingActionIf> getWindowActions(WindowNode windowNode) {
+		if (windowNode != null) {
+			WindowActionManager windowActionManager = windowToActionManagerMap.get(windowNode);
+			if (windowActionManager != null) {
+				return windowActionManager.getOriginalActions();
+			}
+		}
+		return Collections.emptySet();
+	}
+
+	private WindowNode getFocusedWindowNode() {
+		ComponentPlaceholder focusedComponent = windowManager.getFocusedComponent();
+		if (focusedComponent == null) {
+			return null;
+		}
+		return focusedComponent.getWindowNode();
+	}
+
 }

Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java

@@ -21,7 +21,6 @@ import javax.swing.JMenuBar;
 
 import docking.action.DockingActionIf;
 import docking.menu.*;
-import ghidra.util.task.SwingUpdateManager;
 
 public class WindowActionManager {
 	private Map<DockingActionIf, DockingActionProxy> actionToProxyMap;
@@ -29,25 +28,18 @@ public class WindowActionManager {
 	private ToolBarManager toolBarMgr;
 	private final WindowNode node;
 
-	private final DockingWindowManager winMgr;
 	private boolean disposed;
 
-	private ComponentPlaceholder placeHolderForScheduledActionUpdate;
-	private Runnable updateActionsRunnable = () -> processContextChanged();
-	private SwingUpdateManager updateManager =
-		new SwingUpdateManager(500, 500, "Context Update Manager", updateActionsRunnable);
-
-	public WindowActionManager(WindowNode node, MenuHandler menuBarHandler,
+	WindowActionManager(WindowNode node, MenuHandler menuBarHandler,
 			DockingWindowManager winMgr, MenuGroupMap menuGroupMap) {
 
 		this.node = node;
-		this.winMgr = winMgr;
 		actionToProxyMap = new HashMap<>();
 		menuBarMgr = new MenuBarManager(menuBarHandler, menuGroupMap);
 		toolBarMgr = new ToolBarManager(winMgr);
 	}
 
-	public void setActions(List<DockingActionIf> actionList) {
+	void setActions(List<DockingActionIf> actionList) {
 		menuBarMgr.clearActions();
 		toolBarMgr.clearActions();
 		actionToProxyMap.clear();
@@ -56,7 +48,7 @@ public class WindowActionManager {
 		}
 	}
 
-	public void addAction(DockingActionIf action) {
+	void addAction(DockingActionIf action) {
 		if (action.getMenuBarData() != null || action.getToolBarData() != null) {
 			DockingActionProxy proxyAction = new DockingActionProxy(action);
 			actionToProxyMap.put(action, proxyAction);
@@ -65,7 +57,7 @@ public class WindowActionManager {
 		}
 	}
 
-	public void removeAction(DockingActionIf action) {
+	void removeAction(DockingActionIf action) {
 		DockingActionProxy proxyAction = actionToProxyMap.remove(action);
 		if (proxyAction != null) {
 			menuBarMgr.removeAction(proxyAction);
@@ -73,11 +65,11 @@ public class WindowActionManager {
 		}
 	}
 
-	public DockingActionIf getToolbarAction(String actionName) {
+	DockingActionIf getToolbarAction(String actionName) {
 		return toolBarMgr.getAction(actionName);
 	}
 
-	public void update() {
+	void update() {
 		JMenuBar menuBar = menuBarMgr.getMenuBar();
 		if (menuBar.getMenuCount() > 0) {
 			node.setMenuBar(menuBar);
@@ -87,9 +79,8 @@ public class WindowActionManager {
 		node.validate();
 	}
 
-	public synchronized void dispose() {
+	void dispose() {
 		disposed = true;
-		updateManager.dispose();
 		node.setToolBar(null);
 		node.setMenuBar(null);
 		actionToProxyMap.clear();
@@ -97,48 +88,32 @@ public class WindowActionManager {
 		toolBarMgr.dispose();
 	}
 
-	synchronized void contextChanged(ComponentPlaceholder placeHolder) {
+	void contextChanged(ActionContext globalContext, ActionContext localContext,
+			Set<DockingActionIf> excluded) {
 
-		if (!node.isVisible()) {
+		if (!node.isVisible() || disposed) {
 			return;
 		}
 
-		placeHolderForScheduledActionUpdate = placeHolder;
-
-		// Typically, when we get one contextChanged, we get a flurry of contextChanged calls.
-		// In order to make the action updating be as responsive as possible and still be complete,
-		// we have chosen a policy that will reduce a flurry of contextChanged call into two
-		// actual calls - one that occurs immediately and one when the flurry times out.
-		updateManager.update();
-	}
-
-	private synchronized void processContextChanged() {
-		//
-		// This method is called from an invokeLater(), which means that we may be 
-		// disposed while before this Swing call executes.
-		//
-		if (disposed) {
-			return;
-		}
-
-		ActionContext localContext = getContext();
-		ActionContext globalContext = winMgr.getDefaultToolContext();
-
-		// Update actions - make a copy so that we don't get concurrent modification exceptions
-		List<DockingActionIf> list = new ArrayList<>(actionToProxyMap.values());
+		// Update actions - make a copy so that we don't get concurrent modification
+		// exceptions during reentrant operations
+		List<DockingActionIf> list = new ArrayList<>(actionToProxyMap.keySet());
 		for (DockingActionIf action : list) {
-			if (action.isValidContext(localContext)) {
-				action.setEnabled(action.isEnabledForContext(localContext));
+			if (excluded.contains(action)) {
+				continue;
 			}
-			else if (isValidDefaultToolContext(action, globalContext)) {
-				action.setEnabled(action.isEnabledForContext(globalContext));
+
+			DockingActionIf proxyAction = actionToProxyMap.get(action);
+			if (proxyAction.isValidContext(localContext)) {
+				proxyAction.setEnabled(proxyAction.isEnabledForContext(localContext));
+			}
+			else if (isValidDefaultToolContext(proxyAction, globalContext)) {
+				proxyAction.setEnabled(proxyAction.isEnabledForContext(globalContext));
 			}
 			else {
-				action.setEnabled(false);
+				proxyAction.setEnabled(false);
 			}
 		}
-		// Notify listeners if the context provider is the focused provider
-		winMgr.notifyContextListeners(placeHolderForScheduledActionUpdate, localContext);
 	}
 
 	private boolean isValidDefaultToolContext(DockingActionIf action, ActionContext toolContext) {
@@ -146,15 +121,15 @@ public class WindowActionManager {
 			action.isValidContext(toolContext);
 	}
 
-	private ActionContext getContext() {
-		ComponentProvider provider = placeHolderForScheduledActionUpdate == null ? null
-				: placeHolderForScheduledActionUpdate.getProvider();
-
-		ActionContext context = provider == null ? null : provider.getActionContext(null);
-
-		if (context == null) {
-			context = new ActionContext();
-		}
-		return context;
+	/**
+	 * Returns the set of actions for this window.
+	 * 
+	 * <p>Note this returns the the original passed-in actions and not the proxy actions that the
+	 * window uses.
+	 * 
+	 * @return the set of actions for this window
+	 */
+	Set<DockingActionIf> getOriginalActions() {
+		return new HashSet<>(actionToProxyMap.keySet());
 	}
 }

Ghidra/Framework/Generic/src/main/java/ghidra/util/task/AbstractSwingUpdateManager.java

@@ -55,7 +55,7 @@ public abstract class AbstractSwingUpdateManager {
 	protected static final long NONE = 0;
 	public static final int DEFAULT_MAX_DELAY = 30000;
 	protected static final int MIN_DELAY_FLOOR = 10;
-	protected static final int DEFAULT_MIN_DELAY = 250;
+	public static final int DEFAULT_MIN_DELAY = 250;
 	protected static final String DEFAULT_NAME = AbstractSwingUpdateManager.class.getSimpleName();
 	private static final WeakSet<AbstractSwingUpdateManager> instances =
 		WeakDataStructureFactory.createCopyOnReadWeakSet();