Bug 1452487 - Remove directInject functionality in evaluate.sandbox. r?maja_zf draft
authorAndreas Tolfsen <ato@sny.no>
Sun, 08 Apr 2018 18:03:29 +0100
changeset 779041 416636b60834bca4a35258fc1df4d28510912507
parent 779038 cb2bb4a7c1c39a017aa8c24c30b62a5f48922677
push id105638
push userbmo:ato@sny.no
push dateSun, 08 Apr 2018 17:05:53 +0000
reviewersmaja_zf
bugs1452487
milestone61.0a1
Bug 1452487 - Remove directInject functionality in evaluate.sandbox. r?maja_zf The evaluate.sandbox function accepts a directInject argument, which is a relic from Marionette's B2G past when it did not support evaluating scripts with lasting side-effects. The API documentation in GeckoDriver#execute_ mentions directInject as a valid parameter, but it is not picked up or passed on to evaluate.sandbox. This effectively means the directInject functionality is unused. MozReview-Commit-ID: 3rYjRQ2R5GV
testing/marionette/driver.js
testing/marionette/evaluate.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -852,18 +852,16 @@ GeckoDriver.prototype.getContext = funct
  *     sandbox.
  * @param {string=} filename
  *     Filename of the client's program where this script is evaluated.
  * @param {number=} line
  *     Line in the client's program where this script is evaluated.
  * @param {boolean=} debug_script
  *     Attach an <code>onerror</code> event handler on the {@link Window}
  *     object.  It does not differentiate content errors from chrome errors.
- * @param {boolean=} directInject
- *     Evaluate the script without wrapping it in a function.
  *
  * @return {(string|boolean|number|object|WebElement)}
  *     Return value from the script, or null which signifies either the
  *     JavaScript notion of null or undefined.
  *
  * @throws {ScriptTimeoutError}
  *     If the script was interrupted due to reaching the
  *     <var>scriptTimeout</var> or default timeout.
@@ -926,18 +924,16 @@ GeckoDriver.prototype.executeScript = as
  *     sandbox.
  * @param {string=} filename
  *     Filename of the client's program where this script is evaluated.
  * @param {number=} line
  *     Line in the client's program where this script is evaluated.
  * @param {boolean=} debug_script
  *     Attach an <code>onerror</code> event handler on the {@link Window}
  *     object.  It does not differentiate content errors from chrome errors.
