diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/CreateTypeDefAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/CreateTypeDefAction.java index 174c388cf2..22e5fa35f1 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/CreateTypeDefAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/CreateTypeDefAction.java @@ -129,6 +129,7 @@ public class CreateTypeDefAction extends AbstractTypeDefAction { GTreeNode finalParentNode = info.getParentNode(); String newNodeName = newTypeDef.getName(); + dataTypeManager.flushEvents(); gTree.startEditing(finalParentNode, newNodeName); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeCopyMoveDragTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeCopyMoveDragTest.java index 380c2695af..773b40a78f 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeCopyMoveDragTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeCopyMoveDragTest.java @@ -23,7 +23,6 @@ import java.util.ArrayList; import java.util.List; import javax.swing.JButton; -import javax.swing.tree.TreePath; import org.junit.*; @@ -291,16 +290,12 @@ public class DataTypeCopyMoveDragTest extends AbstractGhidraHeadedIntegrationTes CategoryNode miscNode = (CategoryNode) programNode.getChild("MISC"); expandNode(miscNode); - TreePath structureNodePath = structureNode.getTreePath(); - TreePath category3Path = category3Node.getTreePath(); - TreePath miscPath = miscNode.getTreePath(); - // copy/drag ArrayStruct to MISC copyNodeToNode(structureNode, miscNode); - structureNode = (DataTypeNode) tree.getViewNodeForPath(structureNodePath); - category3Node = (CategoryNode) tree.getViewNodeForPath(category3Path); - miscNode = (CategoryNode) tree.getViewNodeForPath(miscPath); + structureNode = (DataTypeNode) tree.getViewNode(structureNode); + category3Node = (CategoryNode) tree.getViewNode(category3Node); + miscNode = (CategoryNode) tree.getViewNode(miscNode); CategoryNode parent = miscNode; DataTypeNode node = @@ -324,16 +319,12 @@ public class DataTypeCopyMoveDragTest extends AbstractGhidraHeadedIntegrationTes CategoryNode miscNode = (CategoryNode) programNode.getChild("MISC"); expandNode(miscNode); - TreePath structureNodePath = structureNode.getTreePath(); - TreePath category3Path = category3Node.getTreePath(); - TreePath miscPath = miscNode.getTreePath(); - // copy/drag ArrayStruct to MISC copyNodeToNode(structureNode, miscNode); - structureNode = (DataTypeNode) tree.getViewNodeForPath(structureNodePath); - category3Node = (CategoryNode) tree.getViewNodeForPath(category3Path); - miscNode = (CategoryNode) tree.getViewNodeForPath(miscPath); + structureNode = (DataTypeNode) tree.getViewNode(structureNode); + category3Node = (CategoryNode) tree.getViewNode(category3Node); + miscNode = (CategoryNode) tree.getViewNode(miscNode); DataTypeNode node = (DataTypeNode) miscNode.getChild(structName); assertNotNull(node); @@ -509,23 +500,19 @@ public class DataTypeCopyMoveDragTest extends AbstractGhidraHeadedIntegrationTes CategoryNode miscNode = (CategoryNode) programNode.getChild("MISC"); expandNode(miscNode); - TreePath structureNodePath = structureNode.getTreePath(); - TreePath category3Path = category3Node.getTreePath(); - TreePath miscPath = miscNode.getTreePath(); - // drag/move ArrayStruct to MISC/ArrayStruct DataTypeNode miscStructureNode = (DataTypeNode) miscNode.getChild("ArrayStruct"); dragNodeToNode(structureNode, miscStructureNode); pressButtonOnOptionDialog("No"); - structureNode = (DataTypeNode) tree.getViewNodeForPath(structureNodePath); + structureNode = (DataTypeNode) tree.getViewNode(structureNode); assertNotNull(structureNode.getParent()); - category3Node = (CategoryNode) tree.getViewNodeForPath(category3Path); + category3Node = (CategoryNode) tree.getViewNode(category3Node); assertNotNull(category3Node.getChild("ArrayStruct")); - miscNode = (CategoryNode) tree.getViewNodeForPath(miscPath); + miscNode = (CategoryNode) tree.getViewNode(miscNode); DataTypeNode node = (DataTypeNode) miscNode.getChild("ArrayStruct"); assertTrue(!structure.isEquivalent(node.getDataType())); } @@ -583,21 +570,17 @@ public class DataTypeCopyMoveDragTest extends AbstractGhidraHeadedIntegrationTes DataTypeNode miscStructureNode = (DataTypeNode) miscNode.getChild("ArrayStruct"); DataType origDt = miscStructureNode.getDataType(); - TreePath structureNodePath = structureNode.getTreePath(); - TreePath category3Path = category3Node.getTreePath(); - TreePath miscPath = miscNode.getTreePath(); - copyNodeToNode(structureNode, miscStructureNode); pressButtonOnOptionDialog("No"); - structureNode = (DataTypeNode) tree.getViewNodeForPath(structureNodePath); + structureNode = (DataTypeNode) tree.getViewNode(structureNode); assertNotNull(structureNode.getParent()); - category3Node = (CategoryNode) tree.getViewNodeForPath(category3Path); + category3Node = (CategoryNode) tree.getViewNode(category3Node); assertNotNull(category3Node.getChild("ArrayStruct")); - miscNode = (CategoryNode) tree.getViewNodeForPath(miscPath); + miscNode = (CategoryNode) tree.getViewNode(miscNode); DataTypeNode node = (DataTypeNode) miscNode.getChild(structName); assertEquals(origDt, node.getDataType()); } @@ -621,21 +604,17 @@ public class DataTypeCopyMoveDragTest extends AbstractGhidraHeadedIntegrationTes DataTypeNode miscStructureNode = (DataTypeNode) miscNode.getChild("ArrayStruct"); DataType miscStructure = miscStructureNode.getDataType(); - TreePath structureNodePath = structureNode.getTreePath(); - TreePath category3Path = category3Node.getTreePath(); - TreePath miscPath = miscNode.getTreePath(); - copyNodeToNode(structureNode, miscStructureNode); pressButtonOnOptionDialog("Yes"); - structureNode = (DataTypeNode) tree.getViewNodeForPath(structureNodePath); + structureNode = (DataTypeNode) tree.getViewNode(structureNode); assertNotNull(structureNode.getParent()); - category3Node = (CategoryNode) tree.getViewNodeForPath(category3Path); + category3Node = (CategoryNode) tree.getViewNode(category3Node); assertNotNull(category3Node.getChild(structName)); - miscNode = (CategoryNode) tree.getViewNodeForPath(miscPath); + miscNode = (CategoryNode) tree.getViewNode(miscNode); DataTypeNode node = (DataTypeNode) miscNode.getChild(structName); assertTrue(structure.isEquivalent(node.getDataType())); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java index ff74e5e16f..a320657d0b 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java @@ -25,7 +25,8 @@ import java.awt.event.MouseListener; import java.io.PrintWriter; import java.util.*; import java.util.List; -import java.util.function.BooleanSupplier; +import java.util.function.Consumer; +import java.util.function.Supplier; import javax.swing.*; import javax.swing.Timer; @@ -60,7 +61,7 @@ public class GTree extends JPanel implements BusyListener { /** * This is the root node of the tree's data model. It may or may not be the root node - * that is currently being displayed by the tree. If there is currently a + * that is currently being displayed by the tree. If there is currently a * filter applied, then then the displayed root node will be a clone whose children have been * trimmed to only those that match the filter. By keeping this variable around, we can give * this node to clients, regardless of the root node visible in the tree. @@ -68,16 +69,16 @@ public class GTree extends JPanel implements BusyListener { private volatile GTreeNode realModelRootNode; /** - * This is the root that is currently being displayed. This node will be either exactly the + * This is the root that is currently being displayed. This node will be either exactly the * same instance as the realModelRootNode (if no filter has been applied) or it will be the - * filtered clone of the realModelRootNode. + * filtered clone of the realModelRootNode. */ private volatile GTreeNode realViewRootNode; /** * The rootParent is a node that is assigned as the parent to the realRootNode. It's primary purpose is * to allow nodes access to the tree. It overrides the getTree() method on GTreeNode to return - * this tree. This eliminated the need for clients to create special root nodes that had + * this tree. This eliminated the need for clients to create special root nodes that had * public setTree/getTree methods. */ private GTreeRootParentNode rootParent = new GTreeRootParentNode(this); @@ -276,17 +277,23 @@ public class GTree extends JPanel implements BusyListener { } public void ignoreFilter(GTreeNode node) { + if (!isFiltered()) { + return; + } ignoredNodes.add(node); updateModelFilter(); } protected void updateModelFilter() { filter = filterProvider.getFilter(); - if (!ignoredNodes.isEmpty()) { + if (filter != null && !ignoredNodes.isEmpty()) { filter = new IgnoredNodesGtreeFilter(filter, ignoredNodes); } - // TODO why? + // Normally this call is made from the update manager, which means we do not need to stop + // it manually. However, this method may be called directly by the tree. In that case, + // we will be performing a filter now, so there is no need to allow the update manager + // to keep buffering. filterUpdateManager.stop(); if (lastFilterTask != null) { @@ -338,7 +345,7 @@ public class GTree extends JPanel implements BusyListener { } /** - * Signal to the tree that it should record its expanded and selected state when a + * Signal to the tree that it should record its expanded and selected state when a * new filter is applied */ void saveFilterRestoreState() { @@ -570,24 +577,50 @@ public class GTree extends JPanel implements BusyListener { return null; } + /** + * Gets the model node for the given node. This is useful if the node that is in the path has + * been replaced by a new node that is equal, but a different instance. One way this happens + * is if the tree is filtered and therefor the displayed nodes are clones of the model nodes. + * This can also happen if the tree nodes are rebuilt for some reason. + * + * @param node the node + * @return the corresponding model node in the tree. If the tree is filtered the viewed node + * will be a clone of the corresponding model node. + */ + public GTreeNode getModelNode(GTreeNode node) { + return getNodeForPath(getModelRoot(), node.getTreePath()); + } + /** * Gets the model node for the given path. This is useful if the node that is in the path has * been replaced by a new node that is equal, but a different instance. One way this happens - * is if the tree is filtered and therefor the displayed nodes are clones of the model nodes. This - * can also happen if the tree nodes are rebuilt for some reason. + * is if the tree is filtered and therefor the displayed nodes are clones of the model nodes. + * This can also happen if the tree nodes are rebuilt for some reason. * * @param path the path of the node - * @return the corresponding model node in the tree. If the tree is filtered the viewed node will - * be a clone of the corresponding model node. + * @return the corresponding model node in the tree. If the tree is filtered the viewed node + * will be a clone of the corresponding model node. */ public GTreeNode getModelNodeForPath(TreePath path) { return getNodeForPath(getModelRoot(), path); } /** - * Gets the view node for the given path. This is useful to translate to a tree path that - * is valid for the currently displayed tree. (Remember that if the tree is filtered, - * then the displayed nodes are clones of the model nodes.) + * Gets the view node for the given node. This is useful to translate to a tree path that is + * valid for the currently displayed tree. (Remember that if the tree is filtered, then the + * displayed nodes are clones of the model nodes.) + * + * @param node the node + * @return the current node in the displayed (possibly filtered) tree + */ + public GTreeNode getViewNode(GTreeNode node) { + return getNodeForPath(getViewRoot(), node.getTreePath()); + } + + /** + * Gets the view node for the given path. This is useful to translate to a tree path that is + * valid for the currently displayed tree. (Remember that if the tree is filtered, then the + * displayed nodes are clones of the model nodes.) * * @param path the path of the node * @return the current node in the displayed (possibly filtered) tree @@ -608,8 +641,9 @@ public class GTree extends JPanel implements BusyListener { } return null; // invalid path--the root of the path is not equal to our root! } + if (node.getRoot() == root) { - return node; + return node; // this node is a valid child of the given root } GTreeNode parentNode = getNodeForPath(root, path.getParentPath()); @@ -735,7 +769,7 @@ public class GTree extends JPanel implements BusyListener { } /** - * Sets the root node for this tree. + * Sets the root node for this tree. *

* NOTE: if this method is not called from the Swing thread, then the root node will be set * later on the Swing thread. That is, this method will return before the work has been done. @@ -782,7 +816,7 @@ public class GTree extends JPanel implements BusyListener { /** * This method returns the root node that was provided to the tree by the client, whether from the - * constructor or from {@link #setRootNode(GTreeNode)}. + * constructor or from {@link #setRootNode(GTreeNode)}. * This node represents the data model and always contains all the nodes regardless of any filter * being applied. If a filter is applied to the tree, then this is not the actual root node being * displayed by the {@link JTree}. @@ -796,7 +830,7 @@ public class GTree extends JPanel implements BusyListener { * This method returns the root node currently being displayed by the {@link JTree}. If there * are no filters applied, then this will be the same as the model root (See {@link #getModelRoot()}). * If a filter is applied, then this will be a clone of the model root that contains clones of all - * nodes matching the filter. + * nodes matching the filter. * @return the root node currently being display by the {@link JTree} */ public GTreeNode getViewRoot() { @@ -956,15 +990,96 @@ public class GTree extends JPanel implements BusyListener { tree.setEditable(editable); } + // Waits for the given model node, passing it to the consumer when available + private void getModelNode(GTreeNode parent, String childName, Consumer consumer) { + + int expireMs = 3000; + Supplier supplier = () -> { + GTreeNode modelParent = getModelNode(parent); + if (modelParent != null) { + return modelParent.getChild(childName); + } + return null; + }; + ExpiringSwingTimer.get(supplier, expireMs, consumer); + } + + // Waits for the given view node, passing it to the consumer when available + private void getViewNode(GTreeNode parent, String childName, Consumer consumer) { + + int expireMs = 3000; + Supplier supplier = () -> { + GTreeNode viewParent = getViewNode(parent); + if (viewParent != null) { + return viewParent.getChild(childName); + } + return null; + }; + ExpiringSwingTimer.get(supplier, expireMs, consumer); + } + /** - * Requests that the node with the given name, in the given parent, be edited. This - * operation is asynchronous. This request will be buffered as needed to wait for - * the given node to be added to the parent, up to a timeout period. + * A specialized method that will get the child node from the given parent node when it + * becomes available to the model. This method will ensure that the named child passes any + * current filter in order for the child to appear in the tree. This effect is temporary and + * will be undone when next the filter changes. + * + *

This method is intended to be used by clients using an asynchronous node model, where + * new nodes will get created by application-level events. Such clients may wish to perform + * work when newly created nodes become available. This method simplifies the concurrent + * nature of the GTree, asynchronous nodes and the processing of asynchronous application-level + * events by providing a callback mechanism for clients. This method is non-blocking. + * + *

Note: this method assumes that the given parent node is in the view and not filtered + * out of the view. This method makes no attempt to ensure the given parent node passes any + * existing filter. + * + *

Note: this method will not wait forever for the given node to appear. It will eventually + * give up if the node never arrives. + * + * @param parent the model's parent node. If the view's parent node is passed, it will + * be translated to the model node. + * @param childName the name of the desired child + * @param consumer the consumer callback to which the child node will be given when available + */ + public void forceNewNodeIntoView(GTreeNode parent, String childName, + Consumer consumer) { + + /* + + If the GTree were to use Java's CompletableStage API, then the code below + could be written thusly: + + tree.getNewNode(modelParent, newName) + .thenCompose(newModelChild -> { + tree.ignoreFilter(newModelChild); + return tree.getNewNode(viewParent, newName); + )) + .thenAccept(consumer); + + */ + + // ensure we operate on the model node which will always have the given child not the view + // node, which may have its child filtered + GTreeNode modelParent = getModelNode(parent); + getModelNode(modelParent, childName, newModelChild -> { + // force the filter to accept the new node + ignoreFilter(newModelChild); + + // Wait for the view to update from any filtering that may take place + getViewNode(modelParent, childName, consumer); + }); + } + + /** + * Requests that the node with the given name, in the given parent, be edited. This operation + * is asynchronous. This request will be buffered as needed to wait for the given node to be + * added to the parent, up to a timeout period. * * @param parent the parent node * @param childName the name of the child to edit */ - public void startEditing(GTreeNode parent, final String childName) { + public void startEditing(GTreeNode parent, String childName) { // we call this here, even though the JTree will do this for us, so that we will trigger // a load call before this task is run, in case lazy nodes are involved in this tree, @@ -973,26 +1088,19 @@ public class GTree extends JPanel implements BusyListener { // // The request to edit the node may be for a node that has not yet been added to this - // tree. Further, some clients will buffer events, which means that the node the client + // tree. Further, some clients will buffer events, which means that the node the client // wishes to edit may not yet be in the parent node even if we run this request later on // the Swing thread. To deal with this, we use a construct that will run our request // once the given node has been added to the parent. // - GTreeNode modelParent = getModelNodeForPath(parent.getTreePath()); - BooleanSupplier isReady = () -> modelParent.getChild(childName) != null; - int expireMs = 3000; - ExpiringSwingTimer.runWhen(isReady, expireMs, () -> { - if (isFiltered()) { - GTreeNode child = modelParent.getChild(childName); - ignoreFilter(child); - updateModelFilter(); - } - runTask(new GTreeStartEditingTask(GTree.this, tree, modelParent, childName)); + GTreeNode modelParent = getModelNode(parent); + forceNewNodeIntoView(modelParent, childName, newViewNode -> { + runTask(new GTreeStartEditingTask(GTree.this, tree, newViewNode)); }); } /** - * Requests that the node be edited. This operation is asynchronous. + * Requests that the node be edited. This operation is asynchronous. * * @param child the node to edit */ @@ -1248,7 +1356,7 @@ public class GTree extends JPanel implements BusyListener { private void updateDefaultKeyBindings() { // Remove the edit keybinding, as the GTree triggers editing via a task, since it - // is multi-threaded. Doing this allows users to assign their own key bindings to + // is multi-threaded. Doing this allows users to assign their own key bindings to // the edit task. KeyBindingUtils.clearKeyBinding(this, "startEditing"); } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeFilterTask.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeFilterTask.java index a54b5f904f..bd8916e6c9 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeFilterTask.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeFilterTask.java @@ -25,12 +25,11 @@ import ghidra.util.task.TaskMonitor; public class GTreeFilterTask extends GTreeTask { private final GTreeFilter filter; - private boolean cancelledProgramatically; + private volatile boolean cancelledProgramatically; public GTreeFilterTask(GTree tree, GTreeFilter filter) { super(tree); this.filter = filter; - tree.saveFilterRestoreState(); } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeTask.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeTask.java index d595a81d17..359ab720bf 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeTask.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeTask.java @@ -18,8 +18,7 @@ package docking.widgets.tree; import javax.swing.JTree; import javax.swing.tree.TreePath; -import ghidra.util.Msg; -import ghidra.util.SystemUtilities; +import ghidra.util.Swing; import ghidra.util.task.TaskMonitor; import ghidra.util.worker.PriorityJob; @@ -43,7 +42,7 @@ public abstract class GTreeTask extends PriorityJob { if (isCancelled()) { return; } - SystemUtilities.runSwingNow(new CheckCancelledRunnable(runnable)); + Swing.runNow(new CheckCancelledRunnable(runnable)); } /** @@ -62,9 +61,7 @@ public abstract class GTreeTask extends PriorityJob { // note: call this on the Swing thread, since the Swing thread maintains the node state // (we have seen errors where the tree will return nodes that are in the process // of being disposed) - Msg.debug(this, "before translate: " + path); - GTreeNode nodeForPath = SystemUtilities.runSwingNow(() -> tree.getViewNodeForPath(path)); - Msg.debug(this, "After translate, node = " + nodeForPath); + GTreeNode nodeForPath = Swing.runNow(() -> tree.getViewNodeForPath(path)); if (nodeForPath != null) { return nodeForPath.getTreePath(); } @@ -73,7 +70,7 @@ public abstract class GTreeTask extends PriorityJob { //================================================================================================== // Inner Classes -//================================================================================================== +//================================================================================================== class CheckCancelledRunnable implements Runnable { private final Runnable runnable; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java index c8f2869863..99f499119e 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java @@ -30,7 +30,7 @@ import ghidra.util.SystemUtilities; public class GTreeModel implements TreeModel { private volatile GTreeNode root; - private List listeners = new ArrayList(); + private List listeners = new ArrayList<>(); private boolean isFiringNodeStructureChanged; private volatile boolean eventsEnabled = true; @@ -82,9 +82,9 @@ public class GTreeModel implements TreeModel { // This can happen if the client code mutates the children of this node in a background // thread such that there are fewer child nodes on this node, and then before the tree // is notified, the JTree attempts to access a child that is no longer present. The - // GTree design specifically allows this situation to occur as a trade off for + // GTree design specifically allows this situation to occur as a trade off for // better performance when performing bulk operations (such as filtering). If this - // does occur, this can be handled easily by temporarily returning a dummy node and + // does occur, this can be handled easily by temporarily returning a dummy node and // scheduling a node structure changed event to reset the JTree. Swing.runLater(() -> fireNodeStructureChanged((GTreeNode) parent)); return new InProgressGTreeNode(); @@ -125,17 +125,17 @@ public class GTreeModel implements TreeModel { "GTreeModel.fireNodeStructuredChanged() must be " + "called from the AWT thread"); // If the tree is filtered and this is called on the original node, we have to - // translate the node to a view node (one the jtree knows). + // translate the node to a view node (one the jtree knows). GTreeNode viewNode = convertToViewNode(changedNode); if (viewNode == null) { return; } if (viewNode != changedNode) { - // This means we are filtered and since the original node's children are invalid, + // This means we are filtered and since the original node's children are invalid, // then the filtered children are invalid also. So clear out the children by - // setting an empty list as we don't want to trigger the node to regenerate its - // children which happens if you set the children to null. + // setting an empty list as we don't want to trigger the node to regenerate its + // children which happens if you set the children to null. // // This won't cause a second event to the jtree because we are protected // by the isFiringNodeStructureChanged variable @@ -170,9 +170,9 @@ public class GTreeModel implements TreeModel { if (viewNode == null) { return; } - // Note - we are passing in the treepath of the node that changed. The javadocs in + // Note - we are passing in the treepath of the node that changed. The javadocs in // TreemodelListener seems to imply that you need to pass in the treepath of the parent - // of the node that changed and then the indexes of the children that changed. But this + // of the node that changed and then the indexes of the children that changed. But this // works and is cheaper then computing the index of the node that changed. TreeModelEvent event = new TreeModelEvent(this, viewNode.getTreePath()); @@ -247,7 +247,7 @@ public class GTreeModel implements TreeModel { } GTree tree = root.getTree(); if (tree != null) { - return tree.getViewNodeForPath(node.getTreePath()); + return tree.getViewNode(node); } return null; } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/support/IgnoredNodesGtreeFilter.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/support/IgnoredNodesGtreeFilter.java index 3e6e22d0a4..d9279e7593 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/support/IgnoredNodesGtreeFilter.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/support/IgnoredNodesGtreeFilter.java @@ -15,6 +15,7 @@ */ package docking.widgets.tree.support; +import java.util.Objects; import java.util.Set; import docking.widgets.tree.GTreeNode; @@ -28,8 +29,8 @@ public class IgnoredNodesGtreeFilter implements GTreeFilter { private Set ignoredNodes; public IgnoredNodesGtreeFilter(GTreeFilter filter, Set ignoredNodes) { - this.filter = filter; - this.ignoredNodes = ignoredNodes; + this.filter = Objects.requireNonNull(filter); + this.ignoredNodes = Objects.requireNonNull(ignoredNodes); } @Override diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeSelectPathsTask.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeSelectPathsTask.java index 065f471591..46dccbb247 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeSelectPathsTask.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeSelectPathsTask.java @@ -98,7 +98,7 @@ public class GTreeSelectPathsTask extends GTreeTask { selectionModel.setSelectionPaths(treePaths, origin); if (treePaths != null && treePaths.length > 0) { - // Scroll to the last item, as the tree will make the given path appear at the + // Scroll to the last item, as the tree will make the given path appear at the // bottom of the view. By scrolling the last item, all the selected items above // this one will appear in the view as well. jTree.scrollPathToVisible(treePaths[treePaths.length - 1]); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeStartEditingTask.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeStartEditingTask.java index a4d6944400..c7a298ce28 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeStartEditingTask.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeStartEditingTask.java @@ -15,7 +15,7 @@ */ package docking.widgets.tree.tasks; -import java.util.*; +import java.util.Objects; import javax.swing.CellEditor; import javax.swing.JTree; @@ -24,28 +24,17 @@ import javax.swing.event.ChangeEvent; import javax.swing.tree.TreePath; import docking.widgets.tree.*; -import ghidra.util.Msg; -import ghidra.util.SystemUtilities; -import ghidra.util.exception.AssertException; import ghidra.util.exception.CancelledException; import ghidra.util.task.TaskMonitor; public class GTreeStartEditingTask extends GTreeTask { private final GTreeNode modelParent; - private final String childName; - private GTreeNode editNode; - - public GTreeStartEditingTask(GTree gTree, JTree jTree, GTreeNode parent, String childName) { - super(gTree); - this.modelParent = parent; - this.childName = childName; - } + private final GTreeNode editNode; public GTreeStartEditingTask(GTree gTree, JTree jTree, GTreeNode editNode) { super(gTree); - this.modelParent = tree.getModelNodeForPath(editNode.getParent().getTreePath()); - this.childName = editNode.getName(); + this.modelParent = tree.getModelNode(editNode.getParent()); this.editNode = editNode; } @@ -65,84 +54,29 @@ public class GTreeStartEditingTask extends GTreeTask { } private void edit() { - GTreeNode viewParent = tree.getViewNodeForPath(modelParent.getTreePath()); - if (editNode == null) { - editNode = viewParent.getChild(childName); - if (editNode == null) { - Msg.debug(this, "Can't find node \"" + childName + "\" to edit."); - return; - } - } TreePath path = editNode.getTreePath(); - final Set childrenBeforeEdit = new HashSet<>(viewParent.getChildren()); - - final CellEditor cellEditor = tree.getCellEditor(); + CellEditor cellEditor = tree.getCellEditor(); cellEditor.addCellEditorListener(new CellEditorListener() { @Override public void editingCanceled(ChangeEvent e) { cellEditor.removeCellEditorListener(this); - SystemUtilities.runSwingLater(this::reselectNode); + reselectNode(); } @Override public void editingStopped(ChangeEvent e) { String newName = Objects.toString(cellEditor.getCellEditorValue()); cellEditor.removeCellEditorListener(this); - SystemUtilities - .runSwingLater(() -> reselectNodeHandlingPotentialChildChange(newName)); + + tree.forceNewNodeIntoView(modelParent, newName, newViewChild -> { + tree.setSelectedNode(newViewChild); + }); } - /** - * Unusual Code Alert!: This method handles the case where editing of a node triggers - * a new node to be created. In this case, reselecting the - * node that was edited can leave the tree with a selection that - * points to a removed node, which has bad consequences to clients. - * We work around this issue by retrieving the node after the edit - * has finished and been applied. - */ private void reselectNode() { - String newName = editNode.getName(); - GTreeNode newModelChild = modelParent.getChild(newName); - - if (newModelChild == null) { - throw new AssertException("Unable to find new node by name: " + newName); - } - tree.setSelectedNode(tree.getViewNodeForPath(newModelChild.getTreePath())); - } - - /** - * Unusual Code Alert!: This method handles the case where editing of a node triggers - * a new node to be created. In this case, reselecting the - * node that was edited can leave the tree with a selection that - * points to a removed node, which has bad consequences to clients. - * We work around this issue by retrieving the node after the edit - * has finished and been applied. - * - * This method takes into account the fact that we are not given - * the new name of the node in our editingStopped() callback. - * As such, we have to deduce the newly added node, based upon - * the state of the edited node's parent, both before and after - * the edit. - */ - private void reselectNodeHandlingPotentialChildChange(String newName) { - SystemUtilities - .runSwingLater(() -> doReselectNodeHandlingPotentialChildChange(newName)); - } - - private void doReselectNodeHandlingPotentialChildChange(String newName) { - GTreeNode newModelChild = modelParent.getChild(newName); - List children = modelParent.getChildren(); - Msg.debug(this, children.toString()); - - if (newModelChild != null) { - tree.ignoreFilter(newModelChild); - - tree.setSelectedNode(newModelChild); - Msg.debug(this, "new child not null"); - } - else { - Msg.debug(this, "child is null"); - } + String name = editNode.getName(); + GTreeNode newModelChild = modelParent.getChild(name); + tree.setSelectedNode(newModelChild); } }); diff --git a/Ghidra/Framework/Generic/src/main/java/generic/timer/ExpiringSwingTimer.java b/Ghidra/Framework/Generic/src/main/java/generic/timer/ExpiringSwingTimer.java index 834c6c5278..b68a94b04b 100644 --- a/Ghidra/Framework/Generic/src/main/java/generic/timer/ExpiringSwingTimer.java +++ b/Ghidra/Framework/Generic/src/main/java/generic/timer/ExpiringSwingTimer.java @@ -17,13 +17,15 @@ package generic.timer; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.BooleanSupplier; +import java.util.function.*; + +import utility.function.Dummy; /** - * This class allows clients to run swing action at some point in the future, when the given + * This class allows clients to run swing action at some point in the future, when the given * condition is met, allowing for the task to timeout. While this class implements the * {@link GhidraTimer} interface, it is really meant to be used to execute a code snippet one - * time at some point in the future. + * time at some point in the future. * *

Both the call to check for readiness and the actual client code will be run on the Swing * thread. @@ -38,10 +40,10 @@ public class ExpiringSwingTimer extends GhidraSwingTimer { private AtomicBoolean didRun = new AtomicBoolean(); /** - * Runs the given client runnable when the given condition returns true. The returned timer + * Runs the given client runnable when the given condition returns true. The returned timer * will be running. * - *

Once the timer has performed the work, any calls to start the returned timer will + *

Once the timer has performed the work, any calls to start the returned timer will * not perform any work. You can check {@link #didRun()} to see if the work has been completed. * * @param isReady true if the code should be run @@ -49,15 +51,47 @@ public class ExpiringSwingTimer extends GhidraSwingTimer { * @param runnable the code to run * @return the timer object that is running, which will execute the given code when ready */ - public static ExpiringSwingTimer runWhen(BooleanSupplier isReady, - int expireMs, + public static ExpiringSwingTimer runWhen(BooleanSupplier isReady, int expireMs, Runnable runnable) { - // Note: we could let the client specify the period, but that would add an extra argument + // Note: we could let the client specify the delay, but that would add an extra argument // to this method. For now, just use something reasonable. int delay = 250; - ExpiringSwingTimer timer = - new ExpiringSwingTimer(delay, expireMs, isReady, runnable); + ExpiringSwingTimer timer = new ExpiringSwingTimer(delay, expireMs, isReady, runnable); + timer.start(); + return timer; + } + + /** + * Calls the given consumer with the non-null value returned from the given supplier. The + * returned timer will be running. + * + *

Once the timer has performed the work, any calls to start the returned timer will + * not perform any work. You can check {@link #didRun()} to see if the work has been completed. + * + * @param the type used by the supplier and consumer + * @param supplier the supplier of the desired value + * @param expireMs the amount of time past which the code will not be run + * @param consumer the consumer to be called with the supplier's value + * @return the timer object that is running, which will execute the given code when ready + */ + public static ExpiringSwingTimer get(Supplier supplier, int expireMs, + Consumer consumer) { + + // Note: we could let the client specify the delay, but that would add an extra argument + // to this method. For now, just use something reasonable. + int delay = 250; + BooleanSupplier isReady = () -> { + T t = supplier.get(); + if (t == null) { + return false; + } + + consumer.accept(t); + return true; + }; + Runnable dummy = Dummy.runnable(); + ExpiringSwingTimer timer = new ExpiringSwingTimer(delay, expireMs, isReady, dummy); timer.start(); return timer; } @@ -65,7 +99,7 @@ public class ExpiringSwingTimer extends GhidraSwingTimer { /** * Constructor * - *

Note: this class sets the parent's initial delay to 0. This is to allow the client + *

Note: this class sets the parent's initial delay to 0. This is to allow the client * code to be executed without delay when the ready condition is true. * * @param delay the delay between calls to check isReady @@ -73,8 +107,7 @@ public class ExpiringSwingTimer extends GhidraSwingTimer { * @param expireMs the amount of time past which the code will not be run * @param runnable the code to run */ - public ExpiringSwingTimer(int delay, int expireMs, BooleanSupplier isReady, - Runnable runnable) { + public ExpiringSwingTimer(int delay, int expireMs, BooleanSupplier isReady, Runnable runnable) { super(0, delay, null); this.expireMs = expireMs; this.isReady = isReady; @@ -84,7 +117,7 @@ public class ExpiringSwingTimer extends GhidraSwingTimer { } /** - * Returns true if the client runnable was run + * Returns true if the client runnable was run * @return true if the client runnable was run */ public boolean didRun() { diff --git a/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java b/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java index 849d4ac806..08c27ece1c 100644 --- a/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java +++ b/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java @@ -17,9 +17,8 @@ package generic.timer; import static org.junit.Assert.*; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BooleanSupplier; +import java.util.concurrent.atomic.*; +import java.util.function.*; import org.junit.Test; @@ -47,6 +46,33 @@ public class ExpiringSwingTimerTest extends AbstractGenericTest { assertEquals("Client code was run more than once", 1, runCount.get()); } + @Test + public void testGet() { + + String expectedResult = "Test result"; + int waitCount = 2; + AtomicInteger counter = new AtomicInteger(); + Supplier supplier = () -> { + if (counter.incrementAndGet() > waitCount) { + return expectedResult; + } + return null; + }; + + AtomicReference result = new AtomicReference<>(); + AtomicInteger runCount = new AtomicInteger(); + Consumer consumer = s -> { + runCount.incrementAndGet(); + result.set(s); + }; + ExpiringSwingTimer.get(supplier, 10000, consumer); + + waitFor(() -> runCount.get() > 0); + assertEquals("", expectedResult, result.get()); + assertTrue("Timer did not wait for the condition to be true", counter.get() > waitCount); + assertEquals("Client code was run more than once", 1, runCount.get()); + } + @Test public void testRunWhenReady_Timeout() { @@ -63,6 +89,22 @@ public class ExpiringSwingTimerTest extends AbstractGenericTest { assertFalse(didRun.get()); } + @Test + public void testGet_Timeout() { + + Supplier supplier = () -> { + return null; + }; + + AtomicBoolean didRun = new AtomicBoolean(); + Consumer consumer = s -> didRun.set(true); + ExpiringSwingTimer timer = ExpiringSwingTimer.get(supplier, 500, consumer); + + waitFor(() -> !timer.isRunning()); + + assertFalse(didRun.get()); + } + @Test public void testWorkOnlyHappensOnce() {