Bug 1504756 - [marionette] Remove default timeout from PollPromise. r=ato
authorHenrik Skupin <mail@hskupin.info>
Thu, 10 Jan 2019 10:12:24 +0000
changeset 453228 bd95222980e2938436273c691740e7d5ff471bcb
parent 453227 ae09fe7cb7359584667d5627e142d9365668e3b5
child 453229 8a0f8116e6bb55cb084306c62513b55e0f2cf50a
push id35349
push userbtara@mozilla.com
push dateThu, 10 Jan 2019 17:19:27 +0000
treeherdermozilla-central@a51746f37520 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1504756
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1504756 - [marionette] Remove default timeout from PollPromise. r=ato By default PollPromise has to behave similar to a normal Promise and wait forever until it gets resolved or rejected. Depends on D13662 Differential Revision: https://phabricator.services.mozilla.com/D15907
testing/marionette/driver.js
testing/marionette/sync.js
testing/marionette/test/unit/test_sync.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -3579,10 +3579,10 @@ async function exitFullscreen(window) {
 function restoreWindow(window) {
   window.restore();
   return new PollPromise((resolve, reject) => {
     if (WindowState.from(window.windowState) != WindowState.Minimized) {
       resolve();
     } else {
       reject();
     }
-  });
+  }, {timeout: 2000});
 }
--- a/testing/marionette/sync.js
+++ b/testing/marionette/sync.js
@@ -87,66 +87,74 @@ function executeSoon(func) {
  *
  *     let els = new PollPromise((resolve, reject) => {
  *       let res = document.querySelectorAll("p");
  *       if (res.length > 0) {
  *         resolve(Array.from(res));
  *       } else {
  *         reject([]);
  *       }
- *     });
+ *     }, {timeout: 1000});
  *
  * @param {Condition} func
  *     Function to run off the main thread.
- * @param {number=} [timeout=2000] timeout
- *     Desired timeout.  If 0 or less than the runtime evaluation
+ * @param {number=} [timeout] timeout
+ *     Desired timeout if wanted.  If 0 or less than the runtime evaluation
  *     time of ``func``, ``func`` is guaranteed to run at least once.
- *     The default is 2000 milliseconds.
+ *     Defaults to using no timeout.
  * @param {number=} [interval=10] interval
  *     Duration between each poll of ``func`` in milliseconds.
  *     Defaults to 10 milliseconds.
  *
  * @return {Promise.<*>}
  *     Yields the value passed to ``func``'s
  *     ``resolve`` or ``reject`` callbacks.
  *
  * @throws {*}
  *     If ``func`` throws, its error is propagated.
  * @throws {TypeError}
  *     If `timeout` or `interval`` are not numbers.
  * @throws {RangeError}
  *     If `timeout` or `interval` are not unsigned integers.
  */
-function PollPromise(func, {timeout = 2000, interval = 10} = {}) {
+function PollPromise(func, {timeout = null, interval = 10} = {}) {
   const timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 
   if (typeof func != "function") {
     throw new TypeError();
   }
-  if (!(typeof timeout == "number" && typeof interval == "number")) {
+  if (timeout != null && typeof timeout != "number") {
     throw new TypeError();
   }
-  if ((!Number.isInteger(timeout) || timeout < 0) ||
+  if (typeof interval != "number") {
+    throw new TypeError();
+  }
+  if ((timeout && (!Number.isInteger(timeout) || timeout < 0)) ||
       (!Number.isInteger(interval) || interval < 0)) {
     throw new RangeError();
   }
 
   return new Promise((resolve, reject) => {
-    const start = new Date().getTime();
-    const end = start + timeout;
+    let start, end;
+
+    if (Number.isInteger(timeout)) {
+      start = new Date().getTime();
+      end = start + timeout;
+    }
 
     let evalFn = () => {
       new Promise(func).then(resolve, rejected => {
         if (error.isError(rejected)) {
           throw rejected;
         }
 
-        // return if timeout is 0, allowing |func| to be evaluated at
-        // least once
-        if (start == end || new Date().getTime() >= end) {
+        // return if there is a timeout and set to 0,
+        // allowing |func| to be evaluated at least once
+        if (typeof end != "undefined" &&
+            (start == end || new Date().getTime() >= end)) {
           resolve(rejected);
         }
       }).catch(reject);
     };
 
     // the repeating slack timer waits |interval|
     // before invoking |evalFn|
     evalFn();
--- a/testing/marionette/test/unit/test_sync.js
+++ b/testing/marionette/test/unit/test_sync.js
@@ -10,18 +10,16 @@ const {
   PollPromise,
   Sleep,
   TimedPromise,
   waitForEvent,
   waitForMessage,
   waitForObserverTopic,
 } = ChromeUtils.import("chrome://marionette/content/sync.js", {});
 
-const DEFAULT_TIMEOUT = 2000;
-
 /**
  * Mimic a DOM node for listening for events.
  */
 class MockElement {
   constructor() {
     this.capture = false;
     this.func = null;
     this.eventName = null;
@@ -135,23 +133,25 @@ add_test(function test_PollPromise_funcT
   }
   new PollPromise(() => {});
   new PollPromise(function() {});
 
   run_next_test();
 });
 
 add_test(function test_PollPromise_timeoutTypes() {
-  for (let timeout of ["foo", null, true, [], {}]) {
+  for (let timeout of ["foo", true, [], {}]) {
     Assert.throws(() => new PollPromise(() => {}, {timeout}), /TypeError/);
   }
   for (let timeout of [1.2, -1]) {
     Assert.throws(() => new PollPromise(() => {}, {timeout}), /RangeError/);
   }
-  new PollPromise(() => {}, {timeout: 42});
+  for (let timeout of [null, undefined, 42]) {
+    new PollPromise(resolve => resolve(1), {timeout});
+  }
 
   run_next_test();
 });
 
 add_test(function test_PollPromise_intervalTypes() {
   for (let interval of ["foo", null, true, [], {}]) {
     Assert.throws(() => new PollPromise(() => {}, {interval}), /TypeError/);
   }
@@ -164,65 +164,62 @@ add_test(function test_PollPromise_inter
 });
 
 add_task(async function test_PollPromise_retvalTypes() {
   for (let typ of [true, false, "foo", 42, [], {}]) {
     strictEqual(typ, await new PollPromise(resolve => resolve(typ)));
   }
 });
 
-add_task(async function test_PollPromise_timeoutElapse() {
-  let nevals = 0;
-  let start = new Date().getTime();
-  await new PollPromise((resolve, reject) => {
-    ++nevals;
-    reject();
-  });
-  let end = new Date().getTime();
-  greaterOrEqual((end - start), DEFAULT_TIMEOUT);
-  greaterOrEqual(nevals, 15);
-});
-
 add_task(async function test_PollPromise_rethrowError() {
   let nevals = 0;
   let err;
   try {
     await PollPromise(() => {
       ++nevals;
       throw new Error();
     });
   } catch (e) {
     err = e;
   }
   equal(1, nevals);
   ok(err instanceof Error);
 });
 
 add_task(async function test_PollPromise_noTimeout() {
+  let nevals = 0;
+  await new PollPromise((resolve, reject) => {
+    ++nevals;
+    nevals < 100 ? reject() : resolve();
+  });
+  equal(100, nevals);
+});
+
+add_task(async function test_PollPromise_zeroTimeout() {
   // run at least once when timeout is 0
   let nevals = 0;
   let start = new Date().getTime();
   await new PollPromise((resolve, reject) => {
     ++nevals;
     reject();
   }, {timeout: 0});
   let end = new Date().getTime();
   equal(1, nevals);
-  less((end - start), DEFAULT_TIMEOUT);
+  less((end - start), 500);
 });
 
-add_task(async function test_PollPromise_timeout() {
+add_task(async function test_PollPromise_timeoutElapse() {
   let nevals = 0;
   let start = new Date().getTime();
   await new PollPromise((resolve, reject) => {
     ++nevals;
     reject();
   }, {timeout: 100});
   let end = new Date().getTime();
-  greater(nevals, 1);
+  lessOrEqual(nevals, 11);
   greaterOrEqual((end - start), 100);
 });
 
 add_task(async function test_PollPromise_interval() {
   let nevals = 0;
   await new PollPromise((resolve, reject) => {
     ++nevals;
     reject();