Bug 1473614 - Add type checks to PollPromise and TimedPromise. r=automatedtester
☠☠ backed out by bcd31adc1e7a ☠ ☠
authorAndreas Tolfsen <ato@sny.no>
Fri, 03 Aug 2018 16:08:45 +0100
changeset 826848 87a0ebcd004ffb9c15716b36635286bb4e3bd167
parent 826832 b3e7de2df93d2805434e2533f8c5a3fa1a6a40b9
child 826849 1ee58097b1412972a3ce2a6be6114025747f3f0a
push id118394
push usermaglione.k@gmail.com
push dateSun, 05 Aug 2018 20:34:06 +0000
reviewersautomatedtester
bugs1473614
milestone63.0a1
Bug 1473614 - Add type checks to PollPromise and TimedPromise. r=automatedtester This patch introduces stricted type checks for the sync module's PollPromise and TimedPromise. This may seem excessive, but we have had real and severe bugs in this area before.
testing/marionette/sync.js
testing/marionette/test/unit/test_sync.js
--- a/testing/marionette/sync.js
+++ b/testing/marionette/sync.js
@@ -81,20 +81,35 @@ const {TYPE_ONE_SHOT, TYPE_REPEATING_SLA
  *     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} = {}) {
   const timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 
+  if (typeof func != "function") {
+    throw new TypeError();
+  }
+  if (!(typeof timeout == "number" && typeof interval == "number")) {
+    throw new TypeError();
+  }
+  if ((!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 evalFn = () => {
       new Promise(func).then(resolve, rejected => {
         if (error.isError(rejected)) {
           throw rejected;
@@ -131,28 +146,43 @@ function PollPromise(func, {timeout = 20
  *
  * @param {Condition} func
  *     Function to run, which will have its ``reject``
  *     callback invoked after the ``timeout`` duration is reached.
  *     It is given two callbacks: ``resolve(value)`` and
  *     ``reject(error)``.
  * @param {timeout=} [timeout=1500] timeout
  *     ``condition``'s ``reject`` callback will be called
- *     after this timeout.
+ *     after this timeout, given in milliseconds.
  * @param {Error=} [throws=TimeoutError] throws
  *     When the ``timeout`` is hit, this error class will be
  *     thrown.  If it is null, no error is thrown and the promise is
  *     instead resolved on timeout.
  *
  * @return {Promise.<*>}
  *     Timed promise.
+ *
+ * @throws {TypeError}
+ *     If `timeout` is not a number.
+ * @throws {RangeError}
+ *     If `timeout` is not an unsigned integer.
  */
 function TimedPromise(fn, {timeout = 1500, throws = TimeoutError} = {}) {
   const timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 
+  if (typeof fn != "function") {
+    throw new TypeError();
+  }
+  if (typeof timeout != "number") {
+    throw new TypeError();
+  }
+  if (!Number.isInteger(timeout) || timeout < 0) {
+    throw new RangeError();
+  }
+
   return new Promise((resolve, reject) => {
     // Reject only if |throws| is given.  Otherwise it is assumed that
     // the user is OK with the promise timing out.
     let bail = () => {
       if (throws !== null) {
         let err = new throws();
         reject(err);
       } else {
--- a/testing/marionette/test/unit/test_sync.js
+++ b/testing/marionette/test/unit/test_sync.js
@@ -1,17 +1,54 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const {PollPromise} = ChromeUtils.import("chrome://marionette/content/sync.js", {});
+const {
+  PollPromise,
+  TimedPromise,
+} = ChromeUtils.import("chrome://marionette/content/sync.js", {});
 
 const DEFAULT_TIMEOUT = 2000;
 
-add_task(async function test_PollPromise_types() {
+add_test(function test_PollPromise_funcTypes() {
+  for (let type of ["foo", 42, null, undefined, true, [], {}]) {
+    Assert.throws(() => new PollPromise(type), /TypeError/);
+  }
+  new PollPromise(() => {});
+  new PollPromise(function() {});
+
+  run_next_test();
+});
+
+add_test(function test_PollPromise_timeoutTypes() {
+  for (let timeout of ["foo", null, true, [], {}]) {
+    Assert.throws(() => new PollPromise(() => {}, {timeout}), /TypeError/);
+  }
+  for (let timeout of [1.2, -1]) {
+    Assert.throws(() => new PollPromise(() => {}, {timeout}), /RangeError/);
+  }
+  new PollPromise(() => {}, {timeout: 42});
+
+  run_next_test();
+});
+
+add_test(function test_PollPromise_intervalTypes() {
+  for (let interval of ["foo", null, true, [], {}]) {
+    Assert.throws(() => new PollPromise(() => {}, {interval}), /TypeError/);
+  }
+  for (let interval of [1.2, -1]) {
+    Assert.throws(() => new PollPromise(() => {}, {interval}), /RangeError/);
+  }
+  new PollPromise(() => {}, {interval: 42});
+
+  run_next_test();
+});
+
+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();
@@ -67,8 +104,30 @@ add_task(async function test_PollPromise
 add_task(async function test_PollPromise_interval() {
   let nevals = 0;
   await new PollPromise((resolve, reject) => {
     ++nevals;
     reject();
   }, {timeout: 100, interval: 100});
   equal(2, nevals);
 });
+
+add_test(function test_TimedPromise_funcTypes() {
+  for (let type of ["foo", 42, null, undefined, true, [], {}]) {
+    Assert.throws(() => new TimedPromise(type), /TypeError/);
+  }
+  new TimedPromise(() => {});
+  new TimedPromise(function() {});
+
+  run_next_test();
+});
+
+add_test(function test_TimedPromise_timeoutTypes() {
+  for (let timeout of ["foo", null, true, [], {}]) {
+    Assert.throws(() => new TimedPromise(() => {}, {timeout}), /TypeError/);
+  }
+  for (let timeout of [1.2, -1]) {
+    Assert.throws(() => new TimedPromise(() => {}, {timeout}), /RangeError/);
+  }
+  new TimedPromise(() => {}, {timeout: 42});
+
+  run_next_test();
+});