Bug 1452597 - Remove debug_script functionality for injected scripts. r=maja_zf
authorAndreas Tolfsen <ato@sny.no>
Mon, 09 Apr 2018 13:13:18 +0100
changeset 780995 785c5c3cf010fe02f235a3084834154097508275
parent 780994 99b6961d14a25843b84aab592a7f0c0a7d86e45f
child 780996 633defe3cba479e3b482aa06589f5949d237b21e
push id106187
push userbmo:sledru@mozilla.com
push dateThu, 12 Apr 2018 11:43:14 +0000
reviewersmaja_zf
bugs1452597
milestone61.0a1
Bug 1452597 - Remove debug_script functionality for injected scripts. r=maja_zf The WebDriver:ExecuteScript and WebDriver:ExecuteAsyncScript commands accepts a "debug_script" parameter that attaches an error handler on the WindowProxy in the sandbox. This used to be necessary because the error handler used to be attached to the content window instead of the sandbox. MozReview-Commit-ID: ImRVkC5T75O
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/driver.js
testing/marionette/evaluate.js
testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1713,36 +1713,33 @@ class Marionette(object):
                 "sandbox": sandbox,
                 "scriptTimeout": script_timeout,
                 "line": int(frame[1]),
                 "filename": filename}
         rv = self._send_message("WebDriver:ExecuteScript", body, key="value")
         return self._from_json(rv)
 
     def execute_async_script(self, script, script_args=(), new_sandbox=True,
-                             sandbox="default", script_timeout=None,
-                             debug_script=False):
+                             sandbox="default", script_timeout=None):
         """Executes an asynchronous JavaScript script, and returns the
         result (or None if the script does return a value).
 
         The script is executed in the context set by the most recent
         :func:`set_context` call, or to the CONTEXT_CONTENT context if
         :func:`set_context` has not been called.
 
         :param script: A string containing the JavaScript to execute.
         :param script_args: An interable of arguments to pass to the script.
         :param sandbox: A tag referring to the sandbox you wish to use; if
             you specify a new tag, a new sandbox will be created.  If you
             use the special tag `system`, the sandbox will be created
             using the system principal which has elevated privileges.
         :param new_sandbox: If False, preserve global variables from
             the last execute_*script call. This is True by default,
             in which case no globals are preserved.
-        :param debug_script: Capture javascript exceptions when in
-            `CONTEXT_CHROME` context.
 
         Usage example:
 
         ::
 
             marionette.timeout.script = 10
             result = self.marionette.execute_async_script('''
               // this script waits 5 seconds, and then returns the number 1
@@ -1757,21 +1754,19 @@ class Marionette(object):
         frame = stack[-2:-1][0]  # grab the second-to-last frame
         filename = frame[0] if sys.platform == "win32" else os.path.relpath(frame[0])
         body = {"script": script.strip(),
                 "args": args,
                 "newSandbox": new_sandbox,
                 "sandbox": sandbox,
                 "scriptTimeout": script_timeout,
                 "line": int(frame[1]),
-                "filename": filename,
-                "debug_script": debug_script}
+                "filename": filename}
 
-        rv = self._send_message("WebDriver:ExecuteAsyncScript",
-                                body, key="value")
+        rv = self._send_message("WebDriver:ExecuteAsyncScript", body, key="value")
         return self._from_json(rv)
 
     def find_element(self, method, target, id=None):
         """Returns an :class:`~marionette_driver.marionette.HTMLElement`
         instance that matches the specified method and target in the current
         context.
 
         An :class:`~marionette_driver.marionette.HTMLElement` instance may be
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -849,19 +849,16 @@ GeckoDriver.prototype.getContext = funct
  * @param {boolean=} newSandbox
  *     Forces the script to be evaluated in a fresh sandbox.  Note that if
  *     it is undefined, the script will normally be evaluted in a fresh
  *     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.
  *
  * @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.
@@ -873,17 +870,16 @@ GeckoDriver.prototype.executeScript = as
   let opts = {
     script: cmd.parameters.script,
     args: cmd.parameters.args,
     timeout: cmd.parameters.scriptTimeout,
     sandboxName: cmd.parameters.sandbox,
     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, 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.
@@ -921,19 +917,16 @@ GeckoDriver.prototype.executeScript = as
  * @param {boolean=} newSandbox
  *     Forces the script to be evaluated in a fresh sandbox.  Note that if
  *     it is undefined, the script will normally be evaluted in a fresh
  *     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.
  *
  * @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.
@@ -945,32 +938,30 @@ GeckoDriver.prototype.executeAsyncScript
   let opts = {
     script: cmd.parameters.script,
     args: cmd.parameters.args,
     timeout: cmd.parameters.scriptTimeout,
     sandboxName: cmd.parameters.sandbox,
     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, opts);
 };
 
 GeckoDriver.prototype.execute_ = async function(
     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());
@@ -979,25 +970,23 @@ GeckoDriver.prototype.execute_ = async f
   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
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -64,18 +64,16 @@ this.evaluate = {};
  *     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
  *     Indicates if the script should return immediately or wait for
  *     the callback to be invoked before returning.
- * @param {boolean=} [debug=false] debug
- *     Attaches an <code>onerror</code> event listener.
  * @param {string=} [file="dummy file"] file
  *     File location of the program in the client.
  * @param {number=} [line=0] line
  *     Line number of th eprogram in the client.
  * @param {string=} sandboxName
  *     Name of the sandbox.  Elevated system privileges, equivalent to
  *     chrome space, will be given if it is <tt>system</tt>.
  * @param {number=} [timeout=DEFAULT_TIMEOUT] timeout
@@ -89,17 +87,16 @@ this.evaluate = {};
  * @throws {JavaScriptError}
  *   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,
       file = "dummy file",
       line = 0,
       sandboxName = null,
       timeout = DEFAULT_TIMEOUT,
     } = {}) {
   let scriptTimeoutID, timeoutHandler, unloadHandler;
 
   let promise = new Promise((resolve, reject) => {
@@ -126,27 +123,16 @@ evaluate.sandbox = function(sb, script, 
     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];
     }
 
-    // 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) => {
-        let err = new JavaScriptError(`${msg} at ${url}:${line}`);
-        reject(err);
-      };
-    }
-
     // timeout and unload handlers
     scriptTimeoutID = setTimeout(timeoutHandler, timeout);
     sb.window.onunload = unloadHandler;
 
     let res;
     try {
       res = Cu.evalInSandbox(src, sb, "1.8", file, line);
     } catch (e) {
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py
@@ -137,13 +137,8 @@ marionetteScriptFinished(5);
     def test_execute_async_js_exception(self):
         # Javascript exceptions are not propagated in chrome code
         self.marionette.timeout.script = 0.2
         self.assertRaises(ScriptTimeoutException,
             self.marionette.execute_async_script, """
             var callback = arguments[arguments.length - 1];
             setTimeout("callback(foo())", 50);
             """)
-        self.assertRaises(JavascriptException,
-            self.marionette.execute_async_script, """
-            var callback = arguments[arguments.length - 1];
-            setTimeout("callback(foo())", 50);
-            """, debug_script=True)