Merge remote-tracking branch 'origin/GP-2371-dragonmacher-tool-restore-exception--SQUASHED' into patch

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

@@ -35,6 +35,12 @@ import utilities.util.reflection.ReflectionUtilities;
  * Class to hold information about a dockable component with respect to its position within the
  * windowing system.  It also holds identification information about the provider so that its
  * location can be reused when the provider is re-opened.
+ * <p>
+ * The placeholder will be used to link previously saved position information.  The tool will
+ * initially construct plugins and their component providers with default position information.
+ * Then, any existing xml data will be restored, which may have provider position information.
+ * The restoring of the xml will create placeholders with this saved information.  Finally, the
+ * restored placeholders will be linked with existing component providers.
  */
 public class ComponentPlaceholder {
 	private String name;
@@ -68,7 +74,7 @@ public class ComponentPlaceholder {
 	 * @param name the name of the component
 	 * @param owner the owner of the component
 	 * @param group the window group
-	 * @param title the title 
+	 * @param title the title
 	 * @param show whether or not the component is showing
 	 * @param node componentNode that has this placeholder
 	 * @param instanceID the instance ID
@@ -116,7 +122,7 @@ public class ComponentPlaceholder {
 		if (node != null && disposed) {
 			//
 			// TODO Hack Alert!  (When this is removed, also update ComponentNode)
-			// 
+			//
 			// This should not happen!  We have seen this bug recently
 			Msg.debug(this, "Found disposed component that was not removed from the hierarchy " +
 				"list: " + this, ReflectionUtilities.createJavaFilteredThrowable());

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

@@ -242,13 +242,13 @@ class DetachedWindowNode extends WindowNode {
 
 	/**
 	 * Creates a list of titles from the given component providers and placeholders.  The utility
-	 * of this method is that it will group like component providers into one title value 
+	 * of this method is that it will group like component providers into one title value
 	 * instead of having one value for each placeholder.
 	 */
 	private List<String> generateTitles(List<ComponentPlaceholder> placeholders) {
 
 		//
-		// Decompose the given placeholders into a mapping of provider names to placeholders 
+		// Decompose the given placeholders into a mapping of provider names to placeholders
 		// that share that name.  This lets us group placeholders that are multiple instances of
 		// the same provider.
 		//
@@ -368,7 +368,7 @@ class DetachedWindowNode extends WindowNode {
 	}
 
 	/**
-	 * Ensures the bounds of this window have a valid location and size 
+	 * Ensures the bounds of this window have a valid location and size
 	 */
 	private void adjustBounds() {
 
@@ -474,6 +474,16 @@ class DetachedWindowNode extends WindowNode {
 	@Override
 	void dispose() {
 
+		disposeWindow();
+
+		if (child != null) {
+			child.parent = null;
+			child.dispose();
+			child = null;
+		}
+	}
+
+	private void disposeWindow() {
 		if (dropTargetHandler != null) {
 			dropTargetHandler.dispose();
 		}
@@ -485,12 +495,24 @@ class DetachedWindowNode extends WindowNode {
 			window.dispose();
 			window = null;
 		}
+	}
 
+	/**
+	 * An oddly named method for an odd use case.  This method is meant to take an existing window,
+	 * such as that created by default when loading plugins, and hide it.   Clients cannot simply
+	 * call dispose(), as that would also dispose the entire child hierarchy of this class.  This
+	 * method is intended to keep all children not disposed while allowing this window node to go
+	 * away.
+	 */
+	void disconnect() {
+
+		// note: do not call child.dispose() here
 		if (child != null) {
 			child.parent = null;
-			child.dispose();
 			child = null;
 		}
+
+		disposeWindow();
 	}
 
 	@Override

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

@@ -43,17 +43,22 @@ class PlaceholderManager {
 	}
 
 	ComponentPlaceholder replacePlaceholder(ComponentProvider provider,
-			ComponentPlaceholder oldPlaceholder) {
+			ComponentPlaceholder defaultPlaceholder) {
 
-		ComponentPlaceholder newPlaceholder = createOrRecyclePlaceholder(provider, oldPlaceholder);
+		// Note: the 'restoredPlaceholder' is from xml; the 'defaultPlaceholder' is that which was
+		// created by a plugin as it was constructed.  If there is no placeholder in the xml,
+		// then the original 'defaultPlaceholder' will be returned from
+		// createOrRecyclePlaceholder().
+		ComponentPlaceholder restoredPlaceholder =
+			createOrRecyclePlaceholder(provider, defaultPlaceholder);
 
-		moveActions(oldPlaceholder, newPlaceholder);
-		if (!oldPlaceholder.isHeaderShowing()) {
-			newPlaceholder.showHeader(false);
+		moveActions(defaultPlaceholder, restoredPlaceholder);
+		if (!defaultPlaceholder.isHeaderShowing()) {
+			restoredPlaceholder.showHeader(false);
 		}
 
-		if (oldPlaceholder.isShowing() != newPlaceholder.isShowing()) {
-			if (newPlaceholder.isShowing()) {
+		if (defaultPlaceholder.isShowing() != restoredPlaceholder.isShowing()) {
+			if (restoredPlaceholder.isShowing()) {
 				provider.componentShown();
 			}
 			else {
@@ -61,11 +66,13 @@ class PlaceholderManager {
 			}
 		}
 
-		if (newPlaceholder != oldPlaceholder) {
-			oldPlaceholder.dispose();
-			removePlaceholder(oldPlaceholder);
+		// if we have found a replacement placeholder, then remove the default placeholder
+		if (restoredPlaceholder != defaultPlaceholder) {
+			defaultPlaceholder.dispose();
+			removePlaceholder(defaultPlaceholder);
 		}
-		return newPlaceholder;
+
+		return restoredPlaceholder;
 	}
 
 	/**
@@ -240,8 +247,8 @@ class PlaceholderManager {
 		// 1) share the same owner (plugin), and
 		// 2) are in the same group, or related groups
 		//
-		// If there are not other providers that share the same owner as the one we are given, 
-		// then we will search all providers.  This allows different plugins to share 
+		// If there are not other providers that share the same owner as the one we are given,
+		// then we will search all providers.  This allows different plugins to share
 		// window arrangement.
 		//
 		Set<ComponentPlaceholder> buddies = activePlaceholders;
@@ -275,7 +282,7 @@ class PlaceholderManager {
 	 * provider.
 	 * @param activePlaceholders the set of currently showing placeholders.
 	 * @param newInfo the placeholder for the new provider to be shown.
-	 * @return an existing matching placeholder or null. 
+	 * @return an existing matching placeholder or null.
 	 */
 	private ComponentPlaceholder findBestPlaceholderToStackUpon(
 			Set<ComponentPlaceholder> activePlaceholders, ComponentPlaceholder newInfo) {

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

@@ -54,7 +54,7 @@ class RootNode extends WindowNode {
 	 * Constructs a new root node for the given DockingWindowsManager.
 	 * @param mgr the DockingWindowsManager
 	 * @param toolName the name of the tool to be displayed in all the top-level windows.
-	 * @param factory a factory for creating drop targets for this nodes windows; may be null 
+	 * @param factory a factory for creating drop targets for this nodes windows; may be null
 	 */
 	RootNode(DockingWindowManager mgr, String toolName, List<Image> images, boolean isModal,
 			DropTargetFactory factory) {
@@ -445,7 +445,13 @@ class RootNode extends WindowNode {
 
 	/**
 	 * Restores the component hierarchy from the given XML JDOM element.
-	 * @param root the XML from which to restore the state.
+	 * <p>
+	 * The process of restoring from xml will create new {@link ComponentPlaceholder}s that will be
+	 * used to replace any existing matching placeholders.  This allows the already loaded default
+	 * placeholders to be replaced by the previously saved configuration.
+	 * 
+	 * @param rootNodeElement the XML from which to restore the state.
+	 * @return the newly created placeholders
 	 */
 	List<ComponentPlaceholder> restoreFromXML(Element rootNodeElement) {
 		invalid = true;
@@ -455,7 +461,7 @@ class RootNode extends WindowNode {
 		detachedWindows.clear();
 		for (DetachedWindowNode windowNode : copy) {
 			notifyWindowRemoved(windowNode);
-			windowNode.dispose();
+			windowNode.disconnect();
 		}
 
 		int x = Integer.parseInt(rootNodeElement.getAttributeValue("X_POS"));
@@ -591,7 +597,7 @@ class RootNode extends WindowNode {
 
 //==================================================================================================
 // Inner Classes
-//==================================================================================================	
+//==================================================================================================
 
 	/** Interface to wrap JDialog and JFrame so that they can be used by one handle */
 	private interface SwingWindowWrapper {