diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java b/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java index eef6d26477..7755e4cd2a 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java @@ -100,8 +100,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder WeakDataStructureFactory.createSingleThreadAccessWeakSet(); // The update should happen fairly quickly, as it is what rebuilds the window layout - private SwingUpdateManager rebuildUpdater = - new SwingUpdateManager(100, 750, "Docking Windows Updater", this::doUpdate); + private SwingUpdateManager rebuildUpdater = new SwingUpdateManager(100, 750, this::doUpdate); private boolean isVisible; private boolean isDocking; private boolean hasStatusBar; @@ -820,16 +819,9 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder actionToGuiMapper.dispose(); root.dispose(); + placeholderManager.disposePlaceholders(); - if (focusedPlaceholder != null) { - focusedPlaceholder.dispose(); - } - - if (nextFocusedPlaceholder != null) { - nextFocusedPlaceholder.dispose(); - } - setNextFocusPlaceholder(null); removeInstance(this); root = null; @@ -1774,7 +1766,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder This method seeks to accomplish 2 goals: 1) find a suitable component over which to center, and 2) ensure that the chosen component is in the parent hierarchy - + */ Component bestComponent = centeredOnComponent; if (SwingUtilities.isDescendingFrom(parent, bestComponent)) { @@ -1808,7 +1800,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder /* Note: Which window should be the parent of the dialog when the user does not specify? - + Some use cases; a dialog is shown from: 1) A toolbar action 2) A component provider's code @@ -1816,7 +1808,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder 4) A background thread 5) The help window 6) A modal password dialog appears over the splash screen - + It seems like the parent should be the active window for 1-2. Case 3 should probably use the window of the dialog provider. Case 4 should probably use the main tool frame, since the user may be @@ -1824,12 +1816,12 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder active window, we can default to the tool's frame. Case 5 should use the help window. Case 6 should use the splash screen as the parent. - + We have not yet solidified how we should parent. This documentation is meant to move us towards clarity as we find Use Cases that don't make sense. (Once we finalize our understanding, we should update the javadoc to list exactly where the given Dialog Component will be shown.) - + Use Case A -The user presses an action on a toolbar from a window on screen 1, while the main tool frame is on screen 2. We want the popup window to appear on screen @@ -1841,8 +1833,8 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder -modal - Java handles this correctly, allowing the new dialog to be used -non-modal - Java prevents the non-modal from being editing if not parented correctly - - + + For now, the easiest mental model to use is to always prefer the active window so that a dialog will appear in the user's view. If we find a case where this is not desired, then document it here. diff --git a/Ghidra/Framework/Docking/src/main/java/docking/SplitNode.java b/Ghidra/Framework/Docking/src/main/java/docking/SplitNode.java index caaba187e2..bf731f510b 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/SplitNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/SplitNode.java @@ -15,6 +15,8 @@ */ package docking; +import ghidra.util.Msg; + import java.awt.Dimension; import java.util.ArrayList; import java.util.List; @@ -23,8 +25,6 @@ import javax.swing.*; import org.jdom.Element; -import ghidra.util.Msg; - /** * Node for managing a JSplitPane view of two component trees. */ @@ -213,17 +213,6 @@ class SplitNode extends Node { String getDescription() { return "Split Node"; } - - @Override - void dispose() { - if (child1 != null) { - child1.dispose(); - } - - if (child2 != null) { - child2.dispose(); - } - } } //================================================================================================== diff --git a/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java b/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java index bb21613f1c..9e93d2d521 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java @@ -35,7 +35,7 @@ public class WindowActionManager { private ComponentPlaceholder placeHolderForScheduledActionUpdate; private Runnable updateActionsRunnable = () -> processContextChanged(); private SwingUpdateManager updateManager = - new SwingUpdateManager(500, 500, "Context Updater", updateActionsRunnable); + new SwingUpdateManager(500, 500, "Context Update Manager", updateActionsRunnable); public WindowActionManager(WindowNode node, MenuHandler menuBarHandler, DockingWindowManager winMgr, MenuGroupMap menuGroupMap) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java index e574a41444..2aba07f810 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java @@ -144,10 +144,6 @@ class ThreadedTableModelUpdateMgr { * is performed. It also is called if too many add/removes have been accumulated. */ void reload() { - if (isDisposed()) { - return; - } - synchronized (addRemoveUpdater) { cancelAllJobs(); runJob(new LoadJob<>(model, monitor)); @@ -155,10 +151,6 @@ class ThreadedTableModelUpdateMgr { } void reloadSpecificData(List data) { - if (isDisposed()) { - return; - } - synchronized (addRemoveUpdater) { cancelAllJobs(); TableData tableData = TableData.createFullDataset(data); @@ -183,10 +175,6 @@ class ThreadedTableModelUpdateMgr { * to be re-sorted. */ void sort(TableSortingContext sortingContext, boolean forceSort) { - if (isDisposed()) { - return; - } - synchronized (addRemoveUpdater) { if (currentJob != null && pendingJob == null && currentJob.requestSort(sortingContext, forceSort)) { @@ -215,10 +203,6 @@ class ThreadedTableModelUpdateMgr { * start a thread to do the work. */ void filter() { - if (isDisposed()) { - return; - } - synchronized (addRemoveUpdater) { if (currentJob != null && pendingJob == null && currentJob.requestFilter()) { return; @@ -242,10 +226,6 @@ class ThreadedTableModelUpdateMgr { * @param item the add/remove item to process. */ void addRemove(AddRemoveListItem item) { - if (isDisposed()) { - return; - } - synchronized (addRemoveUpdater) { if (pendingJob != null) { pendingJob.addRemove(item, getMaxAddRemoveCount()); @@ -347,11 +327,6 @@ class ThreadedTableModelUpdateMgr { } } - private boolean isDisposed() { - // the updater knows when it is disposed; use the updater to avoid keeping our own variable - return addRemoveUpdater.isDisposed(); - } - /** * Kicks the swing update manager to immediately process any accumulated add/removes. */ diff --git a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java b/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java index 5d9e52475b..e069eabb60 100644 --- a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java +++ b/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java @@ -1113,7 +1113,7 @@ public abstract class AbstractGenericTest extends AbstractGTest { * * @param r the runnable code snippet */ - public static void runSwingLater(Runnable r) { + public void runSwingLater(Runnable r) { runSwing(r, false); } @@ -1603,34 +1603,23 @@ public abstract class AbstractGenericTest extends AbstractGTest { (WeakSet) getInstanceField("instances", SwingUpdateManager.class); for (AbstractSwingUpdateManager manager : s) { - - // Ignore update managers that fire often and are unlikely to affect tests. - // (Hardcoding this is brittle, but these update managers live in lower packages - // and a larger refactoring didn't seem worth the cost at this time.) - String name = manager.getName(); - if (name.equals("Context Updater") || - name.equals("Docking Windows Updater")) { - continue; - } - set.add(manager); } }); /* - // performance debug long start = System.nanoTime(); - boolean wasEverBusy = waitForSwing(set); + boolean wasEverBusy = waitForSwing(set, true); long end = System.nanoTime(); Msg.out("\twaitForSwing() - " + TimeUnit.MILLISECONDS.convert(end - start, TimeUnit.NANOSECONDS)); */ - boolean wasEverBusy = waitForSwing(set); + boolean wasEverBusy = waitForSwing(set, true); return wasEverBusy; } - private static boolean waitForSwing(Set managers) { + private static boolean waitForSwing(Set managers, boolean flush) { // Note: not sure how long is too long to wait for the Swing thread and update managers // to finish. This is usually less than a second. We have seen a degenerate @@ -1640,7 +1629,7 @@ public abstract class AbstractGenericTest extends AbstractGTest { int totalTime = 0; // flush all managers up front to get them started before we check them - flushAllManagers(managers); + flushAllManagers(managers, flush); boolean wasEverBusy = false; boolean keepGoing = true; @@ -1659,13 +1648,14 @@ public abstract class AbstractGenericTest extends AbstractGTest { // Msg.out("busy manager: " + manager.toStringDebug()); - doFlush(manager); + doFlush(flush, manager); - keepGoing = true; // true, since we had a busy signal - wasEverBusy = true; boolean isBusy = true; while (isBusy) { + keepGoing = true; // true, since we had a busy signal + wasEverBusy = true; + totalTime += sleep(DEFAULT_WAIT_DELAY); if (totalTime >= MAX_SWING_TIMEOUT) { // eject! @@ -1686,7 +1676,7 @@ public abstract class AbstractGenericTest extends AbstractGTest { return wasEverBusy; } - private static void flushAllManagers(Set managers) { + private static void flushAllManagers(Set managers, boolean flush) { // // Some update managers will make an update that causes another manager to schedule an @@ -1703,26 +1693,21 @@ public abstract class AbstractGenericTest extends AbstractGTest { // which would be 2 int n = 3; for (int i = 0; i < n; i++) { - boolean didFlush = false; for (AbstractSwingUpdateManager manager : managers) { - didFlush = doFlush(manager); - } - - if (!didFlush) { - return; // skip extra waiting when no work was done + doFlush(flush, manager); } } } - private static boolean doFlush(AbstractSwingUpdateManager manager) { - - if (!manager.hasPendingUpdates()) { - return false; // skip the yield call, as it consumes time + private static void doFlush(boolean doFlush, AbstractSwingUpdateManager manager) { + if (!doFlush) { + return; } - runSwingLater(() -> manager.flush()); + runSwing(() -> { + manager.flush(); + }, false); yieldToSwing(); - return true; } /** diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/AbstractSwingUpdateManager.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/AbstractSwingUpdateManager.java index ec1dd8a950..e1346873bd 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/AbstractSwingUpdateManager.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/AbstractSwingUpdateManager.java @@ -56,6 +56,7 @@ public abstract class AbstractSwingUpdateManager { public static final int DEFAULT_MAX_DELAY = 30000; protected static final int MIN_DELAY_FLOOR = 10; protected static final int DEFAULT_MIN_DELAY = 250; + protected static final String DEFAULT_NAME = AbstractSwingUpdateManager.class.getSimpleName(); private static final WeakSet instances = WeakDataStructureFactory.createCopyOnReadWeakSet(); @@ -106,7 +107,7 @@ public abstract class AbstractSwingUpdateManager { * @param maxDelay the maximum amount of time to wait between gui updates. */ protected AbstractSwingUpdateManager(int minDelay, int maxDelay) { - this(minDelay, maxDelay, null); + this(minDelay, maxDelay, DEFAULT_NAME); } /** @@ -123,7 +124,7 @@ public abstract class AbstractSwingUpdateManager { protected AbstractSwingUpdateManager(int minDelay, int maxDelay, String name) { this.maxDelay = maxDelay; - this.name = name != null ? name : getClass().getSimpleName(); + this.name = name; recordInception(); this.minDelay = Math.max(MIN_DELAY_FLOOR, minDelay); @@ -182,15 +183,6 @@ public abstract class AbstractSwingUpdateManager { Swing.runNow(this::checkForWork); } - /** - * Returns the name given to this update manager. If no name was provided at construction, - * then the class name is used. - * @return the name - */ - public String getName() { - return name; - } - /** * Causes this run manager to run if it has a pending update */ @@ -371,10 +363,12 @@ public abstract class AbstractSwingUpdateManager { StackTraceElement[] trace = t.getStackTrace(); String classInfo = trace[0].toString(); + /* // debug source of creation Throwable filtered = ReflectionUtilities.filterJavaThrowable(t); String string = ReflectionUtilities.stackTraceToString(filtered); classInfo = classInfo + "\n\tfrom:\n\n" + string; + */ return classInfo; } diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/BufferedSwingRunner.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/BufferedSwingRunner.java index 2d87251c8b..00bb18009d 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/BufferedSwingRunner.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/BufferedSwingRunner.java @@ -36,11 +36,11 @@ public class BufferedSwingRunner extends AbstractSwingUpdateManager { * @param maxDelay the maximum amount of time to wait between gui updates. */ public BufferedSwingRunner(int minDelay, int maxDelay) { - super(minDelay, maxDelay, null); + super(minDelay, maxDelay, DEFAULT_NAME); } public BufferedSwingRunner() { - super(DEFAULT_MIN_DELAY, DEFAULT_MAX_DELAY, null); + super(DEFAULT_MIN_DELAY, DEFAULT_MAX_DELAY, DEFAULT_NAME); } @Override diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java index affcc432f6..5b4293e5ec 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java @@ -82,7 +82,8 @@ public class SwingUpdateManager extends AbstractSwingUpdateManager { * @param r the runnable that performs the client work. */ public SwingUpdateManager(int minDelay, int maxDelay, Runnable r) { - this(minDelay, maxDelay, null, r); + super(minDelay, maxDelay, DEFAULT_NAME); + this.clientRunnable = r; } /**