[wptrunner] Fix --leak-check plumbing for some Chrome-based products

Forward-fixes #47850

`ChromeBrowser` plumbs `--leak-check` via `ExecutorBrowser`, a view of
the browser (settings) from the executor process. Unfortunately, this
broke `ChromeDriverPrintRefTestExecutor` for `ChromeAndroidBrowserBase`
subclasses, since they inherit from `WebDriverBrowser`, not
`ChromeBrowser`, and therefore don't receive the plumbing.

Fix the layering by passing `--leak-check` to the `WebDriver*Executor`s
directly where the setting is consumed (disabled by default). Also,
introduce `ProtocolPart.after_connect()` so that the part can call other
parts after they're initialized. This avoids a hidden invariant that
`BaseProtocolPart` must be defined before `LeakProtocolPart`.

tools/wptrunner/wptrunner/browsers/chrome.py

@@ -61,8 +61,7 @@ def check_args(**kwargs):
 def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
     return {"binary": kwargs["binary"],
             "webdriver_binary": kwargs["webdriver_binary"],
-            "webdriver_args": kwargs.get("webdriver_args"),
-            "leak_check": kwargs.get("leak_check", False)}
+            "webdriver_args": kwargs.get("webdriver_args")}
 
 
 def executor_kwargs(logger, test_type, test_environment, run_info_data, subsuite,
@@ -74,6 +73,7 @@ def executor_kwargs(logger, test_type, test_environment, run_info_data, subsuite
                                            subsuite, **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["sanitizer_enabled"] = sanitizer_enabled
+    executor_kwargs["leak_check"] = kwargs.get("leak_check", False)
     executor_kwargs["reuse_window"] = kwargs.get("reuse_window", False)
 
     capabilities = {
@@ -209,10 +209,8 @@ def update_properties():
 class ChromeBrowser(WebDriverBrowser):
     def __init__(self,
                  logger: StructuredLogger,
-                 leak_check: bool = False,
                  **kwargs: Any):
         super().__init__(logger, **kwargs)
-        self._leak_check = leak_check
         self._actual_port = None
 
     def restart_on_test_type_change(self, new_test_type: str, old_test_type: str) -> bool:
@@ -258,10 +256,6 @@ def stop(self, force: bool = False, **kwargs: Any) -> bool:
         self._actual_port = None
         return super().stop(force=force, **kwargs)
 
-    def executor_browser(self):
-        browser_cls, browser_kwargs = super().executor_browser()
-        return browser_cls, {**browser_kwargs, "leak_check": self._leak_check}
-
 
 class ChromeDriverOutputHandler(OutputHandler):
     PORT_RE = re.compile(rb'.*was started successfully on port (\d+)\.')

tools/wptrunner/wptrunner/executors/executorchrome.py

@@ -218,6 +218,7 @@ class ChromeDriverProtocol(WebDriverProtocol):
     implements = [
         ChromeDriverDevToolsProtocolPart,
         ChromeDriverFedCMProtocolPart,
+        ChromeDriverLeakProtocolPart,
         ChromeDriverPrintProtocolPart,
         ChromeDriverTestharnessProtocolPart,
     ]
@@ -229,12 +230,6 @@ class ChromeDriverProtocol(WebDriverProtocol):
     # Prefix to apply to vendor-specific WebDriver extension commands.
     vendor_prefix = "goog"
 
-    def __init__(self, executor, browser, capabilities, **kwargs):
-        self.implements = list(ChromeDriverProtocol.implements)
-        if browser.leak_check:
-            self.implements.append(ChromeDriverLeakProtocolPart)
-        super().__init__(executor, browser, capabilities, **kwargs)
-
 
 def _evaluate_leaks(executor_cls):
     if hasattr(executor_cls, "base_convert_result"):

tools/wptrunner/wptrunner/executors/executorwebdriver.py

@@ -727,7 +727,7 @@ class WebDriverTestharnessExecutor(TestharnessExecutor):
 
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
                  close_after_done=True, capabilities=None, debug_info=None,
-                 cleanup_after_test=True, **kwargs):
+                 cleanup_after_test=True, leak_check=False, **kwargs):
         """WebDriver-based executor for testharness.js tests"""
         TestharnessExecutor.__init__(self, logger, browser, server_config,
                                      timeout_multiplier=timeout_multiplier,
@@ -747,6 +747,7 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1,
         self.close_after_done = close_after_done
         self.window_id = str(uuid.uuid4())
         self.cleanup_after_test = cleanup_after_test
+        self.leak_check = leak_check
 
     def is_alive(self):
         return self.protocol.is_alive()
@@ -876,8 +877,9 @@ async def process_bidi_event(method, params):
             protocol.loop.run_until_complete(protocol.bidi_events.unsubscribe_all())
 
         extra = {}
-        if (leak_part := getattr(protocol, "leak", None)) and (counters := leak_part.check()):
-            extra["leak_counters"] = counters
+        if self.leak_check and (leak_part := getattr(protocol, "leak", None)):
+            if counters := leak_part.check():
+                extra["leak_counters"] = counters
 
         # Attempt to clean up any leftover windows, if allowed. This is
         # preferable as it will blame the correct test if something goes wrong
@@ -940,7 +942,8 @@ class WebDriverRefTestExecutor(RefTestExecutor):
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
                  screenshot_cache=None, close_after_done=True,
                  debug_info=None, capabilities=None, debug_test=False,
-                 reftest_screenshot="unexpected", **kwargs):
+                 reftest_screenshot="unexpected", leak_check=False,
+                 **kwargs):
         """WebDriver-based executor for reftests"""
         RefTestExecutor.__init__(self,
                                  logger,
@@ -957,6 +960,7 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1,
         self.close_after_done = close_after_done
         self.has_window = False
         self.debug_test = debug_test
+        self.leak_check = leak_check
 
         with open(os.path.join(here, "test-wait.js")) as f:
             self.wait_script = f.read() % {"classname": "reftest-wait"}
@@ -984,8 +988,9 @@ def do_test(self, test):
 
         result = self.implementation.run_test(test)
 
-        if (leak_part := getattr(self.protocol, "leak", None)) and (counters := leak_part.check()):
-            result.setdefault("extra", {})["leak_counters"] = counters
+        if self.leak_check and (leak_part := getattr(self.protocol, "leak", None)):
+            if counters := leak_part.check():
+                result.setdefault("extra", {})["leak_counters"] = counters
 
         if self.debug_test and result["status"] in ["PASS", "FAIL", "ERROR"] and "extra" in result:
             self.protocol.debug.load_reftest_analyzer(test, result)
@@ -1024,7 +1029,8 @@ class WebDriverCrashtestExecutor(CrashtestExecutor):
 
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
                  screenshot_cache=None, close_after_done=True,
-                 debug_info=None, capabilities=None, **kwargs):
+                 debug_info=None, capabilities=None, leak_check=False,
+                 **kwargs):
         """WebDriver-based executor for crashtests"""
         CrashtestExecutor.__init__(self,
                                    logger,
@@ -1036,6 +1042,7 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1,
         self.protocol = self.protocol_cls(self,
                                           browser,
                                           capabilities=capabilities)
+        self.leak_check = leak_check
 
         with open(os.path.join(here, "test-wait.js")) as f:
             self.wait_script = f.read() % {"classname": "test-wait"}
@@ -1060,6 +1067,7 @@ def do_crashtest(self, protocol, url, timeout):
         protocol.base.load(url)
         protocol.base.execute_script(self.wait_script, asynchronous=True)
         result = {"status": "PASS", "message": None}
-        if (leak_part := getattr(protocol, "leak", None)) and (counters := leak_part.check()):
-            result["extra"] = {"leak_counters": counters}
+        if self.leak_check and (leak_part := getattr(protocol, "leak", None)):
+            if counters := leak_part.check():
+                result["extra"] = {"leak_counters": counters}
         return result

tools/wptrunner/wptrunner/executors/protocol.py

@@ -70,6 +70,8 @@ def setup(self, runner):
 
             msg = "Post-connection steps failed"
             self.after_connect()
+            for cls in self.implements:
+                getattr(self, cls.name).after_connect()
         except Exception:
             message = "Protocol.setup caught an exception:\n"
             message += f"{msg}\n" if msg is not None else ""
@@ -114,6 +116,11 @@ def setup(self):
         """Run any setup steps required for the ProtocolPart."""
         pass
 
+    def after_connect(self):
+        """Run any post-connection steps. This happens after the ProtocolParts are
+        initalized so can depend on a fully-populated object."""
+        pass
+
     def teardown(self):
         """Run any teardown steps required for the ProtocolPart."""
         pass
@@ -620,7 +627,7 @@ class LeakProtocolPart(ProtocolPart):
 
     name = "leak"
 
-    def setup(self):
+    def after_connect(self):
         self.parent.base.load("about:blank")
         self.expected_counters = collections.Counter(self.get_counters())