Bug 1524243 - [marionette] Correctly handle script_timeout for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r=jgraham a=test-only
authorHenrik Skupin <mail@hskupin.info>
Thu, 31 Jan 2019 14:25:26 +0000
changeset 512836 9cf59044f067672a749b8bc8a68321893c96d97d
parent 512835 a142aefab93abd44861b92ddb3b4fe7756fcbd22
child 512837 6eeb1301ae9f2c0aaac9314bf78df9d515e6b342
push id10616
push userarchaeopteryx@coole-files.de
push dateMon, 04 Feb 2019 16:57:19 +0000
treeherdermozilla-beta@e86470dd4c33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgraham, test-only
bugs1524243, 1510929
milestone66.0
Bug 1524243 - [marionette] Correctly handle script_timeout for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r=jgraham a=test-only 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);
+                """)