NEW - bug 249883: [patch] activity timing not correct when AbstractUserActivityMonitor.getLastInteractionTime() is overridden
https://bugs.eclipse.org/bugs/show_bug.cgi?id=249883
diff --git a/org.eclipse.mylyn.monitor.tests/src/org/eclipse/mylyn/monitor/tests/CheckActivityJobTest.java b/org.eclipse.mylyn.monitor.tests/src/org/eclipse/mylyn/monitor/tests/CheckActivityJobTest.java
new file mode 100644
index 0000000..f4bfcc8
--- /dev/null
+++ b/org.eclipse.mylyn.monitor.tests/src/org/eclipse/mylyn/monitor/tests/CheckActivityJobTest.java
@@ -0,0 +1,204 @@
+/*******************************************************************************
+ * Copyright (c) 2004, 2008 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.monitor.tests;
+
+import junit.framework.TestCase;
+
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.mylyn.internal.monitor.ui.CheckActivityJob;
+import org.eclipse.mylyn.internal.monitor.ui.IActivityManagerCallback;
+
+/**
+ * @author Steffen Pingel
+ */
+public class CheckActivityJobTest extends TestCase {
+
+ private StubCallback callback;
+
+ private TestableCheckActivityJob job;
+
+ @Override
+ protected void setUp() throws Exception {
+ callback = new StubCallback();
+ job = new TestableCheckActivityJob(callback);
+ }
+
+ public void testInactivityTimeout() throws Exception {
+ callback.lastEventTime = System.currentTimeMillis() - 41;
+ job.setInactivityTimeout(40);
+ job.run();
+ assertFalse(job.isActive());
+ job.run();
+ assertFalse(job.isActive());
+ callback.lastEventTime = System.currentTimeMillis();
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(0, callback.activeTime);
+ Thread.sleep(6);
+ job.run();
+ assertTrue(job.isActive());
+ assertTrue("expected less than 5 < activeTime < 20, got " + callback.activeTime, callback.activeTime > 5
+ && callback.activeTime < 20);
+ }
+
+ public void testResumeFromSleepNoTimeout() throws Exception {
+ job.setInactivityTimeout(0);
+ job.run();
+ assertTrue(job.isActive());
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(1, callback.eventCount);
+ job.run();
+ assertEquals(2, callback.eventCount);
+ assertTrue(job.isActive());
+ Thread.sleep(11);
+ job.run();
+ assertTrue(job.isActive());
+ assertTrue("expected more than 10 ms, got " + callback.activeTime, callback.activeTime > 10);
+ assertEquals(3, callback.eventCount);
+ }
+
+ public void testResumeFromSleepTimeoutNoEvent() throws Exception {
+ callback.lastEventTime = System.currentTimeMillis();
+ job.setInactivityTimeout(20);
+ job.setTick(20);
+ job.run();
+ assertTrue(job.isActive());
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(1, callback.eventCount);
+ Thread.sleep(61);
+ // resume from sleep past timeout
+ job.run();
+ assertFalse(job.isActive());
+ job.run();
+ assertFalse(job.isActive());
+ assertTrue("expected less than 10 ms, got " + callback.activeTime, callback.activeTime < 10);
+ assertEquals(1, callback.eventCount);
+ assertEquals(callback.lastEventTime, callback.startTime);
+ }
+
+ public void testResumeFromSleepTimeoutEvent() throws Exception {
+ callback.lastEventTime = System.currentTimeMillis();
+ job.setInactivityTimeout(20);
+ job.setTick(20);
+ job.run();
+ assertTrue(job.isActive());
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(1, callback.eventCount);
+ Thread.sleep(41);
+ // resume from sleep past timeout
+ job.run();
+ assertTrue(callback.inactive);
+ assertFalse(job.isActive());
+ Thread.sleep(11);
+ // should still discard events
+ job.run();
+ assertFalse(job.isActive());
+ // start activity
+ callback.lastEventTime = System.currentTimeMillis();
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(1, callback.eventCount);
+ Thread.sleep(11);
+ job.run();
+ // check if time sleeping was logged
+ assertTrue("expected less than 10 < activeTime < 20, got " + callback.activeTime, callback.activeTime > 10
+ && callback.activeTime < 20);
+ assertEquals(2, callback.eventCount);
+ }
+
+ public void testResumeFromSleepTimeoutEventDiscarded() throws Exception {
+ callback.lastEventTime = System.currentTimeMillis();
+ job.setInactivityTimeout(20);
+ job.setTick(20);
+ job.run();
+ assertTrue(job.isActive());
+ job.run();
+ assertTrue(job.isActive());
+ assertEquals(1, callback.eventCount);
+ Thread.sleep(61);
+ // resume from sleep past timeout
+ callback.lastEventTime = System.currentTimeMillis();
+ job.run();
+ assertFalse(callback.inactive);
+ assertTrue(job.isActive());
+ Thread.sleep(6);
+ job.run();
+ assertTrue(job.isActive());
+ // check if time sleeping was logged
+ assertTrue("expected less than 5 < activeTime < 10, got " + callback.activeTime, callback.activeTime > 5
+ && callback.activeTime < 10);
+ assertEquals(2, callback.eventCount);
+ }
+
+ private class TestableCheckActivityJob extends CheckActivityJob {
+
+ public TestableCheckActivityJob(IActivityManagerCallback callback) {
+ super(callback);
+ }
+
+ public IStatus run() {
+ return super.run(new NullProgressMonitor());
+ }
+
+ @Override
+ protected boolean isEnabled() {
+ return true;
+ }
+
+ public void setPreviousEventTime(long previousEventTime) {
+ this.previousEventTime = previousEventTime;
+ }
+
+ public void setTick(long tick) {
+ this.tick = tick;
+ }
+
+ @Override
+ public void reschedule() {
+ // ignore, job is called explicitly from test
+ }
+
+ }
+
+ private class StubCallback implements IActivityManagerCallback {
+
+ private boolean inactive;
+
+ private long lastEventTime;
+
+ private long activeTime;
+
+ private long eventCount;
+
+ private long startTime;
+
+ public void addMonitoredActivityTime(long startTime, long endTime) {
+ this.startTime = startTime;
+ this.activeTime += endTime - startTime;
+ this.eventCount++;
+ }
+
+ public void fireInactive() {
+ this.inactive = true;
+ }
+
+ public long getLastEventTime() {
+ return this.lastEventTime;
+ }
+
+ }
+
+}
diff --git a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/ActivityContextManager.java b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/ActivityContextManager.java
index fd11b57..84ae8dc 100644
--- a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/ActivityContextManager.java
+++ b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/ActivityContextManager.java
@@ -1,5 +1,5 @@
/*******************************************************************************
-* Copyright (c) 2004, 2008 Tasktop Technologies and others.
+ * Copyright (c) 2004, 2008 Tasktop Technologies and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -16,10 +16,6 @@
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;
-import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.Platform;
-import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jface.util.IPropertyChangeListener;
import org.eclipse.jface.util.PropertyChangeEvent;
@@ -46,25 +42,11 @@
@SuppressWarnings("restriction")
public class ActivityContextManager implements IActivityContextManager {
- private final int TICK = 30 * 1000;
-
- private final int SHORT_TICK = 5 * 1000;
-
private AbstractUserActivityMonitor userActivityMonitor;
private final Set<IUserAttentionListener> attentionListeners = new CopyOnWriteArraySet<IUserAttentionListener>();
- private long startTime = -1;
-
- private int timeout;
-
- private final Object startTimeLock = new Object();
-
- private boolean wasTimedOut = true;
-
- private int wait = SHORT_TICK;
-
- private CheckActivityJob checkJob;
+ private final CheckActivityJob checkJob;
private IWorkingSet[] workingSets;
@@ -82,10 +64,23 @@
}
};
- public ActivityContextManager(int timeout, ArrayList<AbstractUserActivityMonitor> monitors) {
+ public ActivityContextManager(ArrayList<AbstractUserActivityMonitor> monitors) {
this.activityMonitors = monitors;
- this.timeout = timeout;
+ checkJob = new CheckActivityJob(new IActivityManagerCallback() {
+ public void addMonitoredActivityTime(long localStartTime, long currentTime) {
+ ActivityContextManager.this.addMonitoredActivityTime(localStartTime, currentTime);
+ }
+ public void fireInactive() {
+ ActivityContextManager.this.fireInactive();
+ }
+
+ public long getLastEventTime() {
+ return ActivityContextManager.this.getLastEventTime();
+ }
+ });
+ checkJob.setSystem(true);
+ checkJob.setPriority(Job.INTERACTIVE);
}
protected void updateWorkingSetSelection() {
@@ -106,22 +101,15 @@
}
updateWorkingSetSelection();
PlatformUI.getWorkbench().getWorkingSetManager().addPropertyChangeListener(WORKING_SET_CHANGE_LISTENER);
- checkJob = new CheckActivityJob();
- checkJob.setSystem(true);
- checkJob.setPriority(Job.DECORATE);
- checkJob.schedule(TICK);
+ checkJob.reschedule();
}
public void stop() {
for (AbstractUserActivityMonitor monitor : activityMonitors) {
monitor.stop();
}
-
PlatformUI.getWorkbench().getWorkingSetManager().removePropertyChangeListener(WORKING_SET_CHANGE_LISTENER);
-
- if (checkJob != null) {
- checkJob.cancel();
- }
+ checkJob.cancel();
}
public void addListener(IUserAttentionListener listener) {
@@ -200,69 +188,12 @@
return -1;
}
- private long getStartTime() {
- synchronized (startTimeLock) {
- return startTime;
- }
- }
-
- private void setStartTime(long startTime) {
- synchronized (startTimeLock) {
- this.startTime = startTime;
- }
- }
-
- class CheckActivityJob extends Job {
-
- public CheckActivityJob() {
- super("Activity Monitor Job");
- }
-
- @Override
- protected IStatus run(IProgressMonitor monitor) {
- try {
- if (Platform.isRunning()) {
- if (!MonitorUiPlugin.getDefault().getWorkbench().isClosing()) {
-
- long localLastEventTime = getLastEventTime();
- long localStartTime = getStartTime();
- long currentTime = System.currentTimeMillis();
- if ((currentTime - localLastEventTime) >= timeout && timeout != 0) {
- if (wasTimedOut == false) {
- fireInactive();
- // timed out
- wasTimedOut = true;
- }
- wait = SHORT_TICK;
- } else {
- if (wasTimedOut) {
- wasTimedOut = false;
- // back...
- setStartTime(localLastEventTime);
- } else {
- addMonitoredActivityTime(localStartTime, currentTime);
- setStartTime(currentTime);
- }
- wait = TICK;
- }
-
- }
- }
- return Status.OK_STATUS;
- } finally {
- if (Platform.isRunning()) {
- checkJob.schedule(wait);
- }
- }
- }
- }
-
public void setInactivityTimeout(int inactivityTimeout) {
- timeout = inactivityTimeout;
+ checkJob.setInactivityTimeout(inactivityTimeout);
}
public int getInactivityTimeout() {
- return timeout;
+ return checkJob.getInactivityTimeout();
}
/**
diff --git a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/CheckActivityJob.java b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/CheckActivityJob.java
new file mode 100644
index 0000000..6792934
--- /dev/null
+++ b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/CheckActivityJob.java
@@ -0,0 +1,131 @@
+/*******************************************************************************
+ * Copyright (c) 2004, 2008 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.monitor.ui;
+
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.Job;
+
+/**
+ * A job that is scheduled periodically to check for user activity.
+ *
+ * @author Robert Elves
+ * @author Steffen Pingel
+ */
+public class CheckActivityJob extends Job {
+
+ /**
+ * If true, the user is assumed to be active.
+ */
+ private boolean active;
+
+ /**
+ * The rescheduling interval in ms when active. Should be reasonably short to provide accurate tracking.
+ */
+ private final int ACTIVE_TICK = 30 * 1000;
+
+ private final IActivityManagerCallback callback;
+
+ /**
+ * The timeout when to assume a user is inactive. If set to 0 it is assumed that a user is always active.
+ */
+ private int inactivityTimeout;
+
+ protected long previousEventTime;
+
+ /**
+ * Protected for testing.
+ */
+ protected long tick = ACTIVE_TICK;
+
+ public CheckActivityJob(IActivityManagerCallback callback) {
+ super("Activity Monitor Job");
+ this.callback = callback;
+ }
+
+ public int getInactivityTimeout() {
+ return this.inactivityTimeout;
+ }
+
+ public boolean isActive() {
+ return active;
+ }
+
+ protected boolean isEnabled() {
+ return Platform.isRunning() && !MonitorUiPlugin.getDefault().getWorkbench().isClosing();
+ }
+
+ public void reschedule() {
+ schedule(active ? tick : tick / 6);
+ }
+
+ @Override
+ protected IStatus run(IProgressMonitor monitor) {
+ if (isEnabled()) {
+ try {
+ long lastEventTime = callback.getLastEventTime();
+ long currentTime = System.currentTimeMillis();
+ // check if the last activity exceeds timeout
+ if ((currentTime - lastEventTime) >= inactivityTimeout && inactivityTimeout != 0) {
+ if (active) {
+ // time out
+ active = false;
+ callback.fireInactive();
+ }
+ } else {
+ if (!active) {
+ active = true;
+ // back, start recording activity
+ if (inactivityTimeout != 0) {
+ previousEventTime = lastEventTime;
+ } else {
+ // if timeouts are disabled only the currentTime is relevant for tracking activity
+ previousEventTime = currentTime;
+ }
+ } else {
+ // check if the activity internal is unreasonably long, it is likely that
+ // the computer came back from sleep at worst difference should be tick * 2
+ if (currentTime - previousEventTime > tick * 3) {
+ if (inactivityTimeout != 0) {
+ // check for recent event
+ if (currentTime - lastEventTime <= tick) {
+ // event since resume
+ previousEventTime = lastEventTime;
+ } else {
+ // time out
+ active = false;
+ callback.fireInactive();
+ }
+ } else {
+ // if timeouts are disabled only the currentTime is relevant for tracking activity
+ previousEventTime = currentTime;
+ }
+ } else {
+ callback.addMonitoredActivityTime(previousEventTime, currentTime);
+ previousEventTime = currentTime;
+ }
+ }
+ }
+ } finally {
+ reschedule();
+ }
+ }
+ return Status.OK_STATUS;
+ }
+
+ public void setInactivityTimeout(int inactivityTimeout) {
+ this.inactivityTimeout = inactivityTimeout;
+ }
+
+}
diff --git a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/IActivityManagerCallback.java b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/IActivityManagerCallback.java
new file mode 100644
index 0000000..0ff5930
--- /dev/null
+++ b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/IActivityManagerCallback.java
@@ -0,0 +1,22 @@
+/*******************************************************************************
+ * Copyright (c) 2004, 2008 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.monitor.ui;
+
+public interface IActivityManagerCallback {
+
+ public abstract void fireInactive();
+
+ public abstract void addMonitoredActivityTime(long localStartTime, long currentTime);
+
+ public abstract long getLastEventTime();
+
+}
diff --git a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/MonitorUiPlugin.java b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/MonitorUiPlugin.java
index 05c4916..ccbba37 100644
--- a/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/MonitorUiPlugin.java
+++ b/org.eclipse.mylyn.monitor.ui/src/org/eclipse/mylyn/internal/monitor/ui/MonitorUiPlugin.java
@@ -1,5 +1,5 @@
/*******************************************************************************
-* Copyright (c) 2004, 2008 Tasktop Technologies and others.
+ * Copyright (c) 2004, 2008 Tasktop Technologies and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -55,8 +55,6 @@
public static final String ID_PLUGIN = "org.eclipse.mylyn.monitor.ui";
- private static final int TIMEOUT_INACTIVITY_MILLIS = 60 * 1000;
-
private static MonitorUiPlugin INSTANCE;
private ShellLifecycleListener shellLifecycleListener;
@@ -410,7 +408,7 @@
monitors.add(new WorkbenchUserActivityMonitor());
new MonitorUiExtensionPointReader().initExtensions();
- activityContextManager = new ActivityContextManager(TIMEOUT_INACTIVITY_MILLIS, monitors);
+ activityContextManager = new ActivityContextManager(monitors);
updateActivityTimout();
