From 1fc258585c3d072a6d32bb6341cb61f75950d08f Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Tue, 3 Dec 2024 00:17:31 -0500 Subject: [PATCH 1/2] [wptrunner] Introduce `WebDriverTestDriverProtocolPart.get_next_message` ... for symmetry with `send_message()`, which the executor uses to settle async testdriver calls browser-side. This is a no-op refactor that gives vendors more flexibility how to exchange messages with the test page. --- .../wptrunner/executors/executorchrome.py | 28 +++-- .../wptrunner/executors/executorwebdriver.py | 112 +++++++++--------- 2 files changed, 73 insertions(+), 67 deletions(-) diff --git a/tools/wptrunner/wptrunner/executors/executorchrome.py b/tools/wptrunner/wptrunner/executors/executorchrome.py index e92d45094d1571..d04e668022777d 100644 --- a/tools/wptrunner/wptrunner/executors/executorchrome.py +++ b/tools/wptrunner/wptrunner/executors/executorchrome.py @@ -17,6 +17,7 @@ WebDriverPrintRefTestExecutor, WebDriverProtocol, WebDriverRefTestExecutor, + WebDriverTestDriverProtocolPart, WebDriverTestharnessExecutor, WebDriverTestharnessProtocolPart, ) @@ -94,6 +95,20 @@ def get_counters(self) -> Mapping[str, int]: return counters +class ChromeDriverTestDriverProtocolPart(WebDriverTestDriverProtocolPart): + def _get_next_message_classic(self, url): + try: + return super()._get_next_message_classic(url) + except error.JavascriptErrorException as js_error: + # TODO(crbug.com/340662810): Cycle testdriver event loop to work + # around `testharnessreport.js` flakily not loaded. + if re.search(r'window\.__wptrunner_process_next_event is not a function', + js_error.message): + time.sleep(0.05) + return None + raise + + class ChromeDriverTestharnessProtocolPart(WebDriverTestharnessProtocolPart): """Implementation of `testharness.js` tests controlled by ChromeDriver. @@ -156,6 +171,7 @@ class ChromeDriverProtocol(WebDriverProtocol): ChromeDriverBaseProtocolPart, ChromeDriverDevToolsProtocolPart, ChromeDriverFedCMProtocolPart, + ChromeDriverTestDriverProtocolPart, ChromeDriverTestharnessProtocolPart, ] for base_part in WebDriverProtocol.implements: @@ -246,18 +262,6 @@ def get_or_create_test_window(self, protocol): self.protocol.testharness.persistent_test_window = test_window return test_window - def _get_next_message_classic(self, protocol, url, test_window): - try: - return super()._get_next_message_classic(protocol, url, test_window) - except error.JavascriptErrorException as js_error: - # TODO(crbug.com/340662810): Cycle testdriver event loop to work - # around `testharnessreport.js` flakily not loaded. - if re.search(r'window\.__wptrunner_process_next_event is not a function', - js_error.message): - time.sleep(0.05) - return None - raise - @_evaluate_sanitized_result class ChromeDriverPrintRefTestExecutor(WebDriverPrintRefTestExecutor): diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 243290841f1b73..24aa1de2248ec3 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -433,6 +433,55 @@ def release(self): class WebDriverTestDriverProtocolPart(TestDriverProtocolPart): def setup(self): self.webdriver = self.parent.webdriver + with open(os.path.join(here, "testharness_webdriver_resume.js")) as f: + self.script_resume = f.read() + + def get_next_message(self, url, test_window): + if hasattr(self.parent, "bidi_script"): + # If `bidi_script` is available, the messages can be handled via BiDi. + return self._get_next_message_bidi(url, test_window) + else: + return self._get_next_message_classic(url) + + def _get_next_message_classic(self, url): + """ + Get the next message from the test_driver using the classic WebDriver async script execution. This will block + the event loop until the test_driver send a message. + """ + return self.parent.base.execute_script(self.script_resume, asynchronous=True, args=[strip_server(url)]) + + def _get_next_message_bidi(self, url, test_window): + """ + Get the next message from the test_driver using async call. This will not block the event loop, which allows for + processing the events from the test_runner to test_driver while waiting for the next test_driver commands. + """ + # As long as we want to be able to use scripts both in bidi and in classic mode, the script should + # be wrapped to some harness to emulate the WebDriver Classic async script execution. The script + # will be provided with the `resolve` delegate, which finishes the execution. After that the + # coroutine is finished as well. + wrapped_script = """async function(...args){ + return new Promise((resolve, reject) => { + args.push(resolve); + (async function(){ + %s + }).apply(null, args); + }) + }""" % self.script_resume + + bidi_url_argument = { + "type": "string", + "value": strip_server(url) + } + + # `run_until_complete` allows processing BiDi events in the same loop while waiting for the next message. + message = self.parent.loop.run_until_complete(self.parent.bidi_script.call_function( + wrapped_script, target={ + "context": test_window + }, + arguments=[bidi_url_argument])) + # The message is in WebDriver BiDi format. Deserialize it. + deserialized_message = bidi_deserialize(message) + return deserialized_message def send_message(self, cmd_id, message_type, status, message=None): obj = { @@ -782,7 +831,6 @@ def run_func(self): class WebDriverTestharnessExecutor(TestharnessExecutor): supports_testdriver = True protocol_cls = WebDriverProtocol - _get_next_message = None def __init__(self, logger, browser, server_config, timeout_multiplier=1, close_after_done=True, capabilities=None, debug_info=None, @@ -792,16 +840,9 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1, timeout_multiplier=timeout_multiplier, debug_info=debug_info) self.protocol = self.protocol_cls(self, browser, capabilities) - with open(os.path.join(here, "testharness_webdriver_resume.js")) as f: - self.script_resume = f.read() with open(os.path.join(here, "window-loaded.js")) as f: self.window_loaded_script = f.read() - if hasattr(self.protocol, 'bidi_script'): - # If `bidi_script` is available, the messages can be handled via BiDi. - self._get_next_message = self._get_next_message_bidi - else: - self._get_next_message = self._get_next_message_classic self.close_after_done = close_after_done self.cleanup_after_test = cleanup_after_test @@ -855,11 +896,13 @@ async def process_bidi_event(method, params): self.logger.debug(f"Received bidi event: {method}, {params}") if hasattr(protocol, 'bidi_browsing_context') and method == "browsingContext.userPromptOpened" and \ params["context"] == test_window: - # User prompts of the test window are handled separately. In classic implementation, this user - # prompt always causes an exception when `_get_next_message` is called. In BiDi it's not a case, - # as the BiDi protocol allows sending commands even with the user prompt opened. However, the - # user prompt can block the testdriver JS execution and cause the dead loop. To overcome this - # issue, the user prompt of the test window is always dismissed and the test is failing. + # User prompts of the test window are handled separately. In classic + # implementation, this user prompt always causes an exception when + # `protocol.testdriver.get_next_message()` is called. In BiDi it's not the + # case, as the BiDi protocol allows sending commands even with the user + # prompt opened. However, the user prompt can block the testdriver JS + # execution and cause a dead loop. To overcome this issue, the user prompt + # of the test window is always dismissed and the test is failing. try: await protocol.bidi_browsing_context.handle_user_prompt(params["context"]) except Exception as e: @@ -896,7 +939,7 @@ async def process_bidi_event(method, params): # TODO: what to do if there are more then 1 unexpected exceptions? raise unexpected_exceptions[0] - test_driver_message = self._get_next_message(protocol, url, test_window) + test_driver_message = protocol.testdriver.get_next_message(url, test_window) self.logger.debug("Receive message from testdriver: %s" % test_driver_message) # As of 2019-03-29, WebDriver does not define expected behavior for @@ -960,47 +1003,6 @@ async def process_bidi_event(method, params): def get_or_create_test_window(self, protocol): return protocol.base.create_window() - def _get_next_message_classic(self, protocol, url, _): - """ - Get the next message from the test_driver using the classic WebDriver async script execution. This will block - the event loop until the test_driver send a message. - :param window: - """ - return protocol.base.execute_script(self.script_resume, asynchronous=True, args=[strip_server(url)]) - - def _get_next_message_bidi(self, protocol, url, test_window): - """ - Get the next message from the test_driver using async call. This will not block the event loop, which allows for - processing the events from the test_runner to test_driver while waiting for the next test_driver commands. - """ - # As long as we want to be able to use scripts both in bidi and in classic mode, the script should - # be wrapped to some harness to emulate the WebDriver Classic async script execution. The script - # will be provided with the `resolve` delegate, which finishes the execution. After that the - # coroutine is finished as well. - wrapped_script = """async function(...args){ - return new Promise((resolve, reject) => { - args.push(resolve); - (async function(){ - %s - }).apply(null, args); - }) - }""" % self.script_resume - - bidi_url_argument = { - "type": "string", - "value": strip_server(url) - } - - # `run_until_complete` allows processing BiDi events in the same loop while waiting for the next message. - message = protocol.loop.run_until_complete(protocol.bidi_script.call_function( - wrapped_script, target={ - "context": test_window - }, - arguments=[bidi_url_argument])) - # The message is in WebDriver BiDi format. Deserialize it. - deserialized_message = bidi_deserialize(message) - return deserialized_message - class WebDriverRefTestExecutor(RefTestExecutor): protocol_cls = WebDriverProtocol From 6fea24dbcc6d206724c223e0652c0b00749bf842 Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Tue, 3 Dec 2024 16:21:52 -0500 Subject: [PATCH 2/2] [wptrunner] Lazily post testdriver result while polling the next message Currently, a typical testdriver call takes three WebDriver commands: 1. Poll the next browser -> executor message via the resume script 2. Execute the requested testdriver command (e.g., "element click" for `test_driver.click()`) 3. Post the result of (2) from executor -> browser, which resolves the call's promise The testharness executor repeats the above steps in an event loop until a message from (1) indicates the test is complete (or a helper thread times out the loop). This PR optimizes the speed of testdriver-heavy tests by combining (3) and (1) for pairs of consecutive calls by running them in the same "execute async script" invocation [a]. This PR should be entirely transparent to tests because the browser should eventually see all `send_message()` calls in the same order, with no changes in testdriver semantics. When tested downstream, no tests regress [b], and the following local test sees a modest speedup from 16 to 15.5 min: ```sh run_headless_shell_wpt external/wpt/editing/other/join-different-white-space-style-paragraphs.html -vv -j 1 ``` [a]: https://w3c.github.io/webdriver/#execute-async-script [b]: https://chromium-review.googlesource.com/c/chromium/src/+/6068186/1?tab=checks See Also: https://crbug.com/381927516 --- .../wptrunner/executors/executorchrome.py | 20 ++++++++++++++++++- .../wptrunner/executors/executorwebdriver.py | 9 +++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tools/wptrunner/wptrunner/executors/executorchrome.py b/tools/wptrunner/wptrunner/executors/executorchrome.py index d04e668022777d..9534eaea17146a 100644 --- a/tools/wptrunner/wptrunner/executors/executorchrome.py +++ b/tools/wptrunner/wptrunner/executors/executorchrome.py @@ -10,6 +10,7 @@ from webdriver import error +from .base import strip_server from .executorwebdriver import ( WebDriverBaseProtocolPart, WebDriverCrashtestExecutor, @@ -96,9 +97,26 @@ def get_counters(self) -> Mapping[str, int]: class ChromeDriverTestDriverProtocolPart(WebDriverTestDriverProtocolPart): + """An interface to the browser-side testdriver infrastructure that lazily settles calls.""" + + def setup(self): + super().setup() + self._pending_message = "" + + def send_message(self, cmd_id, message_type, status, message=None): + message_script = self._format_send_message_script(cmd_id, message_type, status, message) + if message_type == "complete": + assert not self._pending_message, self._pending_message + self._pending_message = message_script + else: + self.webdriver.execute_script(message_script) + def _get_next_message_classic(self, url): try: - return super()._get_next_message_classic(url) + message_script, self._pending_message = self._pending_message, "" + return self.parent.base.execute_script(message_script + self.script_resume, + asynchronous=True, + args=[strip_server(url)]) except error.JavascriptErrorException as js_error: # TODO(crbug.com/340662810): Cycle testdriver event loop to work # around `testharnessreport.js` flakily not loaded. diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 24aa1de2248ec3..ed145823e2b900 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -484,14 +484,19 @@ def _get_next_message_bidi(self, url, test_window): return deserialized_message def send_message(self, cmd_id, message_type, status, message=None): + self.webdriver.execute_script( + self._format_send_message_script(cmd_id, message_type, status, message)) + + def _format_send_message_script(self, cmd_id, message_type, status, message=None): obj = { "cmd_id": cmd_id, - "type": "testdriver-%s" % str(message_type), + "type": f"testdriver-{message_type}", "status": str(status) } if message: obj["message"] = str(message) - self.webdriver.execute_script("window.postMessage(%s, '*')" % json.dumps(obj)) + return f"window.postMessage({json.dumps(obj)}, '*');" + def _switch_to_frame(self, index_or_elem): try: