Skip to content

Commit

Permalink
Bug 1915990 [wpt PR 47908] - Revert "[wptrunner] Implement `--leak-ch…
Browse files Browse the repository at this point in the history
…eck` for Blink-based browsers", a=testonly

Automatic update from web-platform-tests
Revert "[wptrunner] Implement `--leak-check` for Blink-based browsers (#47850)" (#47908)

This reverts commit 980adbd1736ec466358831b34c0206cf668fe98c.
--

wpt-commits: a6745d222a0e97d8a3cd9a47ffc5f2faf1d72acb
wpt-pr: 47908
  • Loading branch information
jonathan-j-lee authored and moz-wptsync-bot committed Sep 1, 2024
1 parent a407a89 commit 50b15fa
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -209,10 +208,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:
Expand Down Expand Up @@ -258,11 +255,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+)\.')
NO_PORT_RE = re.compile(rb'.*was started successfully\.')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# mypy: allow-untyped-defs

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

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

here = os.path.dirname(__file__)

Expand Down Expand Up @@ -63,19 +62,6 @@ 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 @@ -220,54 +206,23 @@ 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 @@ -294,10 +249,8 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -766,8 +766,7 @@ def do_test(self, test):
self.extra_timeout).run()

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

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

Expand Down Expand Up @@ -875,10 +874,6 @@ 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 @@ -890,7 +885,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, extra
return rv

def _get_next_message_classic(self, protocol, url, _):
"""
Expand Down Expand Up @@ -984,9 +979,6 @@ 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 @@ -1006,6 +998,7 @@ 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 @@ -1059,7 +1052,6 @@ 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)
result = {"status": "PASS", "message": None}
if (leak_part := getattr(protocol, "leak", None)) and (counters := leak_part.check()):
result["extra"] = {"leak_counters": counters}
return result

return {"status": "PASS",
"message": None}
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
# 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, Tuple, Type
from typing import Any, Awaitable, Callable, ClassVar, List, Mapping, Optional, Type


def merge_dicts(target, source):
Expand Down Expand Up @@ -614,37 +613,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,6 @@ 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 @@ -340,6 +334,11 @@ 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 50b15fa

Please sign in to comment.