GT-3542 - Symbol Tree - fixed flashing of the symbol tree while

analyzing; fixed bad progress of Decompiler Parameter ID analyzer; fixed
odd progress updating of ConcurrentQ

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/misc/MyProgramChangesDisplayPlugin.java

@@ -255,7 +255,7 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 	 */
 	private void updateChangeMarkers() {
 
-		Swing.assertThisIsTheSwingThread(
+		Swing.assertSwingThread(
 			"Change markers must be manipulated on the Swing thread");
 
 		if (currentProgram == null) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java

@@ -76,6 +76,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 	private List<GTreeTask> bufferedTasks = new ArrayList<>();
 	private SwingUpdateManager domainChangeUpdateManager = new SwingUpdateManager(1000,
 		SwingUpdateManager.DEFAULT_MAX_DELAY, "Symbol Tree Provider", () -> {
+
 			if (bufferedTasks.isEmpty()) {
 				return;
 			}
@@ -404,7 +405,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 	private void addTask(GTreeTask task) {
 		// Note: if we want to call this method from off the Swing thread, then we have to
 		//       synchronize on the list that we are adding to here.
-		Swing.assertThisIsTheSwingThread(
+		Swing.assertSwingThread(
 			"Adding tasks must be done on the Swing thread," +
 				"since they are put into a list that is processed on the Swing thread. ");
 
@@ -535,7 +536,10 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 		public void run(TaskMonitor monitor) throws CancelledException {
 			TreePath[] selectionPaths = tree.getSelectionPaths();
 			doRun(monitor);
-			tree.setSelectionPaths(selectionPaths);
+
+			if (selectionPaths.length != 0) {
+				tree.setSelectionPaths(selectionPaths);
+			}
 		}
 
 		@Override
@@ -612,8 +616,9 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 
 		@Override
 		public void runBulk(TaskMonitor monitor) throws CancelledException {
+
 			if (tasks.size() > MAX_TASK_COUNT) {
-				SystemUtilities.runSwingLater(() -> rebuildTree());
+				Swing.runLater(() -> rebuildTree());
 				return;
 			}
 
@@ -628,7 +633,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 		@Override
 		public void domainObjectChanged(DomainObjectChangedEvent event) {
 
-			if (!tool.isVisible(SymbolTreeProvider.this)) {
+			if (ignoreEvents()) {
 				return;
 			}
 
@@ -686,5 +691,27 @@ public class SymbolTreeProvider extends ComponentProviderAdapter {
 			}
 
 		}
+
+		private boolean ignoreEvents() {
+			if (!isVisible()) {
+				return true;
+			}
+			return treeIsCollapsed();
+		}
+
+		private boolean treeIsCollapsed() {
+			// note: the root's children are visible by default
+			GTreeNode root = tree.getViewRoot();
+			if (!root.isExpanded()) {
+				return true;
+			}
+			List<GTreeNode> children = root.getChildren();
+			for (GTreeNode node : children) {
+				if (node.isExpanded()) {
+					return false;
+				}
+			}
+			return true;
+		}
 	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/cmd/function/DecompilerParameterIdCmd.java

@@ -50,9 +50,10 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 	private boolean commitVoidReturn;
 	private int decompilerTimeoutSecs;
 
-	public DecompilerParameterIdCmd(AddressSetView entries, SourceType sourceTypeClearLevel,
+	public DecompilerParameterIdCmd(String name, AddressSetView entries,
+			SourceType sourceTypeClearLevel,
 			boolean commitDataTypes, boolean commitVoidReturn, int decompilerTimeoutSecs) {
-		super("Create Function Stack Variables", true, true, false);
+		super(name, true, true, false);
 		entryPoints.add(entries);
 		this.sourceTypeClearLevel = sourceTypeClearLevel;
 		this.commitDataTypes = commitDataTypes;
@@ -65,13 +66,13 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 		program = (Program) obj;
 
 		CachingPool<DecompInterface> decompilerPool =
-			new CachingPool<DecompInterface>(new DecompilerFactory());
+			new CachingPool<>(new DecompilerFactory());
 		QRunnable<Address> runnable = new ParallelDecompileRunnable(decompilerPool);
 
 		ConcurrentGraphQ<Address> queue = null;
 
 		try {
-			monitor.setMessage("Analyzing Call Hierarchy...");
+			monitor.setMessage(getName() + " - creating dependency graph...");
 			AcyclicCallGraphBuilder builder =
 				new AcyclicCallGraphBuilder(program, entryPoints, true);
 			AbstractDependencyGraph<Address> graph = builder.getDependencyGraph(monitor);
@@ -80,10 +81,11 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 			}
 
 			GThreadPool pool = AutoAnalysisManager.getSharedAnalsysThreadPool();
-			queue = new ConcurrentGraphQ<Address>(runnable, graph, pool, monitor);
+			queue = new ConcurrentGraphQ<>(runnable, graph, pool, monitor);
 			resetFunctionSourceTypes(graph.getValues());
 
-			monitor.setMessage("Analyzing...");
+			monitor.setMessage(getName() + " - analyzing...");
+			monitor.initialize(graph.size());
 
 			queue.execute();
 		}
@@ -114,7 +116,8 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 	 */
 	private boolean funcIsExternalGlue(Function func) {
 		String blockName = program.getMemory().getBlock(func.getEntryPoint()).getName();
-		return (blockName.equals("EXTERNAL") || blockName.equals(".plt") || blockName.equals("__stub_helper"));
+		return (blockName.equals("EXTERNAL") || blockName.equals(".plt") ||
+			blockName.equals("__stub_helper"));
 	}
 
 	private void resetFunctionSourceTypes(Set<Address> set) {
@@ -147,10 +150,8 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 				}
 			}
 			catch (InvalidInputException e) {
-				Msg.warn(
+				Msg.warn(this,
-					this,
+					"Error changing signature SourceType on--" + func.getName(), e);
-					"Error changing signature SourceType (should not since Input is the same) on--" +
-						func.getName(), e);
 			}
 		}
 	}
@@ -248,10 +249,10 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 	}
 
 	/**
-	 * Check for consistency of returned results.  Trying to propogate, don't want to propagate garbage.
+	 * Check for consistency of returned results.  Trying to propagate, don't want to propagate garbage.
 	 *  
-	 * @param decompRes
+	 * @param decompRes the decompile result
-	 * @return
+	 * @return true if inconsistent results
 	 */
 	private boolean hasInconsistentResults(DecompileResults decompRes) {
 		HighFunction hfunc = decompRes.getHighFunction();
@@ -382,7 +383,7 @@ public class DecompilerParameterIdCmd extends BackgroundCommand {
 		private void doWork(Function function, DecompInterface decompiler, TaskMonitor monitor)
 				throws CancelledException {
 			monitor.checkCanceled();
-			monitor.setMessage("Decompile " + function.getName());
+			monitor.setMessage(getName() + " - decompile " + function.getName());
 			analyzeFunction(decompiler, function, monitor);
 		}
 

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/analysis/DecompilerCallConventionAnalyzer.java

@@ -37,11 +37,10 @@ import ghidra.util.graph.AbstractDependencyGraph;
 import ghidra.util.task.TaskMonitor;
 
 public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
-	private static final String NAME = "Call Convention Identification";
+	private static final String NAME = "Call Convention ID";
 	private static final String DESCRIPTION =
 		"Uses decompiler to figure out unknown calling conventions.";
 
-	private static final String STD_NAMESPACE = "std";
 	private static final String COULD_NOT_RECOVER_CALLING_CONVENTION =
 		"Could not recover calling convention";
 	private static final String OPTION_NAME_DECOMPILER_TIMEOUT_SECS =
@@ -53,8 +52,6 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 
 	private boolean ignoreBookmarks = false;
 
-	private Program program;
-
 //==================================================================================================
 // Interface Methods
 //==================================================================================================	
@@ -68,11 +65,11 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 
 	@Override
 	public boolean canAnalyze(Program program) {
-		boolean cando = program.getLanguage().supportsPcode();
+		boolean can = program.getLanguage().supportsPcode();
 
 		// for a single entry compiler convention, the convention is always used, so no need to identify it
-		cando &= program.getCompilerSpec().getCallingConventions().length > 1;
+		can &= program.getCompilerSpec().getCallingConventions().length > 1;
-		return cando;
+		return can;
 	}
 
 	@Override
@@ -92,8 +89,6 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 	public boolean added(Program program, AddressSetView set, TaskMonitor monitor, MessageLog log)
 			throws CancelledException {
 
-		this.program = program; // set program for use by ParallelDecompilerCallback
-
 		ignoreBookmarks = set.hasSameAddresses(program.getMemory());
 
 		try {
@@ -128,14 +123,14 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 
 		CachingPool<DecompInterface> decompilerPool =
 			new CachingPool<>(new DecompilerFactory(program));
-		QRunnable<Address> callback = new ParallelDecompilerCallback(decompilerPool);
+		QRunnable<Address> callback = new ParallelDecompilerCallback(decompilerPool, program);
 
 		ConcurrentGraphQ<Address> queue = null;
 
 		monitor.initialize(functionEntries.getNumAddresses());
 
 		try {
-			monitor.setMessage("Analyzing Call Hierarchy...");
+			monitor.setMessage(NAME + " - creating dependency graph...");
 			AcyclicCallGraphBuilder builder =
 				new AcyclicCallGraphBuilder(program, functionEntries, true);
 			AbstractDependencyGraph<Address> graph = builder.getDependencyGraph(monitor);
@@ -146,7 +141,8 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 			GThreadPool pool = AutoAnalysisManager.getSharedAnalsysThreadPool();
 			queue = new ConcurrentGraphQ<>(callback, graph, pool, monitor);
 
-			monitor.setMessage("Analyzing Call Conventions...");
+			monitor.setMessage(NAME + " - analyzing call conventions...");
+			monitor.initialize(graph.size());
 
 			queue.execute();
 		}
@@ -272,9 +268,11 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 	private class ParallelDecompilerCallback implements QRunnable<Address> {
 
 		private CachingPool<DecompInterface> pool;
+		private Program program;
 
-		ParallelDecompilerCallback(CachingPool<DecompInterface> decompilerPool) {
+		ParallelDecompilerCallback(CachingPool<DecompInterface> decompilerPool, Program program) {
 			this.pool = decompilerPool;
+			this.program = program;
 		}
 
 		@Override
@@ -287,10 +285,12 @@ public class DecompilerCallConventionAnalyzer extends AbstractAnalyzer {
 			try {
 				Function function = program.getFunctionManager().getFunctionAt(address);
 
+				monitor.setMessage(getName() + " - decompile " + function.getName());
 				performConventionAnalysis(function, decompiler, monitor);
 			}
 			finally {
 				pool.release(decompiler);
+				monitor.incrementProgress(1);
 			}
 			return;
 		}

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/analysis/DecompilerFunctionAnalyzer.java

@@ -71,7 +71,7 @@ public class DecompilerFunctionAnalyzer extends AbstractAnalyzer {
 
 	@Override
 	public boolean added(Program program, AddressSetView set, TaskMonitor monitor, MessageLog log) {
-		DecompilerParameterIdCmd cmd = new DecompilerParameterIdCmd(set,
+		DecompilerParameterIdCmd cmd = new DecompilerParameterIdCmd(NAME, set,
 			sourceTypeClearLevelOption, commitDataTypesOption, commitVoidReturnOption,
 			decompilerTimeoutSecondsOption);
 		cmd.applyTo(program, monitor);

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

@@ -19,7 +19,6 @@ import javax.swing.JTree;
 import javax.swing.tree.TreePath;
 
 import ghidra.util.SystemUtilities;
-import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 import ghidra.util.worker.PriorityJob;
 
@@ -53,6 +52,9 @@ public abstract class GTreeTask extends PriorityJob {
 	 * values on later calls to getSelectedPaths().  So, to handle that 'feature' of the JTree, we
 	 * need to translate the given path to the equivalent path in the current tree (this code may
 	 * not be needed in all uses of this task, but it protects us from the aforementioned case).
+	 * @param path the path to translate
+	 * @param monitor the monitor
+	 * @return the translated path
 	 */
 	protected TreePath translatePath(TreePath path, TaskMonitor monitor) {
 

Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskRunner.java

@@ -124,7 +124,7 @@ class TaskRunner {
 
 	private void showTaskDialog(WrappingTaskMonitor monitor) {
 
-		Swing.assertThisIsTheSwingThread("Must be on the Swing thread build the Task Dialog");
+		Swing.assertSwingThread("Must be on the Swing thread build the Task Dialog");
 
 		taskDialog = buildTaskDialog(parent, monitor);
 		monitor.setDelegate(taskDialog); // initialize the dialog to the current state of the monitor

Ghidra/Framework/Generic/src/main/java/generic/concurrent/ConcurrentGraphQ.java

@@ -15,16 +15,17 @@
  */
 package generic.concurrent;
 
+import java.util.Set;
+
 import ghidra.util.graph.AbstractDependencyGraph;
 import ghidra.util.task.TaskMonitor;
 
-import java.util.Set;
-
 public class ConcurrentGraphQ<I> {
 	private ConcurrentQ<I, Object> queue;
 	private AbstractDependencyGraph<I> graph;
 
-	public ConcurrentGraphQ(QRunnable<I> runnable, AbstractDependencyGraph<I> graph, GThreadPool pool,
+	public ConcurrentGraphQ(QRunnable<I> runnable, AbstractDependencyGraph<I> graph,
+			GThreadPool pool,
 			TaskMonitor monitor) {
 		this.graph = graph;
 		// @formatter:off
@@ -33,7 +34,7 @@ public class ConcurrentGraphQ<I> {
 			.setThreadPool(pool)			
 			.setMonitor(monitor)
 			.setListener(new MyItemListener())
-			.build(new QRunnableAdapter<I>(runnable));		
+			.build(new QRunnableAdapter<>(runnable));		
 		// @formatter:on
 
 	}

Ghidra/Framework/Generic/src/main/java/generic/concurrent/ConcurrentQ.java

@@ -791,7 +791,17 @@ public class ConcurrentQ<I, R> {
 		@Override
 		public void taskEnded(long id, I Item, long total, long progress) {
 			if (!jobsReportProgress) {
-				if (total != monitor.getMaximum()) {
+				// 
+				// This code works in 2 ways.  The default case is that clients place items on 
+				// the queue.  As the amount of work grows, so too does the max progress value. 
+				// This obviates the need for clients to manager progress.  (The downside to this
+				// is that the progress may keep getting pushed back as it approaches the 
+				// current maximum value.)  The second case is where the client has specified a 
+				// true maximum value.  In that case, this code will not change the maxmimum
+				// (assuming that the client does not put more items into the queue than they 
+				// specified).
+				//
+				if (total > monitor.getMaximum()) {
 					monitor.setMaximum(total);
 				}
 				monitor.setProgress(progress);

Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/EventManager.java

@@ -248,7 +248,7 @@ public class EventManager {
 
 	private void sendEvents() {
 
-		Swing.assertThisIsTheSwingThread("Events must be sent on the Swing thread");
+		Swing.assertSwingThread("Events must be sent on the Swing thread");
 
 		synchronized (eventQ) {
 			currentEvent = eventQ.poll();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/util/AcyclicCallGraphBuilder.java

@@ -91,7 +91,7 @@ public class AcyclicCallGraphBuilder {
 		AbstractDependencyGraph<Address> graph = new DependencyGraph<>();
 		Deque<Address> startPoints = findStartPoints();
 		Set<Address> unprocessed = new TreeSet<>(functionSet); // reliable processing order
-
+		monitor.initialize(unprocessed.size());
 		while (!unprocessed.isEmpty()) {
 			monitor.checkCanceled();
 			Address functionEntry = getNextStartFunction(startPoints, unprocessed);
@@ -123,7 +123,7 @@ public class AcyclicCallGraphBuilder {
 		return startPoints;
 	}
 
-	private void initializeNode(StackNode node, Set<Address> unprocessed) {
+	private void initializeNode(StackNode node) {
 		FunctionManager fmanage = program.getFunctionManager();
 		Function function = fmanage.getFunctionAt(node.address);
 		if (function.isThunk()) {
@@ -163,14 +163,16 @@ public class AcyclicCallGraphBuilder {
 			Address startFunction, TaskMonitor monitor) throws CancelledException {
 		VisitStack stack = new VisitStack(startFunction);
 		StackNode curnode = stack.peek();
-		initializeNode(curnode, unprocessed);
+		initializeNode(curnode);
 		graph.addValue(curnode.address);
 		while (!stack.isEmpty()) {
+			monitor.checkCanceled();
+
 			curnode = stack.peek();
 			if (curnode.nextchild >= curnode.children.length) {		// Node more to children to traverse for this node
 				unprocessed.remove(curnode.address);
+				monitor.incrementProgress(1);
 				stack.pop();
-				monitor.checkCanceled();
 			}
 			else {
 				Address childAddr = curnode.children[curnode.nextchild++];
@@ -178,7 +180,7 @@ public class AcyclicCallGraphBuilder {
 					if (unprocessed.contains(childAddr)) {
 						stack.push(childAddr);
 						StackNode nextnode = stack.peek();
-						initializeNode(nextnode, unprocessed);
+						initializeNode(nextnode);
 						childAddr = nextnode.address;
 						graph.addValue(nextnode.address);
 					}

Ghidra/Framework/Utility/src/main/java/ghidra/util/Swing.java

@@ -84,21 +84,18 @@ public class Swing {
 	}
 
 	/**
-	 * A development/testing time method to make sure the current thread is the swing thread.
+	 * Logs a stack trace if the current calling thread is not the Swing thread
-	 * @param errorMessage The message to display when the assert fails
+	 * @param errorMessage The message to display when not on the Swing thread
+	 * @return true if the calling thread is the Swing thread
 	 */
-	public static void assertThisIsTheSwingThread(String errorMessage) {
+	public static boolean assertSwingThread(String errorMessage) {
-		boolean isProductionMode =
-			!SystemUtilities.isInTestingMode() && !SystemUtilities.isInDevelopmentMode();
-		if (isProductionMode) {
-			return; // squash during production mode
-		}
-
 		if (!isSwingThread()) {
 			Throwable t =
 				ReflectionUtilities.filterJavaThrowable(new AssertException(errorMessage));
-			Msg.error(SystemUtilities.class, errorMessage, t);
+			Msg.error(Swing.class, errorMessage, t);
+			return false;
 		}
+		return true;
 	}
 
 	/**