Skip to content

Commit

Permalink
[wptrunner] Implement --leak-check for Blink-based browsers (#47850)
Browse files Browse the repository at this point in the history
... to support Chromium CI testing needs.

* Introduce `LeakProtocolPart`, an optional API that lets wptrunner
  check for leaks programmatically. The default implementation checks
  that navigating to `about:blank` after a test cleans up the DOM
  objects it created.
* For browsers that implement `LeakProtocolPart`, the base WebDriver
  executors run the leak check after each test and report the results as
  an extra test result field named `leak_counters`.
* Currently, the only implementers of `LeakProtocolPart` are
  ChromeDriver-based browsers, which use a nonstandard Chrome DevTools
  Protocol method [0] to count DOM objects. For those browsers, a leak
  is coerced to CRASH to report the result appropriately and induce a
  browser restart.

[0]: https://chromedevtools.github.io/devtools-protocol/tot/Memory/#method-getDOMCountersForLeakDetection

See Also: https://crbug.com/40887057
  • Loading branch information
jonathan-j-lee authored Aug 29, 2024
1 parent eb162cb commit 980adbd
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 18 deletions.
10 changes: 9 additions & 1 deletion tools/wptrunner/wptrunner/browsers/chrome.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ 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")}
"webdriver_args": kwargs.get("webdriver_args"),
"leak_check": kwargs.get("leak_check", False)}


def executor_kwargs(logger, test_type, test_environment, run_info_data, subsuite,
Expand Down Expand Up @@ -208,8 +209,10 @@ 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:
Expand Down Expand Up @@ -255,6 +258,11 @@ 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+)\.')
NO_PORT_RE = re.compile(rb'.*was started successfully\.')
Expand Down
57 changes: 52 additions & 5 deletions tools/wptrunner/wptrunner/executors/executorchrome.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# mypy: allow-untyped-defs

import collections
import os
import time
import traceback
from typing import Type
from typing import Mapping, MutableMapping, Type
from urllib.parse import urljoin

from webdriver import error
Expand All @@ -22,7 +23,7 @@
WebDriverTestharnessExecutor,
WebDriverTestharnessProtocolPart,
)
from .protocol import PrintProtocolPart, ProtocolPart
from .protocol import LeakProtocolPart, PrintProtocolPart, ProtocolPart

here = os.path.dirname(__file__)

Expand Down Expand Up @@ -62,6 +63,19 @@ def convert_from_crashtest_result(test, result):
_SanitizerMixin = make_sanitizer_mixin(WebDriverCrashtestExecutor)


class ChromeDriverLeakProtocolPart(LeakProtocolPart):
def get_counters(self) -> Mapping[str, int]:
response = self.parent.cdp.execute_cdp_command("Memory.getDOMCountersForLeakDetection")
counters: MutableMapping[str, int] = collections.Counter({
counter["name"]: counter["count"]
for counter in response["counters"]
})
# Exclude resources associated with User Agent CSS from leak detection,
# since they are persisted through page navigation.
counters["live_resources"] -= counters.pop("live_ua_css_resources", 0)
return counters


class ChromeDriverTestharnessProtocolPart(WebDriverTestharnessProtocolPart):
"""Implementation of `testharness.js` tests controlled by ChromeDriver.
Expand Down Expand Up @@ -206,23 +220,54 @@ class ChromeDriverProtocol(WebDriverProtocol):
ChromeDriverFedCMProtocolPart,
ChromeDriverPrintProtocolPart,
ChromeDriverTestharnessProtocolPart,
*(part for part in WebDriverProtocol.implements
if part.name != ChromeDriverTestharnessProtocolPart.name and
part.name != ChromeDriverFedCMProtocolPart.name)
]
for base_part in WebDriverProtocol.implements:
if base_part.name not in {part.name for part in implements}:
implements.append(base_part)

reuse_window = False
# 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"):
# Don't wrap more than once, which can cause unbounded recursion.
return executor_cls
executor_cls.base_convert_result = executor_cls.convert_result

def convert_result(self, test, result, **kwargs):
test_result, subtest_results = self.base_convert_result(test, result, **kwargs)
if test_result.extra.get("leak_counters"):
test_result = test.make_result("CRASH",
test_result.message,
test_result.expected,
test_result.extra,
test_result.stack,
test_result.known_intermittent)
return test_result, subtest_results

executor_cls.convert_result = convert_result
return executor_cls


@_evaluate_leaks
class ChromeDriverCrashTestExecutor(WebDriverCrashtestExecutor):
protocol_cls = ChromeDriverProtocol


@_evaluate_leaks
class ChromeDriverRefTestExecutor(WebDriverRefTestExecutor, _SanitizerMixin): # type: ignore
protocol_cls = ChromeDriverProtocol


@_evaluate_leaks
class ChromeDriverTestharnessExecutor(WebDriverTestharnessExecutor, _SanitizerMixin): # type: ignore
protocol_cls = ChromeDriverProtocol

Expand All @@ -249,8 +294,10 @@ def setup(self, runner, protocol=None):
self.protocol.cdp.execute_cdp_command("Browser.setPermission", params)


@_evaluate_leaks
class ChromeDriverPrintRefTestExecutor(ChromeDriverRefTestExecutor):
protocol_cls = ChromeDriverProtocol
is_print = True

def setup(self, runner, protocol=None):
super().setup(runner, protocol)
Expand Down
20 changes: 14 additions & 6 deletions tools/wptrunner/wptrunner/executors/executorwebdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,8 @@ def do_test(self, test):
self.extra_timeout).run()

if success:
return self.convert_result(test, data)
data, extra = data
return self.convert_result(test, data, extra=extra)

return (test.make_result(*data), [])

Expand Down Expand Up @@ -874,6 +875,10 @@ async def process_bidi_event(method, params):
# Use protocol loop to run the async cleanup.
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

# Attempt to clean up any leftover windows, if allowed. This is
# preferable as it will blame the correct test if something goes wrong
# closing windows, but if the user wants to see the test results we
Expand All @@ -885,7 +890,7 @@ async def process_bidi_event(method, params):
# TODO: what to do if there are more then 1 unexpected exceptions?
raise unexpected_exceptions[0]

return rv
return rv, extra

def _get_next_message_classic(self, protocol, url, _):
"""
Expand Down Expand Up @@ -979,6 +984,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.debug_test and result["status"] in ["PASS", "FAIL", "ERROR"] and "extra" in result:
self.protocol.debug.load_reftest_analyzer(test, result)

