Merge remote-tracking branch 'origin/GT-3332_GTree_fixes'

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/CategoryNode.java

@@ -91,6 +91,10 @@ public class CategoryNode extends DataTypeTreeNode {
 		return -1; // CategoryNodes are always come before ****everything else****
 	}
 
+	@Override
+	public int hashCode() {
+		return name.hashCode();
+	}
 	/**
 	 * @see java.lang.Object#equals(java.lang.Object)
 	 */
@@ -164,8 +168,14 @@ public class CategoryNode extends DataTypeTreeNode {
 		}
 
 		CategoryNode node = new CategoryNode(newCategory, filterState);
-		List<GTreeNode> allChildrenList = getChildren();
-		int index = Collections.binarySearch(allChildrenList, node);
+		List<GTreeNode> children = getChildren();
+		int index = Collections.binarySearch(children, node);
+		if (index >= 0) {
+			// if a node with that name exists, then we don't need to add one for the new category
+			if (node.getName().equals(children.get(index).getName())) {
+				return;
+			}
+		}
 		if (index < 0) {
 			index = -index - 1;
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/util/DataTypeTreeDeleteTask.java

@@ -31,13 +31,17 @@ import ghidra.util.task.TaskMonitor;
 
 public class DataTypeTreeDeleteTask extends Task {
 
+	// if the total number of nodes is small, we won't need to collapse the tree before deleting
+	// the nodes to avoid excess tree events
+	private static final int NODE_COUNT_FOR_COLLAPSING_TREE = 100;
 	private Map<ArchiveNode, List<GTreeNode>> nodesByArchive;
 	private DataTypeManagerPlugin plugin;
+	private int nodeCount;
 
 	public DataTypeTreeDeleteTask(DataTypeManagerPlugin plugin, List<GTreeNode> nodes) {
 		super("Delete Nodes", true, true, true);
 		this.plugin = plugin;
-
+		nodeCount = nodes.size();
 		nodes = filterList(nodes);
 
 		nodesByArchive = groupNodeByArchive(nodes);
@@ -105,7 +109,9 @@ public class DataTypeTreeDeleteTask extends Task {
 		DataTypeArchiveGTree tree = provider.getGTree();
 		GTreeState treeState = tree.getTreeState();
 		try {
+			if (nodeCount > NODE_COUNT_FOR_COLLAPSING_TREE) {
 				collapseArchives(tree);
+			}
 
 			Set<Entry<ArchiveNode, List<GTreeNode>>> entries = nodesByArchive.entrySet();
 			for (Entry<ArchiveNode, List<GTreeNode>> entry : entries) {

Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeNode.java

@@ -16,7 +16,6 @@
 package docking.widgets.tree;
 
 import java.util.*;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Stream;
 
 import javax.swing.Icon;
@@ -36,15 +35,42 @@ import util.CollectionUtils;
  * <P>
  * All methods in this class that mutate the children node must perform that operation in
  * the swing thread.
+ * <P>
+ * To create a simple GTreeNode where nodes will be added immediately
+ * using the addNode() methods, simply extend this class and implement the following methods:
+ * <ul>
+ * 		<li>getName()</li>
+ * 		<li>getToolTip()</li>
+ * 		<li>isLeaf()</li>
+ * 		<li>getIcon()</li>
+ * </ul>
+ *
+ * <a name="usage"></a>Usage Notes:
+ * <ul>
+ * 	<li>The <b><tt>equals()</tt></b> method:  The <tt>GTree</tt> has the ability to remember expanded and
+ *      selected states.  This will only work if the nodes in the saved state can be matched
+ *      with the nodes in the <tt>GTree</tt>.  Java will do this by using the <tt>equals()</tt> method.
+ *      There is a potential problem with this usage.  If nodes within the <tt>GTree</tt> get rebuilt (
+ *      i.e., new nodes are created), then, by default, the expanded and selected state
+ *      feature will be unable to find the correct nodes, since the default <tt>equals()</tt>
+ *      method on <tt>GTreeNode</tt> performs a comparison based upon instances.  To fix this problem,
+ *      the {@link #equals()} method has been implemented such that nodes are considered equal if they have
+ *      the same name. The {@link #hashCode()} method will return the hash of the name.  The name
+ *      attribute was chosen because it should be the most unique and descriptive piece of information
+ *      available in a generic GTreeNode.
+ *      <p><br>
+ *      <p>
+ *      There are two situations where the {@link #equals(Object)} and {@link #hashCode()} using the
+ *      name are insufficient.  One is if your tree implementation allows nodes with the same name
+ *      with the same parent.  The other possible situation is if your nodes can change their name, 
+ *      which may confuse the tree.  If either of these situations apply, just override the 
+ *      {@link #equals(Object)} and {@link #hashCode()} methods to make them more robust.
+ *      <p><br>
+ *   </li>
+ * </ul>
  */
+
 public abstract class GTreeNode extends CoreGTreeNode implements Comparable<GTreeNode> {
-	private static AtomicLong NEXT_ID = new AtomicLong();
-
-	private final long id;
-
-	protected GTreeNode() {
-		id = NEXT_ID.incrementAndGet();
-	}
 
 	@Override
 	protected List<GTreeNode> generateChildren() {
@@ -166,10 +192,15 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable<GTre
 	}
 
 	/**
-	 * Returns the total number of leaf nodes in the subtree from this node
+	 * Returns the total number of leaf nodes in the subtree from this node.  Note that if any
+	 * nodes are "lazy" (see {@link GTreeLazyNode}) and not currently loaded, then it will be
+	 * considered as a leaf and return 1. 
 	 * @return the total number of leaf nodes in the subtree from this node
 	 */
 	public int getLeafCount() {
+		if (isLeaf() || !isLoaded()) {
+			return 1;
+		}
 		int count = 0;
 		for (GTreeNode node : children()) {
 			count += node.getLeafCount();
@@ -349,7 +380,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable<GTre
 
 	@Override
 	public int hashCode() {
-		return (int) id;
+		return getName().hashCode();
 	}
 
 	@Override
@@ -364,7 +395,7 @@ public abstract class GTreeNode extends CoreGTreeNode implements Comparable<GTre
 			return false;
 		}
 		GTreeNode other = (GTreeNode) obj;
-		return id == other.id;
+		return getName().equals(other.getName());
 	}
 
 	/**

Ghidra/Framework/Docking/src/test/java/docking/widgets/tree/GTreeNodeTest.java

@@ -236,6 +236,16 @@ public class GTreeNodeTest {
 		assertEquals(4, root.getLeafCount());  // the total of all its children
 	}
 
+	@Test
+	public void testGetLeafCountOnLazyNodes() throws CancelledException {
+		LazyGTestNode node = new LazyGTestNode("Test", 3);
+		assertEquals(1, node.getLeafCount());
+		node.getChildren();// force load
+		assertEquals(3, node.getLeafCount());
+		node.loadAll(TaskMonitor.DUMMY);
+		assertEquals(27, node.getLeafCount());
+	}
+
 	@Test
 	public void testGetIndexInParent() {
 		assertEquals(0, node0.getIndexInParent());
@@ -406,6 +416,24 @@ public class GTreeNodeTest {
 		assertEquals(node1_0, collect.get(6));
 	}
 
+	@Test
+	public void testEqualsAndHashCode() {
+		GTreeNode nodeA = new TestNode("AAA");
+		GTreeNode nodeB = new TestNode("BBB");
+		GTreeNode nodeAA = new TestNode("AAA");
+		assertEquals(nodeA, nodeAA);
+		assertNotEquals(nodeA, nodeB);
+		assertEquals(nodeA.hashCode(), nodeAA.hashCode());
+		assertNotEquals(nodeA.hashCode(), nodeB.hashCode());
+	}
+
+	@Test
+	public void testCloneEquals() throws CloneNotSupportedException {
+		GTreeNode nodeA = new TestNode("AAA");
+		assertEquals(nodeA, nodeA.clone());
+		assertEquals(nodeA.hashCode(), nodeA.clone().hashCode());
+	}
+
 	private class TestFilter implements GTreeFilter {
 
 		private String text;