Merge remote-tracking branch

'origin/GP-5572-dragonmacher-decompiler-action-dialog--SQUASHED'
(Closes #7893)

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/DecompilerActionContext.java

@@ -15,10 +15,6 @@
  */
 package ghidra.app.plugin.core.decompile;
 
-import java.util.function.Supplier;
-
-import docking.ActionContext;
-import docking.action.DockingActionIf;
 import ghidra.app.context.NavigatableActionContext;
 import ghidra.app.context.RestrictedAddressSetContext;
 import ghidra.app.decompiler.*;
@@ -28,9 +24,7 @@ import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.pcode.HighFunction;
-import ghidra.util.Msg;
 import ghidra.util.UndefinedFunction;
-import utility.function.Callback;
 
 public class DecompilerActionContext extends NavigatableActionContext
 		implements RestrictedAddressSetContext {
@@ -168,60 +162,4 @@ public class DecompilerActionContext extends NavigatableActionContext
 
 		return null;
 	}
-
-	/**
-	 * The companion method of {@link #checkActionEnablement(Supplier)}.
-	 * 
-	 * <p>
-	 * Decompiler actions must call this method from their
-	 * {@link DockingActionIf#actionPerformed(ActionContext)} if they require state from the
-	 * Decompiler.
-	 * 
-	 * @param actionCallback the action's code to execute
-	 */
-	public void performAction(Callback actionCallback) {
-
-		if (isDecompiling) {
-			Msg.showInfo(getClass(), getComponentProvider().getComponent(),
-				"Decompiler Action Blocked",
-				"You cannot perform Decompiler actions while the Decompiler is busy");
-			return;
-		}
-
-		actionCallback.call();
-	}
-
-	/**
-	 * The companion method of {@link #performAction(Callback)}.
-	 * 
-	 * <p>
-	 * Decompiler actions must call this method from their
-	 * {@link DockingActionIf#isEnabledForContext(ActionContext)} if they require state from the
-	 * Decompiler.
-	 * 
-	 * @param actionBooleanSupplier the action's code to verify its enablement
-	 * @return true if the action should be considered enabled
-	 */
-	public boolean checkActionEnablement(Supplier<Boolean> actionBooleanSupplier) {
-
-		//
-		// Unusual Code: actions will call this method when their 'isEnabledForContext()' is 
-		//               called.  If the decompiler is still working, we return true here so
-		//               the action is considered enabled.  This allows any key bindings registered
-		//               for the action to get consumed.  If we did not returned false when
-		//               the decompiler was still working, then the key binding would not match and
-		//               the system would pass the key binding up to the global action system,
-		//               which we do not want.
-		//
-		//               Each action that needs state from the decompiler must call this method   
-		//               from 'isEnabledForContext()'.  Also, each action must call 
-		//               'performAction()' on this class, which will skip the action's work and
-		//               show an message if the decompiler is busy.
-		//
-		if (isDecompiling()) {
-			return true;
-		}
-
-		return actionBooleanSupplier.get();
-	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java

@@ -62,17 +62,16 @@ public abstract class AbstractDecompilerAction extends DockingAction {
 	@Override
 	public boolean isEnabledForContext(ActionContext context) {
 		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		return decompilerContext.checkActionEnablement(() -> {
-			return isEnabledForDecompilerContext(decompilerContext);
-		});
+		if (decompilerContext.isDecompiling()) {
+			return false;
+		}
+		return isEnabledForDecompilerContext(decompilerContext);
 	}
 
 	@Override
 	public void actionPerformed(ActionContext context) {
 		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		decompilerContext.performAction(() -> {
-			decompilerActionPerformed(decompilerContext);
-		});
+		decompilerActionPerformed(decompilerContext);
 	}
 
 	/**

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/DecompilerStructureVariableAction.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -40,37 +40,32 @@ public class DecompilerStructureVariableAction extends CreateStructureVariableAc
 			return false;
 		}
 
-		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		return decompilerContext.checkActionEnablement(() -> {
+		Function function = controller.getFunction();
+		if (function == null || function instanceof UndefinedFunction) {
+			return false;
+		}
 
-			Function function = controller.getFunction();
-			if (function == null || function instanceof UndefinedFunction) {
-				return false;
-			}
+		DataType dt = null;
+		boolean isThisParam = false;
 
-			DataType dt = null;
-			boolean isThisParam = false;
+		// get the data type at the location and see if it is OK
+		DecompilerPanel decompilerPanel = controller.getDecompilerPanel();
+		ClangToken tokenAtCursor = decompilerPanel.getTokenAtCursor();
+		if (tokenAtCursor == null) {
+			return false;
+		}
+		int maxPointerSize = controller.getProgram().getDefaultPointerSize();
+		HighVariable var = tokenAtCursor.getHighVariable();
+		if (var != null && !(var instanceof HighConstant)) {
+			dt = var.getDataType();
+			isThisParam = DecompilerUtils.isThisParameter(var, function);
+		}
 
-			// get the data type at the location and see if it is OK
-			DecompilerPanel decompilerPanel = controller.getDecompilerPanel();
-			ClangToken tokenAtCursor = decompilerPanel.getTokenAtCursor();
-			if (tokenAtCursor == null) {
-				return false;
-			}
-			int maxPointerSize = controller.getProgram().getDefaultPointerSize();
-			HighVariable var = tokenAtCursor.getHighVariable();
-			if (var != null && !(var instanceof HighConstant)) {
-				dt = var.getDataType();
-				isThisParam = DecompilerUtils.isThisParameter(var, function);
-			}
+		if (dt == null || dt.getLength() > maxPointerSize) {
+			return false;
+		}
 
-			if (dt == null || dt.getLength() > maxPointerSize) {
-				return false;
-			}
-
-			adjustCreateStructureMenuText(dt, isThisParam);
-			return true;
-
-		});
+		adjustCreateStructureMenuText(dt, isThisParam);
+		return true;
 	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToAddressAction.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -54,18 +54,8 @@ public class FindReferencesToAddressAction extends AbstractFindReferencesToAddre
 		}
 
 		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		return decompilerContext.checkActionEnablement(() -> {
-			updateMenuName(decompilerContext.getAddress());
-			return super.isEnabledForContext(context);
-		});
-	}
-
-	@Override
-	public void actionPerformed(ActionContext context) {
-		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		decompilerContext.performAction(() -> {
-			super.actionPerformed(context);
-		});
+		updateMenuName(decompilerContext.getAddress());
+		return super.isEnabledForContext(context);
 	}
 
 	private void updateMenuName(Address addr) {

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToDataTypeAction.java

@@ -82,23 +82,9 @@ public class FindReferencesToDataTypeAction extends AbstractFindReferencesDataTy
 			return false;
 		}
 
-		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		return decompilerContext.checkActionEnablement(() -> {
-
-			DataType dataType = getDataType(context);
-			updateMenuName(dataType);
-
-			return super.isEnabledForContext(context);
-		});
-	}
-
-	@Override
-	public void actionPerformed(ActionContext context) {
-
-		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		decompilerContext.performAction(() -> {
-			super.actionPerformed(context);
-		});
+		DataType dataType = getDataType(context);
+		updateMenuName(dataType);
+		return super.isEnabledForContext(context);
 	}
 
 	private void updateMenuName(DataType type) {

Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/decompiler/taint/actions/TaintAbstractDecompilerAction.java

@@ -130,17 +130,13 @@ public abstract class TaintAbstractDecompilerAction extends DockingAction {
 	@Override
 	public boolean isEnabledForContext(ActionContext context) {
 		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		return decompilerContext.checkActionEnablement(() -> {
-			return isEnabledForDecompilerContext(decompilerContext);
-		});
+		return isEnabledForDecompilerContext(decompilerContext);
 	}
 
 	@Override
 	public void actionPerformed(ActionContext context) {
 		DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
-		decompilerContext.performAction(() -> {
-			decompilerActionPerformed(decompilerContext);
-		});
+		decompilerActionPerformed(decompilerContext);
 	}
 
 	protected Symbol getSymbol(DecompilerActionContext context) {

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

@@ -15,6 +15,7 @@
  */
 package docking;
 
+import java.awt.Toolkit;
 import java.awt.event.ActionEvent;
 import java.util.List;
 
@@ -46,6 +47,14 @@ public abstract class DockingKeyBindingAction extends AbstractAction {
 		return true; // always enable; this is a internal action that cannot be disabled
 	}
 
+	public void reportNotEnabled() {
+		String name = dockingAction.getName();
+		String ksText = KeyBindingUtils.parseKeyStroke(keyStroke);
+		String message = "Action '%s' (%s) not currently enabled".formatted(name, ksText);
+		tool.setStatusInfo(message, true);
+		Toolkit.getDefaultToolkit().beep();
+	}
+
 	public abstract KeyBindingPrecedence getKeyBindingPrecedence();
 
 	public boolean isSystemKeybindingPrecedence() {

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

@@ -144,12 +144,14 @@ public class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher
 
 		KeyBindingPrecedence keyBindingPrecedence = getValidKeyBindingPrecedence(action);
 		if (keyBindingPrecedence == null) {
-			// Odd Code: we use the precedence as a signal to say that, when it is null, there
-			//           are no valid bindings to be processed.  We used to have a isValidContext()
-			//           method on the action, but it repeated the involved work that is done
-			//           in getKeyBindingPrecedence(), which is needed below.  So, just use the
-			//           one call for both purposes.
-			return false;
+			// Note: we used to return false here.  Returning false allows Java to handle a given 
+			//       key stroke when our actions are disabled. We have decided it is simpler to 
+			//       always consume a given key stroke when we have actions registered.  This 
+			//       prevents inconsistent action firing between Ghidra and Java, depending upon 
+			//       Ghidra's action enablement.   If we find a case that is broken by this change, 
+			//       then we will need a more robust solution here.
+			action.reportNotEnabled();
+			return true;
 		}
 
 		// Process the key event in precedence order.

Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java

@@ -15,10 +15,10 @@
  */
 package docking.action;
 
-import java.awt.Component;
-import java.awt.Window;
+import java.awt.*;
 import java.awt.event.ActionEvent;
 import java.util.*;
+import java.util.List;
 
 import javax.swing.*;
 
@@ -154,6 +154,7 @@ public class MultipleKeyAction extends DockingKeyBindingAction {
 		else {
 			String name = (String) getValue(Action.NAME);
 			tool.setStatusInfo("Action (" + name + ") not valid in this context!", true);
+			Toolkit.getDefaultToolkit().beep();
 		}
 	}
 
@@ -216,28 +217,29 @@ public class MultipleKeyAction extends DockingKeyBindingAction {
 		// 3) Check for default context actions
 		//
 		for (ActionData actionData : actions) {
-			if (actionData.isGlobalAction()) {
-				// When looking for context matches, we prefer local context, even though this
-				// is a 'global' action.  This allows more specific context to be used when
-				// available
-				if (isValidAndEnabled(actionData, localContext)) {
-					list.add(new ExecutableAction(actionData.action, localContext));
-					continue;
-				}
+			if (!actionData.isGlobalAction()) {
+				continue;
+			}
 
-				// this happens if we are in a dialog, default context is not used
-				if (contextMap == null) {
-					continue;
-				}
+			// When looking for context matches, we prefer local context, even though this
+			// is a 'global' action.  This allows more specific context to be used when available
+			if (isValidAndEnabled(actionData, localContext)) {
+				list.add(new ExecutableAction(actionData.action, localContext));
+				continue;
+			}
 
-				if (!actionData.supportsDefaultContext()) {
-					continue;
-				}
+			// this happens if we are in a dialog, default context is not used
+			if (contextMap == null) {
+				continue;
+			}
 
-				ActionContext defaultContext = contextMap.get(actionData.getContextType());
-				if (isValidAndEnabled(actionData, defaultContext)) {
-					list.add(new ExecutableAction(actionData.action, defaultContext));
-				}
+			if (!actionData.supportsDefaultContext()) {
+				continue;
+			}
+
+			ActionContext defaultContext = contextMap.get(actionData.getContextType());
+			if (isValidAndEnabled(actionData, defaultContext)) {
+				list.add(new ExecutableAction(actionData.action, defaultContext));
 			}
 		}
 		return list;