From 53476edf4770bc03aff9071123c2cf151ea03744 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Thu, 28 Jul 2022 18:35:57 -0400 Subject: [PATCH] GP-2371 - Fixed tool restoring exception --- .../java/docking/ComponentPlaceholder.java | 10 ++++-- .../main/java/docking/DetachedWindowNode.java | 30 +++++++++++++--- .../main/java/docking/PlaceholderManager.java | 35 +++++++++++-------- .../src/main/java/docking/RootNode.java | 14 +++++--- 4 files changed, 65 insertions(+), 24 deletions(-) diff --git a/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java b/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java index 9008c949ae..9d4fcafc7d 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java +++ b/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. + *

+ * 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()); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DetachedWindowNode.java b/Ghidra/Framework/Docking/src/main/java/docking/DetachedWindowNode.java index 3d655a03a2..e15055d5cd 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/DetachedWindowNode.java +++ b/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 generateTitles(List 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 diff --git a/Ghidra/Framework/Docking/src/main/java/docking/PlaceholderManager.java b/Ghidra/Framework/Docking/src/main/java/docking/PlaceholderManager.java index 93d1a9c8e5..349bb3faa7 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/PlaceholderManager.java +++ b/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 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 activePlaceholders, ComponentPlaceholder newInfo) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/RootNode.java b/Ghidra/Framework/Docking/src/main/java/docking/RootNode.java index 89c4b38967..03a9d5a589 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/RootNode.java +++ b/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 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. + *

+ * 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 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 {