Bug 1524243 - [marionette] Correctly handle script_timeout for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r=jgraham
authorHenrik Skupin <mail@hskupin.info>
Thu, 31 Jan 2019 14:25:26 +0000
changeset 456275 934099e1c866a27740a22d915535bd2e3e83bf4e
parent 456274 e688af125f982807e7080c45ce4bfb6ea2d4750b
child 456276 71f896682a2ff22e44b7a126281fcd84fe845e46
push id35478
push usershindli@mozilla.com
push dateFri, 01 Feb 2019 03:55:46 +0000
treeherdermozilla-central@945770882765 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgraham
bugs1524243, 1510929
milestone67.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 1524243 - [marionette] Correctly handle script_timeout for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r=jgraham This fixes the following regressions as introduced by bug 1510929 for the Marionette client. 1) The custom timeout as set isn't reset if the script times out. 2) Fractions of a second for the script timeout are lost because the division operates on decimal valus. As such a timeout of 100ms will result in 0ms. Differential Revision: https://phabricator.services.mozilla.com/D18214
testing/marionette/client/marionette_driver/marionette.py
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
@@ -1741,32 +1741,34 @@ class Marionette(object):
             marionette.execute_script("global.test1 = 'foo';")
             result = self.marionette.execute_script("return global.test1;", new_sandbox=False)
             assert result == "foo"
 
         """
         original_timeout = None
         if script_timeout is not None:
             original_timeout = self.timeout.script
-            self.timeout.script = script_timeout / 1000
+            self.timeout.script = script_timeout / 1000.0
 
-        args = self._to_json(script_args)
-        stack = traceback.extract_stack()
-        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,
-                "line": int(frame[1]),
-                "filename": filename}
-        rv = self._send_message("WebDriver:ExecuteScript", body, key="value")
+        try:
+            args = self._to_json(script_args)
+            stack = traceback.extract_stack()
+            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,
+                    "line": int(frame[1]),
+                    "filename": filename}
+            rv = self._send_message("WebDriver:ExecuteScript", body, key="value")
 
-        if script_timeout is not None:
-            self.timeout.script = original_timeout
+        finally:
+            if script_timeout is not None:
+                self.timeout.script = original_timeout
 
         return self._from_json(rv)
 
     def execute_async_script(self, script, script_args=(), new_sandbox=True,
                              sandbox="default", script_timeout=None):
         """Executes an asynchronous JavaScript script, and returns the
         result (or None if the script does return a value).
 