Expand All @@ -998,7 +1006,6 @@ def screenshot(self, test, viewport_size, dpi, page_ranges):

def _screenshot(self, protocol, url, timeout):
self.protocol.base.load(url)

self.protocol.base.execute_script(self.wait_script, True)

screenshot = self.protocol.webdriver.screenshot()
Expand Down Expand Up @@ -1052,6 +1059,7 @@ def do_test(self, test):
def do_crashtest(self, protocol, url, timeout):
protocol.base.load(url)
protocol.base.execute_script(self.wait_script, asynchronous=True)

return {"status": "PASS",
"message": None}
result = {"status": "PASS", "message": None}
if (leak_part := getattr(protocol, "leak", None)) and (counters := leak_part.check()):
result["extra"] = {"leak_counters": counters}
return result
34 changes: 33 additions & 1 deletion tools/wptrunner/wptrunner/executors/protocol.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# mypy: allow-untyped-defs

import collections
import traceback
from http.client import HTTPConnection

from abc import ABCMeta, abstractmethod
from typing import Any, Awaitable, Callable, ClassVar, List, Mapping, Optional, Type
from typing import Any, Awaitable, Callable, ClassVar, List, Mapping, Optional, Tuple, Type


def merge_dicts(target, source):
Expand Down Expand Up @@ -613,6 +614,37 @@ def get(self):
pass


class LeakProtocolPart(ProtocolPart):
"""Protocol part that checks for leaked DOM objects."""
__metaclass__ = ABCMeta

name = "leak"

def setup(self):
self.parent.base.load("about:blank")
self.expected_counters = collections.Counter(self.get_counters())

@abstractmethod
def get_counters(self) -> Mapping[str, int]:
"""Get counts of types of live objects (names are browser-dependent)."""

def check(self) -> Optional[Mapping[str, Tuple[int, int]]]:
"""Check for DOM objects that outlive the current page.
Returns:
A map from object type to (expected, actual) counts, if one or more
types leaked. Otherwise, `None`.
"""
self.parent.base.load("about:blank")
counters = collections.Counter(self.get_counters())
if counters - self.expected_counters:
return {
name: (self.expected_counters[name], counters[name])
for name in set(counters) | set(self.expected_counters)
}
return None


class CoverageProtocolPart(ProtocolPart):
"""Protocol part for collecting per-test coverage data."""
__metaclass__ = ABCMeta
Expand Down
11 changes: 6 additions & 5 deletions tools/wptrunner/wptrunner/wptcommandline.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ def create_parser(product_choices=None):
help="Path to stackwalker program used to analyse minidumps.")
debugging_group.add_argument("--pdb", action="store_true",
help="Drop into pdb on python exception")
debugging_group.add_argument("--leak-check", dest="leak_check", action="store_true", default=None,
help=("Enable leak checking for supported browsers "
"(Gecko: enabled by default for debug builds, "
"silently ignored for opt, mobile)"))
debugging_group.add_argument("--no-leak-check", dest="leak_check", action="store_false", default=None,
help="Disable leak checking")

android_group = parser.add_argument_group("Android specific arguments")
android_group.add_argument("--adb-binary", action="store",
Expand Down Expand Up @@ -334,11 +340,6 @@ def create_parser(product_choices=None):
gecko_group.add_argument("--setpref", dest="extra_prefs", action='append',
default=[], metavar="PREF=VALUE",
help="Defines an extra user preference (overrides those in prefs_root)")
gecko_group.add_argument("--leak-check", dest="leak_check", action="store_true", default=None,
help="Enable leak checking (enabled by default for debug builds, "
"silently ignored for opt, mobile)")
gecko_group.add_argument("--no-leak-check", dest="leak_check", action="store_false", default=None,
help="Disable leak checking")
gecko_group.add_argument("--reftest-internal", dest="reftest_internal", action="store_true",
default=None, help="Enable reftest runner implemented inside Marionette")
gecko_group.add_argument("--reftest-external", dest="reftest_internal", action="store_false",
Expand Down

0 comments on commit 980adbd

Please sign in to comment.