Goktug Gokdogan has uploaded a new change for review.
https://gwt-review.googlesource.com/3321
Change subject: Adds Timer#isRunning().
......................................................................
Adds Timer#isRunning().
This patch uses Integer instead of int to keep track of the running status.
As now, running state is kept, cancel is no longer called for timers that
are not scheduled.
Bug: issue 3935
Change-Id: I0133c64de75d5d95cff863c5ab950d474dda0c56
---
M user/src/com/google/gwt/user/client/Timer.java
M user/test/com/google/gwt/user/MiscSuite.java
D user/test/com/google/gwt/user/client/TimerCancelTest.java
A user/test/com/google/gwt/user/client/TimerTest.java
4 files changed, 171 insertions(+), 139 deletions(-)
diff --git a/user/src/com/google/gwt/user/client/Timer.java
b/user/src/com/google/gwt/user/client/Timer.java
index f95a5a7..ae2dccf 100644
--- a/user/src/com/google/gwt/user/client/Timer.java
+++ b/user/src/com/google/gwt/user/client/Timer.java
@@ -15,7 +15,6 @@
*/
package com.google.gwt.user.client;
-
/**
* A simplified, browser-safe timer class. This class serves the same
purpose as
* java.util.Timer, but is simplified because of the single-threaded
@@ -61,7 +60,7 @@
private boolean isRepeating;
- private int timerId;
+ private Integer timerId = null;
/**
* Workaround for broken clearTimeout in IE. Keeps track of whether
cancel has been called since
@@ -70,15 +69,28 @@
private int cancelCounter = 0;
/**
- * Cancels this timer.
+ * Returns {@code true} if the timer is running. Timer is running if and
only if it is scheduled
+ * but it is not expired or cancelled.
+ */
+ public final boolean isRunning() {
+ return timerId != null;
+ }
+
+ /**
+ * Cancels this timer. If the timer is not running, this is a no-op.
*/
public void cancel() {
+ if (!isRunning()) {
+ return;
+ }
+
cancelCounter++;
if (isRepeating) {
clearInterval(timerId);
} else {
clearTimeout(timerId);
}
+ timerId = null;
}
/**
@@ -88,31 +100,36 @@
public abstract void run();
/**
- * Schedules a timer to elapse in the future.
- *
- * @param delayMillis how long to wait before the timer elapses, in
- * milliseconds
+ * Schedules a timer to elapse in the future. If the timer is already
running then it will be
+ * first canceled before re-scheduling.
+ *
+ * @param delayMillis how long to wait before the timer elapses, in
milliseconds
*/
public void schedule(int delayMillis) {
if (delayMillis < 0) {
throw new IllegalArgumentException("must be non-negative");
}
- cancel();
+ if (isRunning()) {
+ cancel();
+ }
isRepeating = false;
timerId = createTimeout(this, delayMillis, cancelCounter);
}
/**
- * Schedules a timer that elapses repeatedly.
- *
- * @param periodMillis how long to wait before the timer elapses, in
- * milliseconds, between each repetition
+ * Schedules a timer that elapses repeatedly. If the timer is already
running then it will be
+ * first canceled before re-scheduling.
+ *
+ * @param periodMillis how long to wait before the timer elapses, in
milliseconds, between each
+ * repetition
*/
public void scheduleRepeating(int periodMillis) {
if (periodMillis <= 0) {
throw new IllegalArgumentException("must be positive");
}
- cancel();
+ if (isRunning()) {
+ cancel();
+ }
isRepeating = true;
timerId = createInterval(this, periodMillis, cancelCounter);
}
@@ -123,10 +140,15 @@
* Only call run() if cancelCounter has not changed since the timer was
scheduled.
*/
final void fire(int scheduleCancelCounter) {
+ // Workaround for broken clearTimeout in IE.
if (scheduleCancelCounter != cancelCounter) {
return;
}
+ if (!isRepeating) {
+ timerId = null;
+ }
+
// Run the timer's code.
run();
}
diff --git a/user/test/com/google/gwt/user/MiscSuite.java
b/user/test/com/google/gwt/user/MiscSuite.java
index 63eba39..1ad8318 100644
--- a/user/test/com/google/gwt/user/MiscSuite.java
+++ b/user/test/com/google/gwt/user/MiscSuite.java
@@ -25,7 +25,7 @@
import com.google.gwt.user.client.EventTest;
import com.google.gwt.user.client.GestureEventSinkTest;
import com.google.gwt.user.client.HistoryDisabledTest;
-import com.google.gwt.user.client.TimerCancelTest;
+import com.google.gwt.user.client.TimerTest;
import com.google.gwt.user.client.TouchEventSinkTest;
import com.google.gwt.user.client.WindowTest;
import com.google.gwt.user.datepicker.client.CalendarUtilTest;
@@ -54,7 +54,7 @@
suite.addTestSuite(HistoryDisabledTest.class);
suite.addTestSuite(ImageBundleGeneratorTest.class);
suite.addTestSuite(LayoutTest.class);
- suite.addTestSuite(TimerCancelTest.class);
+ suite.addTestSuite(TimerTest.class);
suite.addTestSuite(TouchEventSinkTest.class);
suite.addTestSuite(WindowTest.class);
suite.addTestSuite(XMLTest.class);
diff --git a/user/test/com/google/gwt/user/client/TimerCancelTest.java
b/user/test/com/google/gwt/user/client/TimerCancelTest.java
deleted file mode 100644
index 154bb45..0000000
--- a/user/test/com/google/gwt/user/client/TimerCancelTest.java
+++ /dev/null
@@ -1,124 +0,0 @@
-/*
- * Copyright 2013 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may
not use this file except
- * in compliance with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
ANY KIND, either express
- * or implied. See the License for the specific language governing
permissions and limitations under
- * the License.
- */
-
-package com.google.gwt.user.client;
-
-import com.google.gwt.core.client.Duration;
-import com.google.gwt.junit.client.GWTTestCase;
-
-/**
- * Tests {@link Timer#cancel()} functionality.
- */
-public class TimerCancelTest extends GWTTestCase {
-
- private static final class CountingTimer extends Timer {
- private int timerCount;
- @Override
- public void run() {
- timerCount++;
- }
- }
-
- @Override
- public String getModuleName() {
- return "com.google.gwt.user.UserTest";
- }
-
- public void testCancelTimer() {
- final CountingTimer canceledTimer = new CountingTimer();
-
- Timer cancelingTimer = new Timer() {
- @Override
- public void run() {
- assertEquals(0, canceledTimer.timerCount);
- canceledTimer.cancel();
- }
- };
- cancelingTimer.schedule(50);
- canceledTimer.schedule(100);
-
-
- busyWait(200);
-
- delayTestFinish(500);
- new Timer() {
- @Override
- public void run() {
- assertEquals(0, canceledTimer.timerCount);
- finishTest();
- }
- }.schedule(300);
- }
-
- public void testRestartTimer() {
- final CountingTimer restartedTimer = new CountingTimer();
-
- Timer cancelingTimer = new Timer() {
- @Override
- public void run() {
- assertEquals(0, restartedTimer.timerCount);
- restartedTimer.cancel();
- restartedTimer.schedule(100);
- }
- };
-
- cancelingTimer.schedule(50);
- restartedTimer.schedule(100);
-
- busyWait(200);
-
- delayTestFinish(500);
- new Timer() {
- @Override
- public void run() {
- assertEquals(1, restartedTimer.timerCount);
- finishTest();
- }
- }.schedule(400);
- }
-
- private static void busyWait(double duration) {
- /*
- * It seems that IE adds an event to the javascript event loop
immediately when a timer expires
- * (supposedly from a separate thread). After this has happened,
canceling the timer has no
- * effect because it is already in the queue and no further checks are
done when running the
- * event once it reaches the head of the queue.
- *
- * This means that to trigger the bug, we must ensure the timer has
been added to the event loop
- * queue before it is canceled. To ensure this happens, we will busy
wait until both timers
- * should have fired. This means the following happens:
- *
- * 1) While busy waiting, IE adds events for each timer to the event
loop
- *
- * 2) IE pumps the event loop, running the helper timer that cancels
the tested timer. This does
- * however not have any effect because the timer is already in the
event loop queue.
- *
- * 3) IE pumps the event loop again and runs the event for the tested
timer, without realizing
- * that it has been canceled.
- *
- * Without busy waiting, the tested timer would not yet have been
added to the event loop queue
- * at the point when the timer is canceled, in which case canceling
the timer would work as
- * expected.
- *
- * If the two timers are not scheduled in the same order that they
will run, it seems that IE
- * does some additional checks that makes the problem disappear.
- */
-
- double start = Duration.currentTimeMillis();
- while (Duration.currentTimeMillis() - start <= duration) {
- // Busy wait
- }
- }
-
-}
diff --git a/user/test/com/google/gwt/user/client/TimerTest.java
b/user/test/com/google/gwt/user/client/TimerTest.java
new file mode 100644
index 0000000..7d39b05
--- /dev/null
+++ b/user/test/com/google/gwt/user/client/TimerTest.java
@@ -0,0 +1,134 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
ANY KIND, either express
+ * or implied. See the License for the specific language governing
permissions and limitations under
+ * the License.
+ */
+
+package com.google.gwt.user.client;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Tests {@link Timer} functionality.
+ */
+public class TimerTest extends GWTTestCase {
+
+ private List<Timer> executedTimers;
+
+ @Override
+ public String getModuleName() {
+ return "com.google.gwt.user.UserTest";
+ }
+
+ @Override
+ protected void gwtSetUp() throws Exception {
+ executedTimers = new ArrayList<Timer>();
+ }
+
+ private final class TestTimer extends Timer {
+ @Override
+ public void run() {
+ executedTimers.add(this);
+ }
+ }
+
+ public void testTimer() {
+ Timer timer = new TestTimer();
+ assertFalse(timer.isRunning());
+ timer.schedule(10);
+ assertTrue(timer.isRunning());
+ assertExecutedTimerCount(1);
+ }
+
+ public void testCancelTimer() {
+ Timer timer = new TestTimer();
+ timer.schedule(10);
+ timer.cancel();
+ assertFalse(timer.isRunning());
+ assertExecutedTimerCount(0);
+ }
+
+ public void testRescheduleTimer() {
+ Timer timer = new TestTimer();
+ timer.schedule(10);
+ timer.schedule(20);
+ assertExecutedTimerCount(1);
+ }
+
+ public void testRescheduleTimerRepeatingToNonRepeating() {
+ Timer timer = new TestTimer();
+ timer.scheduleRepeating(10);
+ timer.schedule(20);
+ assertExecutedTimerCount(1);
+ }
+
+ private final class CancelingTestTimer extends Timer {
+ Timer otherTimer;
+
+ @Override
+ public void run() {
+ otherTimer.cancel();
+ executedTimers.add(this);
+ }
+ }
+
+ // Issue
https://code.google.com/p/google-web-toolkit/issues/detail?id=8101
+ public void testCancelTimer_ieBug() {
+ final CancelingTestTimer timer1 = new CancelingTestTimer();
+ final CancelingTestTimer timer2 = new CancelingTestTimer();
+ timer1.otherTimer = timer2;
+ timer2.otherTimer = timer1;
+
+ timer1.schedule(10);
+ timer2.schedule(10);
+
+ // only one of them should have been executed
+ assertExecutedTimerCount(1);
+ }
+
+ private final class ReschedulingTestTimer extends Timer {
+ Timer otherTimer;
+
+ @Override
+ public void run() {
+ otherTimer.schedule(2000); // schedule far ahead, should be
practically same as canceling
+ executedTimers.add(this);
+ }
+ }
+
+ // Issue
https://code.google.com/p/google-web-toolkit/issues/detail?id=8101
+ public void testRescheduleTimer_ieBug() {
+ final ReschedulingTestTimer timer1 = new ReschedulingTestTimer();
+ final ReschedulingTestTimer timer2 = new ReschedulingTestTimer();
+ timer1.otherTimer = timer2;
+ timer2.otherTimer = timer1;
+
+ timer1.schedule(10);
+ timer2.schedule(10);
+
+ // only one of them should have been executed
+ assertExecutedTimerCount(1);
+ }
+
+ private void assertExecutedTimerCount(final int count) {
+ delayTestFinish(400);
+ new Timer() {
+ @Override
+ public void run() {
+ assertEquals(count, executedTimers.size());
+ finishTest();
+ }
+ }.schedule(200);
+ }
+}
--
To view, visit https://gwt-review.googlesource.com/3321
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0133c64de75d5d95cff863c5ab950d474dda0c56
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <[email protected]>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
