GT-2869 - Shared Key Bindings - created new shared keybinding concept

that replaces the DummyKeyBindingsOptionsAction

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/DataAction.java

@@ -17,51 +17,46 @@ package ghidra.app.plugin.core.data;
 
 import javax.swing.KeyStroke;
 
-import docking.action.*;
-import docking.tool.util.DockingToolConstants;
+import docking.action.KeyBindingData;
+import docking.action.MenuData;
 import ghidra.app.context.ListingActionContext;
 import ghidra.app.context.ListingContextAction;
-import ghidra.framework.options.*;
-import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.data.*;
 import ghidra.util.HelpLocation;
 
 /**
  * Base class for actions to create data types
  */
-class DataAction extends ListingContextAction implements OptionsChangeListener {
+class DataAction extends ListingContextAction {
 
 	protected DataType dataType;
 	protected DataPlugin plugin;
-	private String actionName;
-	private DummyKeyBindingsOptionsAction dummyKeybindingsAction;
 
 	public DataAction(DataType dataType, DataPlugin plugin) {
 		this("Define " + dataType.getDisplayName(), "Data", dataType, plugin);
 	}
 
+	/**
+	 * Constructor
+	 * 
+	 * @param name action name
+	 * @param group the action's group
+	 * @param dataType the data type used by this action
+	 * @param plugin the plugin that owns this action
+	 */
 	public DataAction(String name, String group, DataType dataType, DataPlugin plugin) {
 		super(name, plugin.getName(), false);
-		this.actionName = name;
 		this.plugin = plugin;
 		this.dataType = dataType;
 
 		setPopupMenuData(new MenuData(new String[] { "Data", dataType.getDisplayName() }, group));
 		assignHelpID(dataType);
-
-		initializeKeybinding();
+		initKeyStroke(getDefaultKeyStroke());
 	}
 
-	private void initializeKeybinding() {
-		PluginTool tool = plugin.getTool();
-		dummyKeybindingsAction =
-			new DummyKeyBindingsOptionsAction(actionName, getDefaultKeyStroke());
-		tool.addAction(dummyKeybindingsAction);
-		ToolOptions options = tool.getOptions(DockingToolConstants.KEY_BINDINGS);
-		options.addOptionsChangeListener(this);
-		KeyStroke keyStroke =
-			options.getKeyStroke(dummyKeybindingsAction.getFullName(), getDefaultKeyStroke());
-		initKeyStroke(keyStroke);
+	@Override
+	public boolean usesSharedKeyBinding() {
+		return true;
 	}
 
 	protected KeyStroke getDefaultKeyStroke() {
@@ -77,8 +72,8 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 		setUnvalidatedKeyBindingData(new KeyBindingData(keyStroke));
 	}
 
-	protected DockingAction getDummyKeyBindingAction() {
-		return dummyKeybindingsAction;
+	DataType getDataType() {
+		return dataType;
 	}
 
 	@Override
@@ -99,19 +94,6 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 		return plugin.isCreateDataAllowed(context);
 	}
 
-	@Override
-	public void optionsChanged(ToolOptions options, String optionName, Object oldValue,
-			Object newValue) {
-		KeyStroke keyStroke = (KeyStroke) newValue;
-		if (optionName.startsWith(actionName)) {
-			setUnvalidatedKeyBindingData(new KeyBindingData(keyStroke));
-		}
-	}
-
-	DataType getDataType() {
-		return dataType;
-	}
-
 	// Set the help ID according to the data type
 	private void assignHelpID(DataType dt) {
 		String helpID = "Favorites";

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/DataAction.java

@@ -17,12 +17,10 @@ package ghidra.app.plugin.core.function;
 
 import javax.swing.KeyStroke;
 
-import docking.action.*;
-import docking.tool.util.DockingToolConstants;
+import docking.action.KeyBindingData;
+import docking.action.MenuData;
 import ghidra.app.context.ListingActionContext;
 import ghidra.app.context.ListingContextAction;
-import ghidra.framework.options.*;
-import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.data.DataType;
 import ghidra.program.util.ProgramLocation;
 import ghidra.program.util.VariableLocation;
@@ -31,13 +29,11 @@ import ghidra.util.HelpLocation;
 /**
  * Base class for actions to create data types
  */
-class DataAction extends ListingContextAction implements OptionsChangeListener {
+class DataAction extends ListingContextAction {
 
 	private final String group;
 	protected DataType dataType;
 	protected FunctionPlugin plugin;
-	private String actionName;
-	private DummyKeyBindingsOptionsAction dummyKeybindingsAction;
 
 	public DataAction(DataType dataType, FunctionPlugin plugin) {
 		this("Define " + dataType.getDisplayName(), "Function", dataType, plugin);
@@ -46,26 +42,19 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 
 	public DataAction(String name, String group, DataType dataType, FunctionPlugin plugin) {
 		super(name, plugin.getName(), false);
-		this.actionName = name;
 		this.group = group;
 		this.plugin = plugin;
 		this.dataType = dataType;
 
 		setPopupMenu(plugin.getDataActionMenuName(null), true);
 		setHelpLocation(new HelpLocation(plugin.getName(), "DataType"));
-		initializeKeybinding();
+
+		initKeyStroke(getDefaultKeyStroke());
 	}
 
-	private void initializeKeybinding() {
-		PluginTool tool = plugin.getTool();
-		dummyKeybindingsAction =
-			new DummyKeyBindingsOptionsAction(actionName, getDefaultKeyStroke());
-		tool.addAction(dummyKeybindingsAction);
-		ToolOptions options = tool.getOptions(DockingToolConstants.KEY_BINDINGS);
-		options.addOptionsChangeListener(this);
-		KeyStroke keyStroke =
-			options.getKeyStroke(dummyKeybindingsAction.getFullName(), getDefaultKeyStroke());
-		initKeyStroke(keyStroke);
+	@Override
+	public boolean usesSharedKeyBinding() {
+		return true;
 	}
 
 	protected KeyStroke getDefaultKeyStroke() {
@@ -81,10 +70,6 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 		setUnvalidatedKeyBindingData(new KeyBindingData(keyStroke));
 	}
 
-	protected DockingAction getDummyKeyBindingAction() {
-		return dummyKeybindingsAction;
-	}
-
 	void setPopupMenu(String name, boolean isSignatureAction) {
 		setPopupMenuData(new MenuData(
 			new String[] { FunctionPlugin.SET_DATA_TYPE_PULLRIGHT, dataType.getDisplayName() },
@@ -98,11 +83,6 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 		super.dispose();
 	}
 
-	@Override
-	public void actionPerformed(ListingActionContext context) {
-		plugin.createData(dataType, context, true);
-	}
-
 	@Override
 	protected boolean isEnabledForContext(ListingActionContext context) {
 		if (context.hasSelection() || context.getAddress() == null) {
@@ -121,11 +101,7 @@ class DataAction extends ListingContextAction implements OptionsChangeListener {
 	}
 
 	@Override
-	public void optionsChanged(ToolOptions options, String optionName, Object oldValue,
-			Object newValue) {
-		KeyStroke keyStroke = (KeyStroke) newValue;
-		if (optionName.startsWith(actionName)) {
-			setUnvalidatedKeyBindingData(new KeyBindingData(keyStroke));
-		}
+	public void actionPerformed(ListingActionContext context) {
+		plugin.createData(dataType, context, true);
 	}
 }

Ghidra/Features/Base/src/main/java/ghidra/test/TestEnv.java

@@ -225,6 +225,10 @@ public class TestEnv {
 	}
 
 	public void closeTool(PluginTool toolToClose, boolean ignoreChanges) {
+		if (toolToClose == tool) {
+			tool = null;
+		}
+
 		extraTools.remove(toolToClose);
 		AbstractGenericTest.executeOnSwingWithoutBlocking(() -> {
 			if (ignoreChanges) {

Ghidra/Features/Base/src/test.slow/java/ghidra/framework/plugintool/dialog/KeyBindingUtilsTest.java

@@ -32,6 +32,7 @@ import org.junit.*;
 import docking.action.DockingActionIf;
 import docking.options.editor.OptionsDialog;
 import docking.options.editor.OptionsPanel;
+import docking.tool.util.DockingToolConstants;
 import docking.util.KeyBindingUtils;
 import docking.widgets.filechooser.GhidraFileChooser;
 import docking.widgets.tree.GTree;
@@ -49,7 +50,6 @@ import ghidra.framework.options.Options;
 import ghidra.framework.options.ToolOptions;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.framework.plugintool.mgr.OptionsManager;
-import ghidra.framework.plugintool.util.ToolConstants;
 import ghidra.test.AbstractGhidraHeadedIntegrationTest;
 import ghidra.test.TestEnv;
 import ghidra.util.Msg;
@@ -65,7 +65,6 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 
 	private static final String TEST_FILENAME =
 		"KeyBindingUtilsTest_Test_Filename" + KeyBindingUtils.PREFERENCES_FILE_EXTENSION;
-	private static final String TEST_TOOL_NAME = "KeyBindingsUtilsTest_TestTool";
 
 	private Writer debug = new NullWriter();
 
@@ -137,7 +136,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 	@Test
 	public void testExportImportKeyBindings() throws Exception {
 		debug("testExportImportKeyBindings()");
-		ToolOptions defaultKeyBindings = tool.getOptions(ToolConstants.KEY_BINDINGS);
+		ToolOptions defaultKeyBindings = tool.getOptions(DockingToolConstants.KEY_BINDINGS);
 
 		debug("a");
 
@@ -195,7 +194,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 	@Test
 	public void testImportExportWithGUI() throws Exception {
 
-		setUpDialog();
+		setKeyBindingsUpDialog();
 
 		debug("a");
 
@@ -206,11 +205,11 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 
 		// save and reload them to make sure they are the same
 		File saveFile = exportOptions(toolKeyBindingOptions);
-		ToolOptions savedOptions = importOptions(saveFile);
+		ToolOptions originalOptions = importOptions(saveFile);
 
 		assertOptionsMatch(
 			"The Options objects do not contain different data after " + "changes have been made.",
-			toolKeyBindingOptions, savedOptions);
+			toolKeyBindingOptions, originalOptions);
 
 		debug("c");
 
@@ -222,7 +221,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 		// verify the changes are different than the original values
 		assertOptionsDontMatch(
 			"The Options objects do not contain different data after " + "changes have been made.",
-			toolKeyBindingOptions, savedOptions);
+			toolKeyBindingOptions, originalOptions);
 
 		debug("e");
 
@@ -234,13 +233,13 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 		debug("f");
 
 		// verify the data is the same as it was before the changes
-		boolean same = compareOptionsWithKeyStrokeMap(savedOptions, optionsMap);
+		boolean same = compareOptionsWithKeyStrokeMap(originalOptions, optionsMap);
 		assertTrue("The Options object contains different data than was " + "imported.", same);
 
 		debug("g");
 
 		// close the tool *without* applying the changes
-		closeAllWindowsAndFrames();
+		closeAllWindows();
 		env.dispose();
 
 		debug("h");
@@ -250,7 +249,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 		// reload the tool and make sure the values are those of the changes
 		// *before* the last import
 		setUp();
-		setUpDialog();
+		setKeyBindingsUpDialog();
 
 		debug("i");
 
@@ -258,7 +257,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 		assertOptionsMatch(
 			"The options from the first tool instance have changed " +
 				"in the second tool instance even though the testing changes were not applied.",
-			savedOptions, newlyLoadedDefaultOptions);
+			originalOptions, newlyLoadedDefaultOptions);
 
 		debug("j");
 
@@ -281,44 +280,54 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 
 		debug("n");
 
-		saveTool();
-		closeAllWindowsAndFrames();
-		env.dispose();
-
-		debug("o");
-
 		saveFile.delete();
 
 		// reload the tool and make sure the values are those of the changes
 		// *after* the last import
 		// reload with our saved tool
-		setUp();
-		runSwing(() -> {
-			ToolServices services = tool.getProject().getToolServices();
-			tool = (PluginTool) services.launchTool(TEST_TOOL_NAME, null);
-		});
+		saveAndCloseTool();
+		reopenTool(tool);
 
 		debug("p");
 
+		setKeyBindingsUpDialog(tool);
+
 		newlyLoadedDefaultOptions = (ToolOptions) getInstanceField("options", panel);
 		assertOptionsDontMatch(
-			"The options are the same after making changes, applying, " + "closing and reloading.",
-			savedOptions, newlyLoadedDefaultOptions);
+			"The options are the same after making changes, applying, closing and reloading.",
+			originalOptions, newlyLoadedDefaultOptions);
 
 		debug("q");
+		closeAllWindows();
 	}
 
-	private void saveTool() {
-		executeOnSwingWithoutBlocking(() -> tool.saveTool());
+	private void reopenTool(PluginTool tool2) {
+		runSwing(() -> {
+			ToolServices services = tool.getProject().getToolServices();
+			tool = (PluginTool) services.launchTool(tool.getName(), null);
+		});
+		assertNotNull(tool);
 	}
 
-	// open the options dialog and show the key bindings editor
-	private void setUpDialog() throws Exception {
-		debug("setUpDialog()");
+	private void saveAndCloseTool() {
+		runSwing(() -> {
+			ToolServices services = tool.getProject().getToolServices();
+			services.saveTool(tool);
+		});
+		env.closeTool(tool);
+	}
+
+	private void setKeyBindingsUpDialog() throws Exception {
 		env.showTool();
+		setKeyBindingsUpDialog(tool);
+	}
+
+	private void setKeyBindingsUpDialog(PluginTool pluginTool) throws Exception {
+		debug("setUpDialog()");
 		debug("aa");
 
-		final OptionsManager optionsManager = (OptionsManager) getInstanceField("optionsMgr", tool);
+		final OptionsManager optionsManager =
+			(OptionsManager) getInstanceField("optionsMgr", pluginTool);
 
 		debug("bb");
 		executeOnSwingWithoutBlocking(() -> {
@@ -458,7 +467,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	private void closeWarningDialog(boolean proceed) {
-		Window window = waitForWindowByTitleContaining(null, "Continue", DEFAULT_WINDOW_TIMEOUT);
+		Window window = waitForWindowByTitleContaining("Continue");
 		assertNotNull(window);
 
 		String button = proceed ? "Yes" : "No";
@@ -488,7 +497,7 @@ public class KeyBindingUtilsTest extends AbstractGhidraHeadedIntegrationTest {
 	// locates the open file chooser and verifies its state
 	private File findAndTestFileChooser(File path, String filename) throws Exception {
 		// get the file chooser and set the file it will use
-		GhidraFileChooser fileChooser = waitForDialogComponent(null, GhidraFileChooser.class, 5000);
+		GhidraFileChooser fileChooser = waitForDialogComponent(GhidraFileChooser.class);
 		if (fileChooser == null) {
 			Msg.debug(this, "Couldn't find file chooser");
 			printOpenWindows();

Ghidra/Framework/Docking/src/main/java/docking/actions/DockingToolActionManager.java

@@ -93,12 +93,12 @@ public class DockingToolActionManager implements PropertyChangeListener {
 
 	private void setKeyBindingOption(DockingActionIf action) {
 
-		if (!action.isKeyBindingManaged()) {
+		if (action.usesSharedKeyBinding()) {
+			installSharedKeyBinding(action);
 			return;
 		}
 
-		if (action.usesSharedKeyBinding()) {
-			installSharedKeyBinding(action);
+		if (!action.isKeyBindingManaged()) {
 			return;
 		}
 
@@ -159,15 +159,7 @@ public class DockingToolActionManager implements PropertyChangeListener {
 
 		action.addPropertyChangeListener(this);
 		addActionToMap(action);
-		if (action.isKeyBindingManaged()) {
-			KeyStroke ks = action.getKeyBinding();
-			keyBindingOptions.registerOption(action.getFullName(), OptionType.KEYSTROKE_TYPE, ks,
-				null, null);
-			KeyStroke newKs = keyBindingOptions.getKeyStroke(action.getFullName(), ks);
-			if (ks != newKs) {
-				action.setUnvalidatedKeyBindingData(new KeyBindingData(newKs));
-			}
-		}
+		setKeyBindingOption(action);
 		winMgr.addLocalAction(provider, action);
 	}
 
@@ -198,14 +190,26 @@ public class DockingToolActionManager implements PropertyChangeListener {
 	/**
 	 * Get all actions that have the action name which includes the action owner's name.
 	 * 
-	 * @param fullActionName full name for the action, e.g., "My Action (My Plugin)"
+	 * @param fullName full name for the action, e.g., "My Action (My Plugin)"
 	 * @return list of actions; empty if no action exists with the given name
 	 */
 	public List<DockingActionIf> getDockingActionsByFullActionName(String fullName) {
 		List<DockingActionIf> list = actionMap.get(fullName);
 		if (list == null) {
-			return new ArrayList<>();
+			list = new ArrayList<>();
 		}
+
+		// note: we only use the action name for the lookup, as all shared actions are owned
+		//       by the 'Tool'
+		int index = fullName.indexOf(" (Tool)");
+		if (index > 0) {
+			String actionName = fullName.substring(0, index);
+			SharedStubKeyBindingAction sharedAction = sharedActionMap.get(actionName);
+			if (sharedAction != null) {
+				list.add(sharedAction);
+			}
+		}
+
 		return new ArrayList<>(list);
 	}
 
@@ -222,8 +226,8 @@ public class DockingToolActionManager implements PropertyChangeListener {
 	 * @return a list of deduped actions.
 	 */
 	private List<DockingActionIf> getUniqueActionList(String owner) {
-		List<DockingActionIf> matchingActionList = new ArrayList<>();
 
+		List<DockingActionIf> matchingActionList = new ArrayList<>();
 		for (List<DockingActionIf> actionList : actionMap.values()) {
 			// we only want *one* instance of duplicate actions
 			DockingActionIf action = actionList.get(0);
@@ -232,6 +236,12 @@ public class DockingToolActionManager implements PropertyChangeListener {
 			}
 		}
 
+		for (DockingActionIf action : sharedActionMap.values()) {
+			if (owner == null || action.getOwner().equals(owner)) {
+				matchingActionList.add(action);
+			}
+		}
+
 		return matchingActionList;
 	}
 
@@ -289,7 +299,6 @@ public class DockingToolActionManager implements PropertyChangeListener {
 				actionMap.remove(name);
 			}
 		}
-
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/actions/SharedStubKeyBindingAction.java

@@ -83,7 +83,7 @@ class SharedStubKeyBindingAction extends DockingAction implements OptionsChangeL
 	private KeyStroke validateActionsHaveTheSameDefaultKeyStroke(DockingActionIf newAction) {
 
 		// this value may be null
-		KeyBindingData defaultBinding = newAction.getDefaultKeyBindingData();
+		KeyBindingData defaultBinding = newAction.getKeyBindingData();
 		KeyStroke newDefaultKs = getKeyStroke(defaultBinding);
 
 		Set<Entry<DockingActionIf, KeyStroke>> entries = clientActions.entrySet();

Ghidra/Framework/Docking/src/test.slow/java/docking/actions/SharedKeybindingDockingActionTest.java

@@ -21,6 +21,7 @@ import java.awt.event.KeyEvent;
 import java.util.List;
 import java.util.Set;
 
+import javax.swing.JComponent;
 import javax.swing.KeyStroke;
 
 import org.apache.commons.collections4.IterableUtils;
@@ -72,6 +73,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertNoLoggedMessages();
 		assertKeyBinding(action1, DEFAULT_KS_1);
 		assertKeyBinding(action2, DEFAULT_KS_1);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -89,6 +91,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertNoLoggedMessages();
 		assertKeyBinding(action1, newKs);
 		assertKeyBinding(action2, newKs);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -104,6 +107,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertImproperDefaultBindingMessage();
 		assertKeyBinding(action1, DEFAULT_KS_1);
 		assertKeyBinding(action2, DEFAULT_KS_1);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -119,6 +123,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertNoLoggedMessages();
 		assertKeyBinding(action1, null);
 		assertKeyBinding(action2, null);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -133,6 +138,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertImproperDefaultBindingMessage();
 		assertKeyBinding(action1, DEFAULT_KS_1);
 		assertKeyBinding(action2, DEFAULT_KS_1);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -168,6 +174,7 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 
 		assertNoLoggedMessages();
 		assertKeyBinding(action1, DEFAULT_KS_1);
+		assertSharedStubInTool();
 	}
 
 	@Test
@@ -204,14 +211,81 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 		assertKeyBinding(action1, newKs);
 	}
 
+	@Test
+	public void testSharedKeyBinding_SameDefaultKeyBindings_LocalAction() {
+
+		TestAction action1 = new TestAction(OWNER_1, DEFAULT_KS_1);
+		TestAction action2 = new TestAction(OWNER_2, DEFAULT_KS_1);
+
+		DummyComponentProvider provider = new DummyComponentProvider();
+		tool.addLocalAction(provider, action1);
+		tool.addLocalAction(provider, action2);
+
+		assertNoLoggedMessages();
+		assertKeyBinding(action1, DEFAULT_KS_1);
+		assertKeyBinding(action2, DEFAULT_KS_1);
+		assertSharedStubInTool();
+	}
+
+	@Test
+	public void testSharedKeyBinding_RemoveAction_LocalAction() {
+
+		TestAction action1 = new TestAction(OWNER_1, DEFAULT_KS_1);
+		TestAction action2 = new TestAction(OWNER_2, DEFAULT_KS_1);
+
+		DummyComponentProvider provider = new DummyComponentProvider();
+		tool.addLocalAction(provider, action1);
+		tool.addLocalAction(provider, action2);
+
+		tool.removeLocalAction(provider, action1);
+
+		assertActionNotInTool(action1);
+		assertActionInTool(action2);
+
+		tool.removeLocalAction(provider, action2);
+		assertActionNotInTool(action2);
+
+		String sharedName = action1.getFullName();
+		assertNoSharedKeyBindingStubInstalled(sharedName);
+	}
+
+	@Test
+	public void testSharedKeyBinding_RemoveComonentActions() {
+
+		TestAction action1 = new TestAction(OWNER_1, DEFAULT_KS_1);
+		TestAction action2 = new TestAction(OWNER_2, DEFAULT_KS_1);
+
+		DummyComponentProvider provider = new DummyComponentProvider();
+		tool.addLocalAction(provider, action1);
+		tool.addLocalAction(provider, action2);
+		assertActionInTool(action1);
+		assertActionInTool(action2);
+
+		tool.removeComponentProvider(provider);
+
+		assertActionNotInTool(action1);
+		assertActionNotInTool(action2);
+
+		String sharedName = action1.getFullName();
+		assertNoSharedKeyBindingStubInstalled(sharedName);
+	}
+
 //==================================================================================================
 // Private Methods
 //==================================================================================================
 
+	private void assertSharedStubInTool() {
+		// the stub action's name is 'Shared Action Name (Tool)'
+		DockingActionIf action = getAction(tool, SHARED_OWNER, SHARED_NAME);
+		assertNotNull("Shared action stub is not in the tool", action);
+	}
+
 	private void assertOnlyOneVersionOfActionInTool(TestAction action) {
-		Set<DockingActionIf> actions = getActions(tool, action.getName());
-		assertEquals("There should be only one instance of this action in the tool: " + action, 1,
-			actions.size());
+
+		// this  method will fail if more than one action is registered
+		DockingActionIf registeredAction = getAction(tool, action.getOwner(), action.getName());
+		assertNotNull("There should be only one instance of this action in the tool: " + action,
+			registeredAction);
 	}
 
 	private void assertActionInTool(TestAction action) {
@@ -234,7 +308,6 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 	}
 
 	private void assertNoSharedKeyBindingStubInstalled(String sharedName) {
-
 		List<DockingActionIf> actions = tool.getDockingActionsByFullActionName(sharedName);
 		assertTrue("There should be no actions registered for '" + sharedName + "'",
 			actions.isEmpty());
@@ -286,4 +359,16 @@ public class SharedKeybindingDockingActionTest extends AbstractDockingTest {
 			fail("Action performed should not have been called");
 		}
 	}
+
+	private class DummyComponentProvider extends ComponentProvider {
+		public DummyComponentProvider() {
+			super(tool, "Dummy", "Dummy Owner");
+			addToTool();
+		}
+
+		@Override
+		public JComponent getComponent() {
+			return null;
+		}
+	}
 }

Ghidra/Framework/Project/src/main/java/ghidra/framework/project/tool/ToolServicesImpl.java

@@ -15,6 +15,12 @@
  */
 package ghidra.framework.project.tool;
 
+import java.io.*;
+import java.util.*;
+
+import org.jdom.Document;
+import org.jdom.output.XMLOutputter;
+
 import ghidra.framework.ToolUtils;
 import ghidra.framework.data.ContentHandler;
 import ghidra.framework.data.DomainObjectAdapter;
@@ -26,12 +32,6 @@ import ghidra.util.Msg;
 import ghidra.util.classfinder.ClassSearcher;
 import ghidra.util.xml.GenericXMLOutputter;
 
-import java.io.*;
-import java.util.*;
-
-import org.jdom.Document;
-import org.jdom.output.XMLOutputter;
-
 /**
  * Implementation of service used to manipulate tools.
  */
@@ -42,7 +42,7 @@ class ToolServicesImpl implements ToolServices {
 
 	private ToolChest toolChest;
 	private ToolManagerImpl toolManager;
-	private List<DefaultToolChangeListener> listeners = new ArrayList<DefaultToolChangeListener>();
+	private List<DefaultToolChangeListener> listeners = new ArrayList<>();
 	private ToolChestChangeListener toolChestChangeListener;
 	private Set<ContentHandler> contentHandlers;
 
@@ -189,7 +189,7 @@ class ToolServicesImpl implements ToolServices {
 
 	@Override
 	public Set<ToolAssociationInfo> getContentTypeToolAssociations() {
-		Set<ToolAssociationInfo> set = new HashSet<ToolAssociationInfo>();
+		Set<ToolAssociationInfo> set = new HashSet<>();
 
 		// get all known content types
 		Set<ContentHandler> handlers = getContentHandlers();
@@ -232,7 +232,7 @@ class ToolServicesImpl implements ToolServices {
 
 	@Override
 	public Set<ToolTemplate> getCompatibleTools(Class<? extends DomainObject> domainClass) {
-		Map<String, ToolTemplate> nameToTemplateMap = new HashMap<String, ToolTemplate>();
+		Map<String, ToolTemplate> nameToTemplateMap = new HashMap<>();
 
 		//
 		// First, get all compatible tools in the tool chest
@@ -290,12 +290,12 @@ class ToolServicesImpl implements ToolServices {
 			}
 		}
 
-		return new HashSet<ToolTemplate>(nameToTemplateMap.values());
+		return new HashSet<>(nameToTemplateMap.values());
 	}
 
 	private Set<ContentHandler> getCompatibleContentHandlers(
 			Class<? extends DomainObject> domainClass) {
-		Set<ContentHandler> set = new HashSet<ContentHandler>();
+		Set<ContentHandler> set = new HashSet<>();
 		Set<ContentHandler> handlers = getContentHandlers();
 		for (ContentHandler contentHandler : handlers) {
 			Class<? extends DomainObject> handlerDomainClass =
@@ -327,7 +327,7 @@ class ToolServicesImpl implements ToolServices {
 			return contentHandlers;
 		}
 
-		contentHandlers = new HashSet<ContentHandler>();
+		contentHandlers = new HashSet<>();
 		Set<ContentHandler> instances = ClassSearcher.getInstances(ContentHandler.class);
 		for (ContentHandler contentHandler : instances) {
 			// a bit of validation
@@ -392,7 +392,7 @@ class ToolServicesImpl implements ToolServices {
 	private Tool[] getSameNamedRunningTools(Tool tool) {
 		String toolName = tool.getToolName();
 		Tool[] tools = toolManager.getRunningTools();
-		List<Tool> toolList = new ArrayList<Tool>(tools.length);
+		List<Tool> toolList = new ArrayList<>(tools.length);
 		for (Tool element : tools) {
 			if (toolName.equals(element.getToolName())) {
 				toolList.add(element);