Merge remote-tracking branch 'origin/GP-1505-dragonmacher-task-dialog-fix'

This commit is contained in:
Ryan Kurtz
2021-11-18 12:26:51 -05:00
2 changed files with 65 additions and 27 deletions
@@ -18,8 +18,7 @@ package ghidra.util.task;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.*;
import javax.swing.SwingUtilities;
import javax.swing.SwingWorker;
@@ -62,7 +61,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
private int taskDialogDelay = 500;
private boolean hasProgress = true;
private T cachedValue;
private SwingWorker<T, Object> worker;
private SwingWorkerImpl worker;
private WorkerTaskMonitor taskMonitor = new WorkerTaskMonitor();
/**
@@ -86,7 +85,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
/**
* Subclasses must implement this method to create the object being managed/cached.
* @param monitor A task monitor that can be used to provide progress information to
* a progress dialog is it is being shown. Implementers should also check the monitor
* a progress dialog is it is being shown. Implementers should also check the monitor
* periodically to check for cancelled. If cancelled, this method should not throw a
* cancelled exception but instead either return null or a partial result. (For example,
* if the object is a list being generated by a search, then it might make sense to return
@@ -97,7 +96,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
/**
* Returns the object that this class is managing/caching. It will return the object if it is
* already created or it will block until the object can be created. If called from the Swing
* already created or it will block until the object can be created. If called from the Swing
* thread, it will also launch a modal progress dialog while waiting for the object to be
* created.
*
@@ -113,7 +112,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
}
addMonitor(monitor);
SwingWorker<T, Object> swingWorker = getWorker();
SwingWorkerImpl swingWorker = getWorker();
if (SwingUtilities.isEventDispatchThread()) {
blockSwingWithProgressDialog(swingWorker);
}
@@ -175,7 +174,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
}
}
private void blockSwingWithProgressDialog(final SwingWorker<T, Object> localWorker) {
private void blockSwingWithProgressDialog(SwingWorkerImpl localWorker) {
if (!localWorker.isDone()) {
TaskDialog dialog = new SwingWorkerTaskDialog(name, hasProgress, localWorker);
@@ -185,7 +184,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
}
}
private synchronized SwingWorker<T, Object> getWorker() {
private synchronized SwingWorkerImpl getWorker() {
if (worker == null) {
worker = new SwingWorkerImpl();
worker.execute();
@@ -220,19 +219,19 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
//==================================================================================================
// Inner Classes
//==================================================================================================
//==================================================================================================
private class SwingWorkerTaskDialog extends TaskDialog {
private SwingWorker<T, Object> taskWorker;
private SwingWorkerImpl taskWorker;
SwingWorkerTaskDialog(String title, boolean showProgress, SwingWorker<T, Object> worker) {
super(title, true, true, showProgress);
SwingWorkerTaskDialog(String title, boolean showProgress, SwingWorkerImpl worker) {
super(title, true, true, showProgress, worker.getFinishedLatch());
this.taskWorker = worker;
}
@Override
// Overridden to allow us to check for the worker's completion. If we don't do
// Overridden to allow us to check for the worker's completion. If we don't do
// this, then there is no way for the task dialog to know when the work is finished.
// The result of this is that the task dialog gets shown every time, regardless of
// whether the worker finished before the 'grace' period.
@@ -242,9 +241,20 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
}
private class SwingWorkerImpl extends SwingWorker<T, Object> {
// We update this latch *in the background thread* to notify clients in other threads that
// the work has been finished. This prevents the Swing thread from blocking while waiting
// for the callbacks that happen on the Swing thread, such as done().
private CountDownLatch finished = new CountDownLatch(1);
CountDownLatch getFinishedLatch() {
return finished;
}
@Override
protected T doInBackground() throws Exception {
T result = runInBackground(taskMonitor);
finished.countDown();
return result;
}
@@ -263,7 +273,7 @@ public abstract class CachingSwingWorker<T> implements CachingLoader<T> {
}
private class WorkerTaskMonitor extends TaskMonitorAdapter {
private List<TaskMonitor> monitors = new CopyOnWriteArrayList<TaskMonitor>();
private List<TaskMonitor> monitors = new CopyOnWriteArrayList<>();
private int min = 0;
private int max = 0;
private int progress = 0;
@@ -35,7 +35,9 @@ import ghidra.util.timer.GTimerMonitor;
/**
* Dialog that is displayed to show activity for a Task that is running outside of the
* Swing Thread.
* Swing Thread. This dialog uses a delay before showing in order to give the background task
* thread a chance to finish. This prevents a flashing dialog for tasks that finish before the
* delay time period.
*
* <p>Implementation note:
* if this class is constructed with a {@code hasProgress} value of {@code false},
@@ -47,6 +49,16 @@ import ghidra.util.timer.GTimerMonitor;
*
* <p>This dialog can be toggled between indeterminate mode and progress mode via calls to
* {@link #setIndeterminate(boolean)}.
*
* <p><b>API Usage Note: </b>If this dialog is used outside of the task API, then the client must
* be sure to call {@link #taskProcessed()}<b> from the background thread performing the work</b>.
* Otherwise, this dialog will always wait for the {@code delay} amount of time for the background
* thread to finish. This happens since the default completed notification mechanism is performed
* on the Swing thread. If a client has triggered blocking on the Swing thread, then the
* notification on the Swing thread must wait, causing the full delay to take place. Calling
* {@link #taskProcessed()} from the background thread allows the dialog to get notified before the
* {@code delay} period has expired. The blocking issue only exists with a non-0 {@code delay}
* value.
*/
public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
@@ -66,23 +78,23 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
* -calls iternalCancel()
* -triggers taskProcessed()
* -calls closeDialog runnable
*
*
* Cancel Button Pressed:
* -(same as Dialog Close Button Pressed)
*
* Task Monitor Stop Button Pressed:
* -triggers taskProcessed()
* -calls closeDialog runnable
*
*
* Public API dispose() is Called:
* -calls iternalCancel()
* -triggers taskProcessed()
* -calls closeDialog runnable
*
*
* Task Monitor Cancelled by API:
* -triggers taskProcessed()
* -calls closeDialog runnable
*
*
* Task Finishes Normally:
* -triggers taskProcessed()
* -calls closeDialog runnable
@@ -100,7 +112,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
};
private GTimerMonitor showTimer = GTimerMonitor.DUMMY;
private CountDownLatch finished = new CountDownLatch(1);
private CountDownLatch finished;
private boolean supportsProgress;
private JPanel mainPanel;
@@ -121,12 +133,11 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
* @param centerOnComp component to be centered over when shown, otherwise center over parent.
* If both centerOnComp and parent are null, dialog will be centered on screen.
* @param task the Task that this dialog will be associated with
* @param finished overrides the latch used by this dialog to know when the task is finished
* @param finished the finished latch used by the background thread to notify of completion
*/
TaskDialog(Component centerOnComp, Task task, CountDownLatch finished) {
this(centerOnComp, task.getTaskTitle(), task.isModal(), task.canCancel(),
task.hasProgress());
this.finished = finished;
task.hasProgress(), finished);
}
/**
@@ -147,7 +158,21 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
* @param hasProgress true if the dialog should show a progress bar
*/
public TaskDialog(String title, boolean canCancel, boolean isModal, boolean hasProgress) {
this(null, title, isModal, canCancel, hasProgress);
this(title, isModal, canCancel, hasProgress, new CountDownLatch(1));
}
/**
* Constructor
*
* @param title title for the dialog
* @param canCancel true if the task can be canceled
* @param isModal true if the dialog should be modal
* @param hasProgress true if the dialog should show a progress bar
* @param finished the finished latch used by the background thread to notify of completion
*/
public TaskDialog(String title, boolean canCancel, boolean isModal, boolean hasProgress,
CountDownLatch finished) {
this(null, title, isModal, canCancel, hasProgress, finished);
}
/**
@@ -159,12 +184,14 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
* @param isModal true if the dialog should be modal
* @param canCancel true if the task can be canceled
* @param hasProgress true if the dialog should show a progress bar
* @param finished the finished latch used by the background thread to notify of completion
*/
private TaskDialog(Component centerOnComp, String title, boolean isModal, boolean canCancel,
boolean hasProgress) {
boolean hasProgress, CountDownLatch finished) {
super(title, isModal, true, canCancel, true);
this.centerOnComponent = centerOnComp;
this.supportsProgress = hasProgress;
this.finished = finished;
setup(canCancel);
}
@@ -362,6 +389,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
/**
* Cancels the task and closes this dialog
*/
@Override
public void dispose() {
internalCancel();
}
@@ -409,7 +437,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
setIndeterminate(false);
}
// Note: it is not clear why we only wish to show progress if the monitor is not
// Note: it is not clear why we only wish to show progress if the monitor is not
// visible. This seems wrong. If someone knows, please update this code.
//if (!monitorComponent.isShowing()) {
installProgressMonitor();
@@ -431,7 +459,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
supportsProgress = !indeterminate;
monitorComponent.setIndeterminate(indeterminate);
// Assumption: if the client calls this method to show progress, then we should honor
// Assumption: if the client calls this method to show progress, then we should honor
// that request. If we find that nested monitor usage causes dialogs to incorrectly
// toggle monitors, then we need to update those clients to use a wrapping style
// monitor that prevents the behavior.