Bug 1442543 - Add type checks for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r?whimboo draft
authorAndreas Tolfsen <ato@sny.no>
Fri, 02 Mar 2018 13:25:59 +0000
changeset 763651 f94b375e2dfa983dc3a19f1104f94054273ddffa
parent 763635 a007dd56b9947a93c276e82275d7065db1949c9e
push id101508
push userbmo:ato@sny.no
push dateTue, 06 Mar 2018 11:30:00 +0000
reviewerswhimboo
bugs1442543
milestone60.0a1
Bug 1442543 - Add type checks for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r?whimboo Adds type checks for the WebDriver:ExecuteScript and WebDriver:ExecuteAsyncScript commands. This should help prevent accidental programming mistakes with the Marionette Python client. MozReview-Commit-ID: JjKgG9OWrdL
testing/marionette/driver.js
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -36,16 +36,17 @@ const {
   NoSuchFrameError,
   NoSuchWindowError,
   SessionNotCreatedError,
   UnknownError,
   UnsupportedOperationError,
   WebDriverError,
 } = ChromeUtils.import("chrome://marionette/content/error.js", {});
 ChromeUtils.import("chrome://marionette/content/evaluate.js");
+const {pprint} = ChromeUtils.import("chrome://marionette/content/format.js", {});
 ChromeUtils.import("chrome://marionette/content/interaction.js");
 ChromeUtils.import("chrome://marionette/content/l10n.js");
 ChromeUtils.import("chrome://marionette/content/legacyaction.js");
 ChromeUtils.import("chrome://marionette/content/modal.js");
 ChromeUtils.import("chrome://marionette/content/proxy.js");
 ChromeUtils.import("chrome://marionette/content/reftest.js");
 ChromeUtils.import("chrome://marionette/content/session.js");
 const {
@@ -832,17 +833,17 @@ GeckoDriver.prototype.getContext = funct
  * causing any change it makes on the global state of the document to have
  * lasting side-effects.
  *
  * @param {string} script
  *     Script to evaluate as a function body.
  * @param {Array.<(string|boolean|number|object|WebElement)>} args
  *     Arguments exposed to the script in <code>arguments</code>.
  *     The array items must be serialisable to the WebDriver protocol.
- * @param {number} scriptTimeout
+ * @param {number=} scriptTimeout
  *     Duration in milliseconds of when to interrupt and abort the
  *     script evaluation.
  * @param {string=} sandbox
  *     Name of the sandbox to evaluate the script in.  The sandbox is
  *     cached for later re-use on the same Window object if
  *     <var>newSandbox</var> is false.  If he parameter is undefined,
  *     the script is evaluated in a mutable sandbox.  If the parameter
  *     is "system", it will be evaluted in a sandbox with elevated system
@@ -867,31 +868,28 @@ GeckoDriver.prototype.getContext = funct
  *
  * @throws {ScriptTimeoutError}
  *     If the script was interrupted due to reaching the
  *     <var>scriptTimeout</var> or default timeout.
  * @throws {JavaScriptError}
  *     If an {@link Error} was thrown whilst evaluating the script.
  */
 GeckoDriver.prototype.executeScript = async function(cmd, resp) {
-  assert.open(this.getCurrentWindow());
-
-  let {script, args, scriptTimeout} = cmd.parameters;
-  scriptTimeout = scriptTimeout || this.timeouts.script;
-
+  let {script, args} = cmd.parameters;
   let opts = {
+    script: cmd.parameters.script,
+    args: cmd.parameters.args,
+    timeout: cmd.parameters.scriptTimeout,
     sandboxName: cmd.parameters.sandbox,
-    newSandbox: !!(typeof cmd.parameters.newSandbox == "undefined") ||
-        cmd.parameters.newSandbox,
+    newSandbox: cmd.parameters.newSandbox,
     file: cmd.parameters.filename,
     line: cmd.parameters.line,
     debug: cmd.parameters.debug_script,
   };
-
-  resp.body.value = await this.execute_(script, args, scriptTimeout, opts);
+  resp.body.value = await this.execute_(script, args, opts);
 };
 
 /**
  * Executes a JavaScript function in the context of the current browsing
  * context, if in content space, or in chrome space otherwise, and returns
  * the object passed to the callback.
  *
  * The callback is always the last argument to the <var>arguments</var>
@@ -944,54 +942,88 @@ GeckoDriver.prototype.executeScript = as
  *
  * @throws {ScriptTimeoutError}
  *     If the script was interrupted due to reaching the
  *     <var>scriptTimeout</var> or default timeout.
  * @throws {JavaScriptError}
  *     If an Error was thrown whilst evaluating the script.
  */
 GeckoDriver.prototype.executeAsyncScript = async function(cmd, resp) {
-  assert.open(this.getCurrentWindow());
-
-  let {script, args, scriptTimeout} = cmd.parameters;
-  scriptTimeout = scriptTimeout || this.timeouts.script;
-
+  let {script, args} = cmd.parameters;
   let opts = {
+    script: cmd.parameters.script,
+    args: cmd.parameters.args,
+    timeout: cmd.parameters.scriptTimeout,
     sandboxName: cmd.parameters.sandbox,
-    newSandbox: !!(typeof cmd.parameters.newSandbox == "undefined") ||
-        cmd.parameters.newSandbox,
+    newSandbox: cmd.parameters.newSandbox,
     file: cmd.parameters.filename,
     line: cmd.parameters.line,
     debug: cmd.parameters.debug_script,
     async: true,
   };
-
-  resp.body.value = await this.execute_(script, args, scriptTimeout, opts);
+  resp.body.value = await this.execute_(script, args, opts);
 };
 
 GeckoDriver.prototype.execute_ = async function(
-    script, args, timeout, opts = {}) {
+    script,
+    args = [],
+    {
+      timeout = null,
+      sandboxName = null,
+      newSandbox = false,
+      file = "",
+      line = 0,
+      debug = false,
+      async = false,
+    } = {}) {
+
+  if (typeof timeout == "undefined" || timeout === null) {
+    timeout = this.timeouts.script;
+  }
+
+  assert.open(this.getCurrentWindow());
+
+  assert.string(script, pprint`Expected "script" to be a string: ${script}`);
+  assert.array(args, pprint`Expected script args to be an array: ${args}`);
+  assert.positiveInteger(timeout, pprint`Expected script timeout to be a positive integer: ${timeout}`);
+  if (sandboxName !== null) {
+    assert.string(sandboxName, pprint`Expected sandbox name to be a string: ${sandboxName}`);
+  }
+  assert.boolean(newSandbox, pprint`Expected newSandbox to be boolean: ${newSandbox}`);
+  assert.string(file, pprint`Expected file to be a string: ${file}`);
+  assert.number(line, pprint`Expected line to be a number: ${line}`);
+  assert.boolean(debug, pprint`Expected debug_script to be boolean: ${debug}`);
+
+  let opts = {
+    timeout,
+    sandboxName,
+    newSandbox,
+    file,
+    line,
+    debug,
+    async,
+  };
+
   let res, els;
 
   switch (this.context) {
     case Context.Content:
       // evaluate in content with lasting side-effects
-      if (!opts.sandboxName) {
-        res = await this.listener.execute(script, args, timeout, opts);
+      if (!sandboxName) {
+        res = await this.listener.execute(script, args, opts);
 
       // evaluate in content with sandbox
       } else {
-        res = await this.listener.executeInSandbox(
-            script, args, timeout, opts);
+        res = await this.listener.executeInSandbox(script, args, opts);
       }
 
       break;
 
     case Context.Chrome:
-      let sb = this.sandboxes.get(opts.sandboxName, opts.newSandbox);
+      let sb = this.sandboxes.get(sandboxName, newSandbox);
       opts.timeout = timeout;
       let wargs = evaluate.fromJSON(args, this.curBrowser.seenEls, sb.window);
       res = await evaluate.sandbox(sb, script, wargs, opts);
       els = this.curBrowser.seenEls;
       break;
 
     default:
       throw new TypeError(`Unknown context: ${this.context}`);
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -663,24 +663,22 @@ function sendOk(uuid) {
  *     Error to notify chrome of.
  * @param {UUID} uuid
  *     Unique identifier of the request.
  */
 function sendError(err, uuid) {
   sendToServer(uuid, err);
 }
 
-async function execute(script, args, timeout, opts) {
-  opts.timeout = timeout;
+async function execute(script, args, opts) {
   let sb = sandbox.createMutable(curContainer.frame);
   return evaluate.sandbox(sb, script, args, opts);
 }
 
-async function executeInSandbox(script, args, timeout, opts) {
-  opts.timeout = timeout;
+async function executeInSandbox(script, args, opts) {
   let sb = sandboxes.get(opts.sandboxName, opts.newSandbox);
   return evaluate.sandbox(sb, script, args, opts);
 }
 
 function emitTouchEvent(type, touch) {
   logger.info(`Emitting Touch event of type ${type} ` +
       `to element with id: ${touch.target.id} ` +
       `and tag name: ${touch.target.tagName} ` +