- * @param {boolean=} directInject
- *     Evaluate the script without wrapping it in a function.
  *
  * @return {(string|boolean|number|object|WebElement)}
  *     Return value from the script, or null which signifies either the
  *     JavaScript notion of null or undefined.
  *
  * @throws {ScriptTimeoutError}
  *     If the script was interrupted due to reaching the
  *     <var>scriptTimeout</var> or default timeout.
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -31,47 +31,39 @@ const FINISH = "finish";
 const MARIONETTE_SCRIPT_FINISHED = "marionetteScriptFinished";
 
 /** @namespace */
 this.evaluate = {};
 
 /**
  * Evaluate a script in given sandbox.
  *
- * If the option var>directInject</var> is not specified, the script
- * will be executed as a function with the <var>args</var> argument
- * applied.
+ * The the provided `script` will be wrapped in an anonymous function
+ * with the `args` argument applied.
  *
- * The arguments provided by the <var>args</var> argument are exposed
- * through the <code>arguments</code> object available in the script
- * context, and if the script is executed asynchronously with the
- * <var>async</var> option, an additional last argument that is synonymous
- * to the <code>marionetteScriptFinished</code> global is appended, and
- * can be accessed through <code>arguments[arguments.length - 1]</code>.
+ * The arguments provided by the `args<` argument are exposed
+ * through the `arguments` object available in the script context,
+ * and if the script is executed asynchronously with the `async`
+ * option, an additional last argument that is synonymous to the
+ * `marionetteScriptFinished` global is appended, and can be accessed
+ * through `arguments[arguments.length - 1]`.
  *
- * The <var>timeout</var> option specifies the duration for how long
- * the script should be allowed to run before it is interrupted and aborted.
+ * The `timeout` option specifies the duration for how long the
+ * script should be allowed to run before it is interrupted and aborted.
  * An interrupted script will cause a {@link ScriptTimeoutError} to occur.
  *
- * The <var>async</var> option indicates that the script will
- * not return until the <code>marionetteScriptFinished</code> global
- * callback is invoked, which is analogous to the last argument of the
- * <code>arguments</code> object.
+ * The `async` option indicates that the script will not return
+ * until the `marionetteScriptFinished` global callback is invoked,
+ * which is analogous to the last argument of the `arguments` object.
  *
- * The option <var>directInject</var> causes the script to be evaluated
- * without being wrapped in a function and the provided arguments will
- * be disregarded.  This will cause such things as root scope return
- * statements to throw errors because they are not used inside a function.
+ * The `file` option is used in error messages to provide information
+ * on the origin script file in the local end.
  *
- * The <var>file</var> option is used in error messages to provide
- * information on the origin script file in the local end.
- *
- * The <var>line</var> option is used in error messages, along with
- * <var>filename</var>, to provide the line number in the origin script
- * file on the local end.
+ * The `line` option is used in error messages, along with `filename`,
+ * to provide the line number in the origin script file on the local end.
  *
  * @param {nsISandbox} sb
  *     Sandbox the script will be evaluted in.
  * @param {string} script
  *     Script to evaluate.
  * @param {Array.<?>=} args
  *     A sequence of arguments to call the script with.
  * @param {boolean=} [async=false] async
@@ -98,54 +90,50 @@ this.evaluate = {};
  *   If an {@link Error} was thrown whilst evaluating the script.
  * @throws {ScriptTimeoutError}
  *   If the script was interrupted due to script timeout.
  */
 evaluate.sandbox = function(sb, script, args = [],
     {
       async = false,
       debug = false,
-      directInject = false,
       file = "dummy file",
       line = 0,
       sandboxName = null,
       timeout = DEFAULT_TIMEOUT,
     } = {}) {
   let scriptTimeoutID, timeoutHandler, unloadHandler;
 
   let promise = new Promise((resolve, reject) => {
     let src = "";
     sb[COMPLETE] = resolve;
     timeoutHandler = () => reject(new ScriptTimeoutError("Timed out"));
     unloadHandler = sandbox.cloneInto(
         () => reject(new JavaScriptError("Document was unloaded")),
         sb);
 
-    // wrap in function
-    if (!directInject) {
-      if (async) {
-        sb[CALLBACK] = sb[COMPLETE];
-      }
-      sb[ARGUMENTS] = sandbox.cloneInto(args, sb);
+    if (async) {
+      sb[CALLBACK] = sb[COMPLETE];
+    }
+    sb[ARGUMENTS] = sandbox.cloneInto(args, sb);
 
-      // callback function made private
-      // so that introspection is possible
-      // on the arguments object
-      if (async) {
-        sb[CALLBACK] = sb[COMPLETE];
-        src += `${ARGUMENTS}.push(rv => ${CALLBACK}(rv));`;
-      }
+    // callback function made private
+    // so that introspection is possible
+    // on the arguments object
+    if (async) {
+      sb[CALLBACK] = sb[COMPLETE];
+      src += `${ARGUMENTS}.push(rv => ${CALLBACK}(rv));`;
+    }
 
-      src += `(function() { ${script} }).apply(null, ${ARGUMENTS})`;
+    src += `(function() { ${script} }).apply(null, ${ARGUMENTS})`;
 
-      // marionetteScriptFinished is not WebDriver conformant,
-      // hence it is only exposed to immutable sandboxes
-      if (sandboxName) {
-        sb[MARIONETTE_SCRIPT_FINISHED] = sb[CALLBACK];
-      }
+    // marionetteScriptFinished is not WebDriver conformant,
+    // hence it is only exposed to immutable sandboxes
+    if (sandboxName) {
+      sb[MARIONETTE_SCRIPT_FINISHED] = sb[CALLBACK];
     }
 
     // onerror is not hooked on by default because of the inability to
     // differentiate content errors from chrome errors.
     //
     // see bug 1128760 for more details
     if (debug) {
       sb.window.onerror = (msg, url, line) => {