Permalink
Browse files

Bug 665411 - waitFor's timeout is inaccurate (potentially wildly so).…

… r=hskupin, r=jhammel
  • Loading branch information...
1 parent 3fedc7a commit 029a748bc740c6ea280b4cea3ce9d91bdb6b906e @xabolcs xabolcs committed with whimboo Oct 5, 2012
Showing with 72 additions and 11 deletions.
  1. +13 −11 mozmill/mozmill/extension/resource/stdlib/utils.js
  2. +59 −0 mutt/mutt/tests/js/utils/waitfor.js
@@ -260,33 +260,35 @@ function waitFor(callback, message, timeout, interval, thisObject) {
timeout = timeout || 5000;
interval = interval || 100;
- var self = {counter: 0,
- result: callback.call(thisObject)};
+ var self = {
+ timeIsUp: false,
+ result: callback.call(thisObject)
+ };
+ var deadline = Date.now() + timeout;
function wait() {
- self.counter += interval;
- self.result = callback.call(thisObject);
+ if (self.result !== true) {
+ self.result = callback.call(thisObject);
+ self.timeIsUp = Date.now() > deadline;
+ }
}
var timeoutInterval = hwindow.setInterval(wait, interval);
var thread = Cc["@mozilla.org/thread-manager;1"]
.getService().currentThread;
- while (true) {
+ while (self.result !== true && !self.timeIsUp) {
+ thread.processNextEvent(true);
+
let type = typeof(self.result);
if (type !== 'boolean')
throw TypeError("waitFor() callback has to return a boolean" +
" instead of '" + type + "'");
-
- if (self.result === true || self.counter >= timeout)
- break;
-
- thread.processNextEvent(true);
}
hwindow.clearInterval(timeoutInterval);
- if (self.counter >= timeout) {
+ if (self.result !== true && self.timeIsUp) {
message = message || arguments.callee.name + ": Timeout exceeded for '" + callback + "'";
throw new TimeoutError(message);
}
@@ -27,3 +27,62 @@ var testWaitForCallback = function () {
});
}, "TypeError", "Return type 'Object' in callback is not supported.");
}
+
+var testWaitForFalseAfterTrue = function () {
+ let onlyTheFirst = true;
+
+ expect.doesNotThrow(function () {
+ controller.waitFor(function () {
+ let res = onlyTheFirst;
+ onlyTheFirst = false;
+
+ return res;
+ }, undefined, 200);
+ }, "TimeoutError", "waitFor() has to pass after the first true is returned.");
+}
+
+var testWaitForCallbackCounter = function () {
+ let counter = -1;
+
+ expect.doesNotThrow(function () {
+ controller.waitFor(function () {
+ counter++;
+ return counter > 0;
+ }, undefined, 200);
+ }, "TimeoutError", "waitFor() shouldn't call callback after the first true result.");
+
+ expect.equal(counter, 1, "waitFor() shouldn't call callback after the first true result. (Second check)");
+}
+
+var testWaitForTimeoutAccuracy = function () {
+ const waitForInnerSleep = 500;
+ const waitForTimeout = 2500;
+ const waitForInterval = 100;
+
+ /**
+ * expectedElaspedTime constist of:
+ * A - (inner sleep)ms while initializing waitFor
+ * B - (interval)ms when the event loop get spinned, till the interval fires
+ * C - (timeout)ms when the timeout occures
+ *
+ * maxAllowedDifference should aware of inner sleep
+ **/
+ const expectedElaspedTime = waitForInnerSleep + waitForTimeout + waitForInterval;
+ const maxAllowedDifference = 2 * waitForInnerSleep;
+
+ var time = Date.now();
+
+ expect.throws(function () {
+ controller.waitFor(function () {
+ controller.sleep(waitForInnerSleep);
+ return false;
+ }, null, waitForTimeout, waitForInterval);
+ }, "TimeoutError", "waitFor() should have run into a timeout.");
+
+ var endTime = Date.now();
+ var elaspedTime = endTime - time;
+ var difference = Math.abs(expectedElaspedTime - elaspedTime);
+
+ expect.ok(difference <= maxAllowedDifference, "Expected waitFor() timeout" +
+ " is less than " + maxAllowedDifference + "ms.");
+}

0 comments on commit 029a748

Please sign in to comment.