diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java index eef44f14fb..e705e5c580 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java @@ -49,7 +49,7 @@ public class OrganizationNode extends SymbolTreeNode { private OrganizationNode(List list, int max, int parentLevel, TaskMonitor monitor) throws CancelledException { - setChildren(computeChildren(list, max, this, parentLevel, monitor)); + doSetChildren(computeChildren(list, max, this, parentLevel, monitor)); GTreeNode child = getChild(0); baseName = child.getName().substring(0, getPrefixSizeForGrouping(getChildren(), 1) + 1); @@ -128,8 +128,7 @@ public class OrganizationNode extends SymbolTreeNode { monitor.checkCanceled(); String str = list.get(i).getName(); if (stringsDiffer(prevStr, str, characterOffset)) { - addNode(children, list, start, i - 1, maxNodes, characterOffset, - monitor); + addNode(children, list, start, i - 1, maxNodes, characterOffset, monitor); start = i; } prevStr = str; @@ -144,16 +143,14 @@ public class OrganizationNode extends SymbolTreeNode { return true; } return s1.substring(0, diffLevel + 1) - .compareToIgnoreCase( - s2.substring(0, diffLevel + 1)) != 0; + .compareToIgnoreCase(s2.substring(0, diffLevel + 1)) != 0; } private static void addNode(List children, List list, int start, int end, - int max, int diffLevel, TaskMonitor monitor) - throws CancelledException { + int max, int diffLevel, TaskMonitor monitor) throws CancelledException { if (end - start > 0) { - children.add(new OrganizationNode(list.subList(start, end + 1), max, diffLevel, - monitor)); + children.add( + new OrganizationNode(list.subList(start, end + 1), max, diffLevel, monitor)); } else { GTreeNode node = list.get(start); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java index ac0912127f..098444f696 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java @@ -79,8 +79,8 @@ public abstract class SymbolCategoryNode extends SymbolTreeNode { while (it.hasNext()) { Symbol s = it.next(); monitor.incrementProgress(1); + monitor.checkCanceled(); if (s != null && (s.getSymbolType() == symbolType)) { - monitor.checkCanceled(); list.add(SymbolNode.createNode(s, program)); } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java index 01d50f14a6..099e92c268 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java @@ -18,8 +18,11 @@ package docking.widgets.tree; import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; +import javax.swing.JTree; + import docking.widgets.tree.internal.InProgressGTreeNode; import ghidra.util.Swing; +import ghidra.util.SystemUtilities; /** * This class exists to help prevent threading errors in {@link GTreeNode} and subclasses, @@ -27,24 +30,34 @@ import ghidra.util.Swing; *

* This implementation uses a {@link CopyOnWriteArrayList} to store its children. The theory is * that this will allow direct thread-safe access to the children without having to worry about - * {@link ConcurrentModificationException}s. Also, the assumption is that accessing the children - * will occur much more frequently than modifying the children. This should only be a problem if - * a direct descendent of AbstractGTreeNode creates it children by calling + * {@link ConcurrentModificationException}s while iterating the children. Also, the assumption + * is that accessing the children will occur much more frequently than modifying the children. + * This should only be a problem if a direct descendent of GTreeNode creates its children by calling * addNode many times. But in that case, the tree should be using Lazy or * SlowLoading nodes which always load into another list first and all the children will be set * on a node in a single operation. *

- * Subclasses that need access to the children - * can call the {@link #children()} method which will ensure that the children are - * loaded (not null). Since this class uses a {@link CopyOnWriteArrayList}, subclasses that call - * the {@link #children()} method can safely operate and iterate on the list they get back without - * having to worry about getting a {@link ConcurrentModificationException}. + * Subclasses that need access to the children can call the {@link #children()} method which will + * ensure that the children are loaded (not null). Since this class uses a + * {@link CopyOnWriteArrayList}, subclasses that call the {@link #children()} method can safely + * iterate the list without having to worry about getting a {@link ConcurrentModificationException}. *

* This class uses synchronization to assure that the parent/children relationship is stable across * threads. To avoid deadlocks, the sychronization strategy is that if you have the lock on - * a parent node, you can safely acquire the lock on any of its descendants, put never its + * a parent node, you can safely acquire the lock on any of its descendants, but never its * ancestors. To facilitate this strategy, the {@link #getParent()} is not synchronized, but it * is made volatile to assure the current value is always used. + *

+ * Except for the {@link #doSetChildren(List)} method, all other calls that mutate the + * children must be called on the swing thread. The idea is that bulk operations can work efficiently + * by avoiding constantly switching to the swing thread to mutate the tree. This works because + * the bulk setting of the children generates a coarse "node structure changed" event, which causes the + * underlying {@link JTree} to rebuild its internal cache of the tree. Individual add/remove operations + * have to be done very carefully such that the {@link JTree} is always updated on one change before any + * additional changes are done. This is why those operations are required to be done on the swing + * thread, which combined with the fact that all mutate operations are synchronized, keeps the JTree + * happy. + * */ abstract class CoreGTreeNode implements Cloneable { // the parent is volatile to facilitate the synchronization strategy (see comments above) @@ -118,6 +131,25 @@ abstract class CoreGTreeNode implements Cloneable { */ protected abstract List generateChildren(); + /** + * Sets the children of this node to the given list of child nodes and fires the appropriate + * tree event to kick the tree to update the display. Note: This method must be called + * from the swing thread because it will notify the underlying JTree. + * @param childList the list of child nodes to assign as children to this node + * @see #doSetChildren(List) if calling from a background thread. + */ + protected synchronized void doSetChildrenAndFireEvent(List childList) { + doSetChildren(childList); + doFireNodeStructureChanged(); + } + + /** + * Sets the children of this node to the given list of child nodes, but does not notify the + * tree. This method does not have to be called from the swing thread. It is intended to be + * used by background threads that want to populate all or part of the tree, but wait until + * the bulk operations are completed before notifying the tree. + * @param childList the list of child nodes to assign as children to this node + */ protected synchronized void doSetChildren(List childList) { List oldChildren = children; children = null; @@ -142,6 +174,57 @@ abstract class CoreGTreeNode implements Cloneable { } } + /** + * Adds a node to this node's children. Must be called from the swing thread. + * @param node the node to add as a child to this node + */ + protected synchronized void doAddNode(GTreeNode node) { + children().add(node); + node.setParent((GTreeNode) this); + doFireNodeAdded(node); + } + + /** + * Adds a node to this node's children at the given index and notifies the tree. + * Must be called from the swing thread. + * @param index the index at which to add the new node + * @param node the node to add as a child to this node + */ + protected synchronized void doAddNode(int index, GTreeNode node) { + List kids = children(); + int insertIndex = Math.min(kids.size(), index); + kids.add(insertIndex, node); + node.setParent((GTreeNode) this); + doFireNodeAdded(node); + } + + /** + * Removes the node from this node's children and notifies the tree. Must be called + * from the swing thread. + * @param node the node to remove + */ + protected synchronized void doRemoveNode(GTreeNode node) { + List kids = children(); + int index = kids.indexOf(node); + if (index >= 0) { + kids.remove(index); + node.setParent(null); + doFireNodeRemoved(node, index); + } + } + + /** + * Adds the given nodes to this node's children. Must be called from the swing thread. + * @param nodes the nodes to add to the children this node + */ + protected synchronized void doAddNodes(List nodes) { + for (GTreeNode node : nodes) { + node.setParent((GTreeNode) this); + } + children().addAll(nodes); + doFireNodeStructureChanged(); + } + /** * Creates a clone of this node. The clone should contain a shallow copy of all the node's * attributes except that the parent and children are null. @@ -157,7 +240,6 @@ abstract class CoreGTreeNode implements Cloneable { } public void dispose() { - List oldChildren; synchronized (this) { oldChildren = children; @@ -242,4 +324,44 @@ abstract class CoreGTreeNode implements Cloneable { return false; } + protected void doFireNodeAdded(GTreeNode newNode) { + assertSwing(); + GTree tree = getTree(); + if (tree != null) { + tree.getModel().fireNodeAdded((GTreeNode) this, newNode); + tree.refilterLater(newNode); + } + } + + protected void doFireNodeRemoved(GTreeNode removedNode, int index) { + assertSwing(); + GTree tree = getTree(); + if (tree != null) { + tree.getModel().fireNodeRemoved((GTreeNode) this, removedNode, index); + } + } + + protected void doFireNodeStructureChanged() { + assertSwing(); + GTree tree = getTree(); + if (tree != null) { + tree.getModel().fireNodeStructureChanged((GTreeNode) this); + tree.refilterLater(); + } + } + + protected void doFireNodeChanged() { + assertSwing(); + GTree tree = getTree(); + if (tree != null) { + tree.getModel().fireNodeDataChanged((GTreeNode) this); + tree.refilterLater(); + } + } + + private void assertSwing() { + SystemUtilities + .assertThisIsTheSwingThread("tree events must be called from the AWT thread"); + } + } 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 2f876131cd..6a64e00b09 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,6 +25,7 @@ import java.awt.event.MouseListener; import java.io.PrintWriter; import java.util.*; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import javax.swing.*; @@ -40,8 +41,8 @@ import docking.widgets.tree.internal.*; import docking.widgets.tree.support.*; import docking.widgets.tree.support.GTreeSelectionEvent.EventOrigin; import docking.widgets.tree.tasks.*; -import ghidra.util.FilterTransformer; -import ghidra.util.SystemUtilities; +import ghidra.util.*; +import ghidra.util.exception.AssertException; import ghidra.util.exception.CancelledException; import ghidra.util.task.*; import ghidra.util.worker.PriorityWorker; @@ -56,13 +57,20 @@ public class GTree extends JPanel implements BusyListener { private GTreeModel model; /** - * This is the root node that either is the actual current root node, or the node that will - * be the real root node, once the Worker has loaded it. Thus, it is possible that a call to - * {@link GTreeModel#getRoot()} will return an {@link InProgressGTreeRootNode}. By keeping - * this variable around, we can give this node to clients, regardless of the root node - * visible in the tree. + * 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 + * 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. */ - private GTreeNode realRootNode; + private volatile GTreeNode realModelRootNode; + + /** + * 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. + */ + private volatile GTreeNode realViewRootNode; /** * The rootParent is a node that is assigned as the parent to the realRootNode. It's primary purpose is @@ -108,7 +116,8 @@ public class GTree extends JPanel implements BusyListener { */ public GTree(GTreeNode root) { uniquePreferenceKey = generateFilterPreferenceKey(); - this.realRootNode = root; + this.realModelRootNode = root; + this.realViewRootNode = root; monitor = new TaskMonitorComponent(); monitor.setShowProgressValue(false);// the tree's progress is fabricated--don't paint it worker = new PriorityWorker("GTree Worker", monitor); @@ -265,7 +274,8 @@ public class GTree extends JPanel implements BusyListener { if (root != null) { root.dispose(); } - realRootNode.dispose();// just in case we were loading + realModelRootNode.dispose(); + realViewRootNode.dispose(); model.dispose(); } @@ -728,57 +738,66 @@ public class GTree extends JPanel implements BusyListener { * @param rootNode The node to set. */ public void setRootNode(GTreeNode rootNode) { - GTreeNode oldRoot = doSetRootNode(rootNode, true); - oldRoot.dispose(); - if (filter != null) { - filterUpdateManager.update(); - } - } - - private GTreeNode doSetRootNode(GTreeNode rootNode, boolean waitForJobs) { worker.clearAllJobs(); - GTreeNode root = model.getModelRoot(); - - this.realRootNode = rootNode; rootNode.setParent(rootParent); - - // - // We need to use our standard 'worker pipeline' for mutations to the tree. This means - // that requests from the Swing thread must go through the worker. However, - // non-Swing-thread requests can just block while we wait for cancelled work to finish - // and setup the new root. The assumption is that other threads (like test threads and - // client background threads) will want to block in order to get real-time data. Further, - // since they are not in the Swing thread, blocking will not lock-up the GUI. - // - if (SwingUtilities.isEventDispatchThread()) { - model.setRootNode(new InProgressGTreeRootNode()); - runTask(new SetRootNodeTask(this, rootNode, model)); - } - else { - if (waitForJobs) { - worker.waitUntilNoJobsScheduled(Integer.MAX_VALUE); + realModelRootNode = rootNode; + realViewRootNode = rootNode; + GTreeNode oldRoot; + try { + oldRoot = doSetModelRootNode(rootNode); + oldRoot.dispose(); + if (filter != null) { + filterUpdateManager.update(); } - monitor.clearCanceled(); - model.setRootNode(rootNode); } - return root; + catch (CancelledException e) { + throw new AssertException("Setting the root node should never be cancelled"); + } } void setFilteredRootNode(GTreeNode filteredRootNode) { filteredRootNode.setParent(rootParent); - GTreeNode currentRoot = (GTreeNode) model.getRoot(); - model.setRootNode(filteredRootNode); - if (currentRoot != realRootNode) { - currentRoot.disposeClones(); + realViewRootNode = filteredRootNode; + try { + GTreeNode currentRoot = doSetModelRootNode(filteredRootNode); + if (currentRoot != realModelRootNode) { + currentRoot.disposeClones(); + } + } + catch (CancelledException e) { + // the filter task was cancelled } } void restoreNonFilteredRootNode() { - GTreeNode currentRoot = (GTreeNode) model.getRoot(); - model.setRootNode(realRootNode); - if (currentRoot != realRootNode) { - currentRoot.disposeClones(); + realViewRootNode = realModelRootNode; + try { + GTreeNode currentRoot = doSetModelRootNode(realModelRootNode); + if (currentRoot != realModelRootNode) { + currentRoot.disposeClones(); + } } + catch (CancelledException e) { + // the filter task was cancelled + } + } + + private GTreeNode doSetModelRootNode(GTreeNode rootNode) throws CancelledException { + // If this method is called from a background filter task, then it may be cancelled + // by other tree operations. Not all tasks can be cancelled. + AtomicBoolean wasCancelled = new AtomicBoolean(true); + GTreeNode node = Swing.runNow(() -> { + GTreeNode old = model.getModelRoot(); + model.setRootNode(rootNode); + + wasCancelled.set(false); + return old; + }); + + if (wasCancelled.get()) { + throw new CancelledException(); + } + return node; } /** @@ -790,7 +809,7 @@ public class GTree extends JPanel implements BusyListener { * @return the root node as provided by the client. */ public GTreeNode getModelRoot() { - return realRootNode; + return realModelRootNode; } /** @@ -801,7 +820,7 @@ public class GTree extends JPanel implements BusyListener { * @return the root node currently being display by the {@link JTree} */ public GTreeNode getViewRoot() { - return (GTreeNode) model.getRoot(); + return realViewRootNode; } /** diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeNode.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeNode.java index 8944dd9875..618d22d7b9 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeNode.java @@ -33,6 +33,9 @@ import util.CollectionUtils; * all their children in hand when initially constructed (either in their constructor or externally * using {@link #addNode(GTreeNode)} or {@link #setChildren(List)}. For large trees, subclasses * should instead extend {@link GTreeLazyNode} or {@link GTreeSlowLoadingNode} + *

+ * All methods in this class that mutate the children node must perform that operation in + * the swing thread. */ public abstract class GTreeNode extends CoreGTreeNode implements Comparable { private static AtomicLong NEXT_ID = new AtomicLong(); @@ -85,9 +88,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable doAddNode(node)); } /** @@ -95,11 +96,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable nodes) { - for (GTreeNode node : nodes) { - node.setParent(this); - } - children().addAll(nodes); - fireNodeStructureChanged(this); + Swing.runNow(() -> doAddNodes(nodes)); } /** @@ -108,9 +105,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable doAddNode(index, node)); } /** @@ -216,17 +211,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable children = children(); - if (children != null) { - for (GTreeNode child : children) { - child.dispose(); - } - children().clear(); - fireNodeStructureChanged(this); - } + Swing.runNow(() -> doSetChildrenAndFireEvent(null)); } /** @@ -234,14 +219,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable children = children(); - if (children.remove(node)) { - node.setParent(null); - fireNodeRemoved(this, node); - } + Swing.runNow(() -> doRemoveNode(node)); } /** @@ -249,8 +227,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable childList) { - doSetChildren(childList); - fireNodeStructureChanged(this); + Swing.runNow(() -> doSetChildrenAndFireEvent(childList)); } /** @@ -430,11 +407,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable tree.getModel().fireNodeStructureChanged(node)); - tree.refilterLater(); - } + Swing.runNow(() -> doFireNodeStructureChanged()); } /** @@ -443,28 +416,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable tree.getModel().fireNodeDataChanged(parentNode, node)); - tree.refilterLater(); - } - } - - protected void fireNodeAdded(GTreeNode parentNode, GTreeNode newNode) { - GTree tree = getTree(); - if (tree != null) { - Swing.runNow(() -> tree.getModel().fireNodeAdded(parentNode, newNode)); - tree.refilterLater(newNode); - } - } - - protected void fireNodeRemoved(GTreeNode parentNode, GTreeNode removedNode) { - - GTree tree = getTree(); - if (tree != null) { - Swing.runNow( - () -> tree.getModel().fireNodeRemoved(parentNode, removedNode)); - } + Swing.runNow(() -> doFireNodeChanged()); } private GTreeNode[] getPathToRoot(GTreeNode node, int depth) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeSlowLoadingNode.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeSlowLoadingNode.java index 73dbb0accb..85683f44ba 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeSlowLoadingNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeSlowLoadingNode.java @@ -37,8 +37,7 @@ public abstract class GTreeSlowLoadingNode extends GTreeLazyNode { * @return the list of children for this node. * @throws CancelledException if the monitor is cancelled */ - public abstract List generateChildren(TaskMonitor monitor) - throws CancelledException; + public abstract List generateChildren(TaskMonitor monitor) throws CancelledException; @Override protected final List generateChildren() { 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 f8f8497f60..a147e89f77 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 @@ -25,11 +25,12 @@ import javax.swing.tree.TreePath; import docking.widgets.tree.GTree; import docking.widgets.tree.GTreeNode; +import ghidra.util.Swing; import ghidra.util.SystemUtilities; public class GTreeModel implements TreeModel { - private GTreeNode root; + private volatile GTreeNode root; private List listeners = new ArrayList(); private boolean isFiringNodeStructureChanged; private volatile boolean eventsEnabled = true; @@ -76,8 +77,14 @@ public class GTreeModel implements TreeModel { return gTreeParent.getChild(index); } catch (IndexOutOfBoundsException e) { - // children must have be changed outside of swing thread, should get another event - // to fix things up, so just return an in-progress node + // 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 + // 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 + // scheduling a node structure changed event to reset the JTree. + Swing.runLater(() -> fireNodeStructureChanged((GTreeNode) parent)); return new InProgressGTreeNode(); } } @@ -121,9 +128,10 @@ public class GTreeModel implements TreeModel { } if (node != changedNode) { node.setChildren(null); + return; // the previous call will generate the proper event, so bail } - TreeModelEvent event = new TreeModelEvent(this, changedNode.getTreePath()); + TreeModelEvent event = new TreeModelEvent(this, node.getTreePath()); for (TreeModelListener listener : listeners) { listener.treeStructureChanged(event); } @@ -137,43 +145,23 @@ public class GTreeModel implements TreeModel { if (!eventsEnabled) { return; } - SystemUtilities.runIfSwingOrPostSwingLater(new Runnable() { - @Override - public void run() { - GTreeNode rootNode = root; - if (rootNode != null) { - fireNodeStructureChanged(root); - } + Swing.runIfSwingOrRunLater(() -> { + GTreeNode rootNode = root; + if (rootNode != null) { + fireNodeStructureChanged(rootNode); } }); } - public void fireNodeDataChanged(final GTreeNode parentNode, final GTreeNode changedNode) { + public void fireNodeDataChanged(GTreeNode changedNode) { if (!eventsEnabled) { return; } SystemUtilities.assertThisIsTheSwingThread( "GTreeModel.fireNodeDataChanged() must be " + "called from the AWT thread"); - TreeModelEvent event; - if (parentNode == null) { // special case when root node changes. - event = new TreeModelEvent(this, root.getTreePath(), null, null); - } - else { - GTreeNode node = convertToViewNode(changedNode); - if (node == null) { - return; - } + TreeModelEvent event = getChangedNodeEvent(changedNode); - int indexInParent = node.getIndexInParent(); - if (indexInParent < 0) { - return; - } - event = - new TreeModelEvent(this, node.getParent().getTreePath(), - new int[] { indexInParent }, - new Object[] { changedNode }); - } for (TreeModelListener listener : listeners) { listener.treeNodesChanged(event); } @@ -186,33 +174,49 @@ public class GTreeModel implements TreeModel { SystemUtilities.assertThisIsTheSwingThread( "GTreeModel.fireNodeAdded() must be " + "called from the AWT thread"); - GTreeNode node = convertToViewNode(parentNode); - if (node == null) { + GTreeNode parent = convertToViewNode(parentNode); + if (parent == null) { // it will be null if filtered out return; } - TreeModelEvent event = new TreeModelEvent(this, node.getTreePath()); + int index = parent.getIndexOfChild(newNode); + if (index < 0) { + // the index will be -1 if filtered out + return; + } + + TreeModelEvent event = new TreeModelEvent(this, parent.getTreePath(), new int[] { index }, + new Object[] { newNode }); for (TreeModelListener listener : listeners) { - listener.treeStructureChanged(event); + listener.treeNodesInserted(event); } } - public void fireNodeRemoved(final GTreeNode parentNode, final GTreeNode removedNode) { + public void fireNodeRemoved(GTreeNode parentNode, GTreeNode removedNode, int index) { SystemUtilities.assertThisIsTheSwingThread( "GTreeModel.fireNodeRemoved() must be " + "called from the AWT thread"); - GTreeNode node = convertToViewNode(parentNode); - if (node == null) { + GTreeNode parent = convertToViewNode(parentNode); + if (parent == null) { // will be null if filtered out return; } - if (node != parentNode) { - node.removeNode(removedNode); + + // if filtered, remove filtered node + if (parent != parentNode) { + index = removeFromFiltered(parent, removedNode); + return; // the above call will generate the event for the filtered node } - TreeModelEvent event = new TreeModelEvent(this, node.getTreePath()); + if (index < 0) { // will be -1 if filtered out + return; + } + + TreeModelEvent event = new TreeModelEvent(this, parent.getTreePath(), new int[] { index }, + new Object[] { removedNode }); + for (TreeModelListener listener : listeners) { - listener.treeStructureChanged(event); + listener.treeNodesRemoved(event); } } @@ -224,6 +228,27 @@ public class GTreeModel implements TreeModel { eventsEnabled = b; } + private TreeModelEvent getChangedNodeEvent(GTreeNode changedNode) { + GTreeNode parentNode = changedNode.getParent(); + if (parentNode == null) { // tree requires different event form when it is the root that changes + return new TreeModelEvent(this, root.getTreePath(), null, null); + } + + GTreeNode node = convertToViewNode(changedNode); + if (node == null) { + return null; + } + + int indexInParent = node.getIndexInParent(); + if (indexInParent < 0) { + return null; + } + + return new TreeModelEvent(this, node.getParent().getTreePath(), new int[] { indexInParent }, + new Object[] { changedNode }); + + } + private GTreeNode convertToViewNode(GTreeNode node) { if (node.getRoot() == root) { return node; @@ -234,4 +259,12 @@ public class GTreeModel implements TreeModel { } return null; } + + private int removeFromFiltered(GTreeNode parent, GTreeNode removedNode) { + int index = parent.getIndexOfChild(removedNode); + if (index >= 0) { + parent.removeNode(removedNode); + } + return index; + } } diff --git a/Ghidra/Framework/Docking/src/test/java/docking/widgets/tree/GTreeNodeTest.java b/Ghidra/Framework/Docking/src/test/java/docking/widgets/tree/GTreeNodeTest.java index 3db7274b2d..3f684871ff 100644 --- a/Ghidra/Framework/Docking/src/test/java/docking/widgets/tree/GTreeNodeTest.java +++ b/Ghidra/Framework/Docking/src/test/java/docking/widgets/tree/GTreeNodeTest.java @@ -285,7 +285,7 @@ public class GTreeNodeTest { assertEquals(1, events.size()); TestEvent event = events.get(0); assertEquals(EventType.NODE_REMOVED, event.type); - assertEquals(root, event.node); + assertEquals(root, event.parent); } @Test @@ -432,23 +432,23 @@ public class GTreeNodeTest { } @Override - public void fireNodeStructureChanged(GTreeNode node) { - events.add(new TestEvent(EventType.STRUCTURE_CHANGED, null, node, -1)); + public void doFireNodeStructureChanged() { + events.add(new TestEvent(EventType.STRUCTURE_CHANGED, null, this, -1)); } @Override - public void fireNodeChanged(GTreeNode parent, GTreeNode node) { - events.add(new TestEvent(EventType.NODE_CHANGED, parent, node, -1)); + public void doFireNodeChanged() { + events.add(new TestEvent(EventType.NODE_CHANGED, getParent(), this, -1)); } @Override - protected void fireNodeAdded(GTreeNode parentNode, GTreeNode newNode) { - events.add(new TestEvent(EventType.NODE_ADDED, parentNode, newNode, -1)); + protected void doFireNodeAdded(GTreeNode newNode) { + events.add(new TestEvent(EventType.NODE_ADDED, this, newNode, -1)); } @Override - protected void fireNodeRemoved(GTreeNode parentNode, GTreeNode removedNode) { - events.add(new TestEvent(EventType.NODE_REMOVED, null, parentNode, -1)); + protected void doFireNodeRemoved(GTreeNode removedNode, int index) { + events.add(new TestEvent(EventType.NODE_REMOVED, this, removedNode, -1)); } }