Always schedule repositories view refresh job if requested
Don't rely on Job.getState() API: "Note that job state is inherently
volatile, and in most cases clients cannot rely on the result of this
method being valid by the time the result is obtained."
The fix should not add any extra overhead but avoid unstable tests and
occasional missing refreshes of the repositories view.
Cleaned up the job scheduling logic and tracing. To fix unstable tests,
made the asyncronous execution of the UI part of the job synchronous.
Change-Id: I7ffef1b5773e4f40a0719c4a0e9f940ed34b330c
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
index 1f27bc1..5458b24 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
@@ -181,11 +181,9 @@
private volatile long lastInputChange = 0L;
- private volatile long lastRepositoryChange = 0L;
-
private volatile long lastInputUpdate = -1L;
- private boolean reactOnSelection = false;
+ private boolean reactOnSelection;
private final IPreferenceChangeListener configurationListener;
@@ -210,7 +208,6 @@
myRefsChangedListener = new RefsChangedListener() {
@Override
public void onRefsChanged(RefsChangedEvent e) {
- lastRepositoryChange = System.currentTimeMillis();
scheduleRefresh(DEFAULT_REFRESH_DELAY);
}
};
@@ -218,7 +215,6 @@
myIndexChangedListener = new IndexChangedListener() {
@Override
public void onIndexChanged(IndexChangedEvent event) {
- lastRepositoryChange = System.currentTimeMillis();
scheduleRefresh(DEFAULT_REFRESH_DELAY);
}
@@ -227,7 +223,6 @@
myConfigChangeListener = new ConfigChangedListener() {
@Override
public void onConfigChanged(ConfigChangedEvent event) {
- lastRepositoryChange = System.currentTimeMillis();
scheduleRefresh(DEFAULT_REFRESH_DELAY);
}
};
@@ -617,64 +612,53 @@
scheduleRefresh(0);
}
- private Job scheduleRefresh(long delay) {
- boolean trace = GitTraceLocation.REPOSITORIESVIEW.isActive();
- if (trace)
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW.getLocation(),
- "Entering scheduleRefresh()"); //$NON-NLS-1$
+ private void trace(String message) {
+ GitTraceLocation.getTrace().trace(
+ GitTraceLocation.REPOSITORIESVIEW.getLocation(), message);
+ }
- if (scheduledJob != null
- && (scheduledJob.getState() == Job.RUNNING
- || scheduledJob.getState() == Job.WAITING || scheduledJob
- .getState() == Job.SLEEPING)) {
- if (trace)
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW.getLocation(),
- "Pending refresh job, returning"); //$NON-NLS-1$
- return scheduledJob;
+ private Job scheduleRefresh(long delay) {
+ if (GitTraceLocation.REPOSITORIESVIEW.isActive()) {
+ trace("Entering scheduleRefresh()"); //$NON-NLS-1$
}
- final CommonViewer tv = getCommonViewer();
- final boolean needsNewInput = lastInputChange > lastInputUpdate;
-
- if (trace)
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW.getLocation(),
- "New input required: " + needsNewInput); //$NON-NLS-1$
+ if (scheduledJob != null) {
+ schedule(scheduledJob, delay);
+ return scheduledJob;
+ }
Job job = new Job("Refreshing Git Repositories view") { //$NON-NLS-1$
@Override
protected IStatus run(IProgressMonitor monitor) {
- boolean actTrace = GitTraceLocation.REPOSITORIESVIEW.isActive();
- if (actTrace)
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW.getLocation(),
- "Running the update"); //$NON-NLS-1$
- lastInputUpdate = System.currentTimeMillis();
- if (needsNewInput)
- initRepositoriesAndListeners();
-
- if (!UIUtils.isUsable(tv))
+ final CommonViewer tv = getCommonViewer();
+ if (!UIUtils.isUsable(tv)) {
return Status.CANCEL_STATUS;
+ }
+ final boolean trace = GitTraceLocation.REPOSITORIESVIEW
+ .isActive();
+ final boolean needsNewInput = lastInputChange > lastInputUpdate;
+ if (trace) {
+ trace("Running the update, new input required: " //$NON-NLS-1$
+ + (lastInputChange > lastInputUpdate));
+ }
+ lastInputUpdate = System.currentTimeMillis();
+ if (needsNewInput) {
+ initRepositoriesAndListeners();
+ }
+
PlatformUI.getWorkbench().getDisplay()
- .asyncExec(new Runnable() {
+ .syncExec(new Runnable() {
@Override
public void run() {
- if (!UIUtils.isUsable(tv))
+ if (!UIUtils.isUsable(tv)) {
return;
- long start = 0;
- boolean traceActive = GitTraceLocation.REPOSITORIESVIEW
- .isActive();
- if (traceActive) {
- start = System.currentTimeMillis();
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW
- .getLocation(),
- "Starting async update job"); //$NON-NLS-1$
}
-
+ long start = 0;
+ if (trace) {
+ start = System.currentTimeMillis();
+ trace("Starting async update job"); //$NON-NLS-1$
+ }
if (needsNewInput) {
// keep expansion state and selection so that we can
@@ -684,8 +668,9 @@
tv.setInput(ResourcesPlugin.getWorkspace()
.getRoot());
tv.setExpandedElements(expanded);
- } else
+ } else {
tv.refresh(true);
+ }
IViewPart part = PlatformUI.getWorkbench()
.getActiveWorkbenchWindow().getActivePage()
@@ -693,59 +678,54 @@
if (part instanceof PropertySheet) {
PropertySheet sheet = (PropertySheet) part;
IPage page = sheet.getCurrentPage();
- if (page instanceof PropertySheetPage)
+ if (page instanceof PropertySheetPage) {
((PropertySheetPage) page).refresh();
+ }
}
- if (traceActive)
- GitTraceLocation
- .getTrace()
- .trace(
- GitTraceLocation.REPOSITORIESVIEW
- .getLocation(),
- "Ending async update job after " + (System.currentTimeMillis() - start) + " ms"); //$NON-NLS-1$ //$NON-NLS-2$
- if (!repositories.isEmpty())
+ if (trace) {
+ trace("Ending async update job after " //$NON-NLS-1$
+ + (System.currentTimeMillis() - start)
+ + " ms"); //$NON-NLS-1$
+ }
+ if (!repositories.isEmpty()) {
layout.topControl = getCommonViewer().getControl();
- else
+ } else {
layout.topControl = emptyArea;
+ }
emptyArea.getParent().layout(true, true);
}
});
-
- if (lastInputChange > lastInputUpdate
- || lastRepositoryChange > lastInputUpdate) {
- if (actTrace)
- GitTraceLocation.getTrace()
- .trace(
- GitTraceLocation.REPOSITORIESVIEW
- .getLocation(),
- "Rescheduling refresh job"); //$NON-NLS-1$
- schedule(DEFAULT_REFRESH_DELAY);
+ if (monitor.isCanceled()) {
+ return Status.CANCEL_STATUS;
}
return Status.OK_STATUS;
}
@Override
public boolean belongsTo(Object family) {
- if (JobFamilies.REPO_VIEW_REFRESH.equals(family))
- return true;
- return super.belongsTo(family);
+ return JobFamilies.REPO_VIEW_REFRESH.equals(family);
}
};
job.setSystem(true);
- IWorkbenchSiteProgressService service = CommonUtils.getService(getSite(), IWorkbenchSiteProgressService.class);
-
- if (trace)
- GitTraceLocation.getTrace().trace(
- GitTraceLocation.REPOSITORIESVIEW.getLocation(),
- "Scheduling refresh job"); //$NON-NLS-1$
- service.schedule(job, delay);
+ schedule(job, delay);
scheduledJob = job;
return scheduledJob;
}
+ private void schedule(Job job, long delay) {
+ IWorkbenchSiteProgressService service = CommonUtils.getService(getSite(), IWorkbenchSiteProgressService.class);
+
+ if (GitTraceLocation.REPOSITORIESVIEW.isActive()) {
+ GitTraceLocation.getTrace().trace(
+ GitTraceLocation.REPOSITORIESVIEW.getLocation(),
+ "Scheduling refresh job"); //$NON-NLS-1$
+ }
+ service.schedule(job, delay);
+ }
+
private void unregisterRepositoryListener() {
for (ListenerHandle lh : myListeners)
lh.remove();