@@ -1798,33 +1800,35 @@ class Marionette(object):
                 resolve(1);
               }, 5000);
             ''')
             assert result == 1
         """
         original_timeout = None
         if script_timeout is not None:
             original_timeout = self.timeout.script
-            self.timeout.script = script_timeout / 1000
+            self.timeout.script = script_timeout / 1000.0
 
-        args = self._to_json(script_args)
-        stack = traceback.extract_stack()
-        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}
-        rv = self._send_message("WebDriver:ExecuteAsyncScript", body, key="value")
+        try:
+            args = self._to_json(script_args)
+            stack = traceback.extract_stack()
+            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}
+            rv = self._send_message("WebDriver:ExecuteAsyncScript", body, key="value")
 
-        if script_timeout is not None:
-            self.timeout.script = original_timeout
+        finally:
+            if script_timeout is not None:
+                self.timeout.script = original_timeout
 
         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.
 
--- 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
@@ -11,44 +11,55 @@ from marionette_harness import Marionett
 
 
 class TestExecuteAsyncContent(MarionetteTestCase):
     def setUp(self):
         super(TestExecuteAsyncContent, self).setUp()
         self.marionette.timeout.script = 1
 
     def test_execute_async_simple(self):
-        self.assertEqual(1, self.marionette.execute_async_script("arguments[arguments.length-1](1);"))
+        self.assertEqual(1,
+                         self.marionette.execute_async_script("arguments[arguments.length-1](1);"))
 
     def test_execute_async_ours(self):
         self.assertEqual(1, self.marionette.execute_async_script("arguments[0](1);"))
 
-    def test_execute_async_timeout(self):
-        self.assertRaises(ScriptTimeoutException, self.marionette.execute_async_script, "var x = 1;")
+    def test_script_timeout_error(self):
+        with self.assertRaisesRegexp(ScriptTimeoutException, "Timed out after 100 ms"):
+            self.marionette.execute_async_script("var x = 1;", script_timeout=100)
 
-    def test_execute_async_unique_timeout(self):
-        self.assertEqual(2, self.marionette.execute_async_script("setTimeout(() => arguments[0](2), 2000);", script_timeout=5000))
-        self.assertRaises(ScriptTimeoutException, self.marionette.execute_async_script, "setTimeout(() => arguments[0](3), 2000);")
+    def test_script_timeout_reset_after_timeout_error(self):
+        script_timeout = self.marionette.timeout.script
+        with self.assertRaises(ScriptTimeoutException):
+            self.marionette.execute_async_script("var x = 1;", script_timeout=100)
+        self.assertEqual(self.marionette.timeout.script, script_timeout)
+
+    def test_script_timeout_no_timeout_error(self):
+        self.assertTrue(self.marionette.execute_async_script("""
+            var callback = arguments[arguments.length - 1];
+            setTimeout(function() { callback(true); }, 500);
+            """, script_timeout=1000))
 
     def test_no_timeout(self):
         self.marionette.timeout.script = 10
         self.assertTrue(self.marionette.execute_async_script("""
             var callback = arguments[arguments.length - 1];
             setTimeout(function() { callback(true); }, 500);
             """))
 
     def test_execute_async_unload(self):
         self.marionette.timeout.script = 5
         unload = """
                 window.location.href = "about:blank";
                  """
         self.assertRaises(JavascriptException, self.marionette.execute_async_script, unload)
 
     def test_check_window(self):
-        self.assertTrue(self.marionette.execute_async_script("arguments[0](window != null && window != undefined);"))
+        self.assertTrue(self.marionette.execute_async_script(
+            "arguments[0](window != null && window != undefined);"))
 
     def test_same_context(self):
         var1 = 'testing'
         self.assertEqual(self.marionette.execute_script("""
             this.testvar = '{}';
             return this.testvar;
             """.format(var1)), var1)
         self.assertEqual(self.marionette.execute_async_script(
@@ -96,22 +107,22 @@ arguments[0](4);
         self.marionette.execute_async_script("this.foobar = [23, 42];"
                                              "arguments[0]();")
         self.assertEqual(self.marionette.execute_async_script(
             "arguments[0](this.foobar);", new_sandbox=False), [23, 42])
 
     def test_sandbox_refresh_arguments(self):
         self.marionette.execute_async_script("this.foobar = [arguments[0], arguments[1]];"
                                              "let resolve = "
-                                                 "arguments[arguments.length - 1];"
+                                             "arguments[arguments.length - 1];"
                                              "resolve();",
                                              script_args=[23, 42])
         self.assertEqual(self.marionette.execute_async_script(
             "arguments[0](this.foobar);", new_sandbox=False),
-                         [23, 42])
+            [23, 42])
 
     # Functions defined in higher privilege scopes, such as the privileged
     # content frame script listener.js runs in, cannot be accessed from
     # content.  This tests that it is possible to introspect the objects on
     # `arguments` without getting permission defined errors.  This is made
     # possible because the last argument is always the callback/complete
     # function.
     #
@@ -127,20 +138,20 @@ class TestExecuteAsyncChrome(TestExecute
         super(TestExecuteAsyncChrome, self).setUp()
         self.marionette.set_context("chrome")
 
     def test_execute_async_unload(self):
         pass
 
     def test_execute_permission(self):
         self.assertEqual(5, self.marionette.execute_async_script("""
-var c = Components.classes;
-arguments[0](5);
-"""))
+            var c = Components.classes;
+            arguments[0](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);
-            """)
+        with self.assertRaises(ScriptTimeoutException):
+            self.marionette.execute_async_script("""
+                var callback = arguments[arguments.length - 1];
+                setTimeout("callback(foo())", 50);
+                """)