Bug 1352357 - Add leak checking support for web-platform-tests, r=AutomatedTester draft
authorJames Graham <james@hoppipolla.co.uk>
Mon, 30 Jan 2017 08:18:36 -0800
changeset 557349 512e67f1fbadb9093317a6aadad245db7dac6b4a
parent 557282 facaf90aeaaf6d7cf5e2966f9f918319536bddea
child 623032 0e463fbe169dbe223757b0d7ed84b4ce56f5fea7
push id52696
push userbmo:james@hoppipolla.co.uk
push dateThu, 06 Apr 2017 17:56:36 +0000
reviewersAutomatedTester
bugs1352357
milestone55.0a1
Bug 1352357 - Add leak checking support for web-platform-tests, r=AutomatedTester Implement gecko leak checking for web-platform-tests, but do not enable it yet due to oranges. Allows disabling leak checks for a specific test using a 'leaks' key in test metadata. This causes the browser to restart before running the (group of) tests with leak checking disabled. The feature can be enabled by passing --leak-check on the command line. MozReview-Commit-ID: 1kBnJkOaeu8
testing/web-platform/README.md
testing/web-platform/harness/requirements_firefox.txt
testing/web-platform/harness/wptrunner/browsers/base.py
testing/web-platform/harness/wptrunner/browsers/chrome.py
testing/web-platform/harness/wptrunner/browsers/edge.py
testing/web-platform/harness/wptrunner/browsers/firefox.py
testing/web-platform/harness/wptrunner/browsers/servodriver.py
testing/web-platform/harness/wptrunner/manifestexpected.py
testing/web-platform/harness/wptrunner/testrunner.py
testing/web-platform/harness/wptrunner/wptcommandline.py
testing/web-platform/harness/wptrunner/wpttest.py
--- a/testing/web-platform/README.md
+++ b/testing/web-platform/README.md
@@ -165,16 +165,29 @@ Some tests require specific prefs to be 
 prefs can be set in the expectation data using a `prefs` key with a
 comma-seperate list of `pref.name:value` items to set e.g.
 
     [filename.html]
         prefs: [dom.serviceWorkers.enabled:true,
                 dom.serviceWorkers.exemptFromPerDomainMax:true,
                 dom.caches.enabled:true]
 
+Disabling Leak Checks
+----------------------
+
+When a test is imported that leaks, it may be necessary to temporarily
+disable leak checking for that test in order to allow the import to
+proceed. This works in basically the same way as disabling a test, but
+with the key 'leaks' e.g.
+
+    [filename.html]
+        type: testharness
+        leaks:
+            if os == "linux": https://bugzilla.mozilla.org/show_bug.cgi?id=1234567
+
 Setting per-Directory Metadata
 ------------------------------
 
 Occasionally it is useful to set metadata for an entire directory of
 tests e.g. to disable then all, or to enable prefs for every test. In
 that case it is possible to create a `__dir__.ini` file in the
 metadata directory corresponding to the tests for which you want to
 set this metadata e.g. to disable all the tests in
--- a/testing/web-platform/harness/requirements_firefox.txt
+++ b/testing/web-platform/harness/requirements_firefox.txt
@@ -1,5 +1,6 @@
 marionette_driver >= 0.4
 mozprofile >= 0.21
 mozprocess >= 0.19
 mozcrash >= 0.13
 mozrunner >= 6.7
+mozleak >= 0.1
--- a/testing/web-platform/harness/wptrunner/browsers/base.py
+++ b/testing/web-platform/harness/wptrunner/browsers/base.py
@@ -82,18 +82,21 @@ class Browser(object):
 
     def __exit__(self, *args, **kwargs):
         self.cleanup()
 
     def setup(self):
         """Used for browser-specific setup that happens at the start of a test run"""
         pass
 
+    def settings(self, test):
+        return {}
+
     @abstractmethod
-    def start(self):
+    def start(self, **kwargs):
         """Launch the browser object and get it into a state where is is ready to run tests"""
         pass
 
     @abstractmethod
     def stop(self, force=False):
         """Stop the running browser process."""
         pass
 
@@ -125,17 +128,17 @@ class Browser(object):
         in the browser, or an empty list if no crashes occurred"""
         self.logger.crash(process, test)
 
 
 class NullBrowser(Browser):
     def __init__(self, logger, **kwargs):
         super(NullBrowser, self).__init__(logger)
 
-    def start(self):
+    def start(self, **kwargs):
         """No-op browser to use in scenarios where the TestRunnerManager shouldn't
         actually own the browser process (e.g. Servo where we start one browser
         per test)"""
         pass
 
     def stop(self, force=False):
         pass
 
--- a/testing/web-platform/harness/wptrunner/browsers/chrome.py
+++ b/testing/web-platform/harness/wptrunner/browsers/chrome.py
@@ -62,17 +62,17 @@ class ChromeBrowser(Browser):
 
     def __init__(self, logger, binary, webdriver_binary="chromedriver"):
         """Creates a new representation of Chrome.  The `binary` argument gives
         the browser binary to use for testing."""
         Browser.__init__(self, logger)
         self.binary = binary
         self.server = ChromeDriverServer(self.logger, binary=webdriver_binary)
 
-    def start(self):
+    def start(self, **kwargs):
         self.server.start(block=False)
 
     def stop(self, force=False):
         self.server.stop(force=Force)
 
     def pid(self):
         return self.server.pid
 
--- a/testing/web-platform/harness/wptrunner/browsers/edge.py
+++ b/testing/web-platform/harness/wptrunner/browsers/edge.py
@@ -43,17 +43,17 @@ class EdgeBrowser(Browser):
     used_ports = set()
 
     def __init__(self, logger, webdriver_binary):
         Browser.__init__(self, logger)
         self.server = EdgeDriverServer(self.logger, binary=webdriver_binary)
         self.webdriver_host = "localhost"
         self.webdriver_port = self.server.port
 
-    def start(self):
+    def start(self, **kwargs):
         print self.server.url
         self.server.start()
 
     def stop(self):
         self.server.stop()
 
     def pid(self):
         return self.server.pid
--- a/testing/web-platform/harness/wptrunner/browsers/firefox.py
+++ b/testing/web-platform/harness/wptrunner/browsers/firefox.py
@@ -4,16 +4,17 @@
 
 import os
 import platform
 import signal
 import subprocess
 import sys
 
 import mozinfo
+import mozleak
 from mozprocess import ProcessHandler
 from mozprofile import FirefoxProfile, Preferences
 from mozprofile.permissions import ServerLocations
 from mozrunner import FirefoxRunner
 from mozrunner.utils import get_stack_fixer_function
 from mozcrash import mozcrash
 
 from .base import (get_free_port,
@@ -72,17 +73,18 @@ def browser_kwargs(test_type, run_info_d
             "stackwalk_binary": kwargs["stackwalk_binary"],
             "certutil_binary": kwargs["certutil_binary"],
             "ca_certificate_path": kwargs["ssl_env"].ca_cert_path(),
             "e10s": kwargs["gecko_e10s"],
             "stackfix_dir": kwargs["stackfix_dir"],
             "binary_args": kwargs["binary_args"],
             "timeout_multiplier": get_timeout_multiplier(test_type,
                                                          run_info_data,
-                                                         **kwargs)}
+                                                         **kwargs),
+            "leak_check": kwargs["leak_check"]}
 
 
 def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
                     **kwargs):
     executor_kwargs = base_executor_kwargs(test_type, server_config,
                                            cache_manager, **kwargs)
     executor_kwargs["close_after_done"] = test_type != "reftest"
     executor_kwargs["timeout_multiplier"] = get_timeout_multiplier(test_type,
@@ -122,17 +124,17 @@ def update_properties():
 class FirefoxBrowser(Browser):
     used_ports = set()
     init_timeout = 60
     shutdown_timeout = 60
 
     def __init__(self, logger, binary, prefs_root, extra_prefs=None, debug_info=None,
                  symbols_path=None, stackwalk_binary=None, certutil_binary=None,
                  ca_certificate_path=None, e10s=False, stackfix_dir=None,
-                 binary_args=None, timeout_multiplier=None):
+                 binary_args=None, timeout_multiplier=None, leak_check=False):
         Browser.__init__(self, logger)
         self.binary = binary
         self.prefs_root = prefs_root
         self.extra_prefs = extra_prefs
         self.marionette_port = None
         self.runner = None
         self.debug_info = debug_info
         self.profile = None
@@ -142,22 +144,30 @@ class FirefoxBrowser(Browser):
         self.certutil_binary = certutil_binary
         self.e10s = e10s
         self.binary_args = binary_args
         if self.symbols_path and stackfix_dir:
             self.stack_fixer = get_stack_fixer_function(stackfix_dir,
                                                         self.symbols_path)
         else:
             self.stack_fixer = None
+
         if timeout_multiplier:
             self.init_timeout = self.init_timeout * timeout_multiplier
 
-    def start(self):
-        self.marionette_port = get_free_port(2828, exclude=self.used_ports)
-        self.used_ports.add(self.marionette_port)
+        self.leak_report_file = None
+        self.leak_check = leak_check
+
+    def settings(self, test):
+        return {"check_leaks": self.leak_check and not test.leaks}
+
+    def start(self, **kwargs):
+        if self.marionette_port is None:
+            self.marionette_port = get_free_port(2828, exclude=self.used_ports)
+            self.used_ports.add(self.marionette_port)
 
         env = os.environ.copy()
         env["MOZ_DISABLE_NONLOCAL_CONNECTIONS"] = "1"
 
         locations = ServerLocations(filename=os.path.join(here, "server-locations.txt"))
 
         preferences = self.load_prefs()
 
@@ -167,16 +177,24 @@ class FirefoxBrowser(Browser):
                                       "marionette.port": self.marionette_port,
                                       "dom.disable_open_during_load": False,
                                       "network.dns.localDomains": ",".join(hostnames),
                                       "network.proxy.type": 0,
                                       "places.history.enabled": False})
         if self.e10s:
             self.profile.set_preferences({"browser.tabs.remote.autostart": True})
 
+        if self.leak_check and kwargs.get("check_leaks", True):
+            self.leak_report_file = os.path.join(self.profile.profile, "runtests_leaks.log")
+            if os.path.exists(self.leak_report_file):
+                os.remove(self.leak_report_file)
+            env["XPCOM_MEM_BLOAT_LOG"] = self.leak_report_file
+        else:
+            self.leak_report_file = None
+
         # Bug 1262954: winxp + e10s, disable hwaccel
         if (self.e10s and platform.system() in ("Windows", "Microsoft") and
             '5.1' in platform.version()):
             self.profile.set_preferences({"layers.acceleration.disabled": True})
 
         if self.ca_certificate_path is not None:
             self.setup_ssl()
 
@@ -222,16 +240,34 @@ class FirefoxBrowser(Browser):
                     if not force or not clean:
                         retcode = stop_f()
                         if retcode is not None:
                             self.logger.info("Browser exited with return code %s" % retcode)
                             break
             except OSError:
                 # This can happen on Windows if the process is already dead
                 pass
+        self.logger.debug("stopped")
+
+    def process_leaks(self):
+        self.logger.debug("PROCESS LEAKS %s" % self.leak_report_file)
+        if self.leak_report_file is None:
+            return
+        mozleak.process_leak_log(
+            self.leak_report_file,
+            leak_thresholds={
+                "default": 0,
+                "tab": 10000,  # See dependencies of bug 1051230.
+                # GMP rarely gets a log, but when it does, it leaks a little.
+                "geckomediaplugin": 20000,
+            },
+            ignore_missing_leaks=["geckomediaplugin"],
+            log=self.logger,
+            stack_fixer=self.stack_fixer
+        )
 
     def pid(self):
         if self.runner.process_handler is None:
             return None
 
         try:
             return self.runner.process_handler.pid
         except AttributeError:
@@ -248,16 +284,17 @@ class FirefoxBrowser(Browser):
 
     def is_alive(self):
         if self.runner:
             return self.runner.is_running()
         return False
 
     def cleanup(self):
         self.stop()
+        self.process_leaks()
 
     def executor_browser(self):
         assert self.marionette_port is not None
         return ExecutorBrowser, {"marionette_port": self.marionette_port}
 
     def log_crash(self, process, test):
         dump_dir = os.path.join(self.profile.profile, "minidumps")
 
--- a/testing/web-platform/harness/wptrunner/browsers/servodriver.py
+++ b/testing/web-platform/harness/wptrunner/browsers/servodriver.py
@@ -85,17 +85,17 @@ class ServoWebDriverBrowser(Browser):
         self.webdriver_host = webdriver_host
         self.webdriver_port = None
         self.proc = None
         self.debug_info = debug_info
         self.hosts_path = make_hosts_file()
         self.command = None
         self.user_stylesheets = user_stylesheets if user_stylesheets else []
 
-    def start(self):
+    def start(self, **kwargs):
         self.webdriver_port = get_free_port(4444, exclude=self.used_ports)
         self.used_ports.add(self.webdriver_port)
 
         env = os.environ.copy()
         env["HOST_FILE"] = self.hosts_path
         env["RUST_BACKTRACE"] = "1"
 
         debug_args, command = browser_command(
--- a/testing/web-platform/harness/wptrunner/manifestexpected.py
+++ b/testing/web-platform/harness/wptrunner/manifestexpected.py
@@ -111,16 +111,20 @@ class ExpectedManifest(ManifestItem):
     def disabled(self):
         return bool_prop("disabled", self)
 
     @property
     def restart_after(self):
         return bool_prop("restart-after", self)
 
     @property
+    def leaks(self):
+        return bool_prop("leaks", self)
+
+    @property
     def tags(self):
         return tags(self)
 
     @property
     def prefs(self):
         return prefs(self)
 
 
@@ -129,16 +133,20 @@ class DirectoryManifest(ManifestItem):
     def disabled(self):
         return bool_prop("disabled", self)
 
     @property
     def restart_after(self):
         return bool_prop("restart-after", self)
 
     @property
+    def leaks(self):
+        return bool_prop("leaks", self)
+
+    @property
     def tags(self):
         return tags(self)
 
     @property
     def prefs(self):
         return prefs(self)
 
 
@@ -174,16 +182,20 @@ class TestNode(ManifestItem):
     def disabled(self):
         return bool_prop("disabled", self)
 
     @property
     def restart_after(self):
         return bool_prop("restart-after", self)
 
     @property
+    def leaks(self):
+        return bool_prop("leaks", self)
+
+    @property
     def tags(self):
         return tags(self)
 
     @property
     def prefs(self):
         return prefs(self)
 
     def append(self, node):
--- a/testing/web-platform/harness/wptrunner/testrunner.py
+++ b/testing/web-platform/harness/wptrunner/testrunner.py
@@ -152,21 +152,30 @@ def next_manager_number():
 
 class BrowserManager(object):
     init_lock = threading.Lock()
 
     def __init__(self, logger, browser, command_queue, no_timeout=False):
         self.logger = logger
         self.browser = browser
         self.no_timeout = no_timeout
+        self.browser_settings = None
 
         self.started = False
 
         self.init_timer = None
 
+    def update_settings(self, test):
+        browser_settings = self.browser.settings(test)
+        restart_required = ((self.browser_settings is not None and
+                             self.browser_settings != browser_settings) or
+                            test.expected() == "CRASH")
+        self.browser_settings = browser_settings
+        return restart_required
+
     def init(self):
         """Launch the browser that is being tested,
         and the TestRunner process that will run the tests."""
         # It seems that this lock is helpful to prevent some race that otherwise
         # sometimes stops the spawned processes initalising correctly, and
         # leaves this thread hung
         if self.init_timer is not None:
             self.init_timer.cancel()
@@ -177,17 +186,18 @@ class BrowserManager(object):
             # Guard against problems initialising the browser or the browser
             # remote control method
             if not self.no_timeout:
                 self.init_timer = threading.Timer(self.browser.init_timeout,
                                                   self.init_timeout)
             try:
                 if self.init_timer is not None:
                     self.init_timer.start()
-                self.browser.start()
+                self.logger.debug("Starting browser with settings %r" % self.browser_settings)
+                self.browser.start(**self.browser_settings)
                 self.browser_pid = self.browser.pid()
             except:
                 self.logger.warning("Failure during init %s" % traceback.format_exc())
                 if self.init_timer is not None:
                     self.init_timer.cancel()
                 self.logger.error(traceback.format_exc())
                 succeeded = False
             else:
@@ -436,16 +446,18 @@ class TestRunnerManager(threading.Thread
             return RunnerManagerState.initalizing(test, test_queue, 0)
 
     def init(self):
         assert isinstance(self.state, RunnerManagerState.initalizing)
         if self.state.failure_count > self.max_restarts:
             self.logger.error("Max restarts exceeded")
             return RunnerManagerState.error()
 
+        self.browser.update_settings(self.state.test)
+
         result = self.browser.init()
         if result is Stop:
             return RunnerManagerState.error()
         elif not result:
             return RunnerManagerState.initalizing(self.state.test,
                                                   self.state.test_queue,
                                                   self.state.failure_count + 1)
         else:
@@ -503,16 +515,21 @@ class TestRunnerManager(threading.Thread
             except Empty:
                 pass
         return test, test_queue
 
     def run_test(self):
         assert isinstance(self.state, RunnerManagerState.running)
         assert self.state.test is not None
 
+        if self.browser.update_settings(self.state.test):
+            self.logger.info("Restarting browser for new test environment")
+            return RunnerManagerState.restarting(self.state.test,
+                                                 self.state.test_queue)
+
         self.logger.test_start(self.state.test.id)
         self.send_message("run_test", self.state.test)
 
     def test_ended(self, test, results):
         """Handle the end of a test.
 
         Output the result of each subtest, and the result of the overall
         harness to the logs.
--- a/testing/web-platform/harness/wptrunner/wptcommandline.py
+++ b/testing/web-platform/harness/wptrunner/wptcommandline.py
@@ -173,16 +173,18 @@ scheme host and port.""")
                              help="Path to the folder containing browser prefs")
     gecko_group.add_argument("--disable-e10s", dest="gecko_e10s", action="store_false", default=True,
                              help="Run tests without electrolysis preferences")
     gecko_group.add_argument("--stackfix-dir", dest="stackfix_dir", action="store",
                              help="Path to directory containing assertion stack fixing scripts")
     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",
+                             help="Enable leak checking")
 
     servo_group = parser.add_argument_group("Servo-specific")
     servo_group.add_argument("--user-stylesheet",
                              default=[], action="append", dest="user_stylesheets",
                              help="Inject a user CSS stylesheet into every test.")
 
 
     parser.add_argument("test_list", nargs="*",
--- a/testing/web-platform/harness/wptrunner/wpttest.py
+++ b/testing/web-platform/harness/wptrunner/wpttest.py
@@ -168,16 +168,24 @@ class Test(object):
     def restart_after(self):
         for meta in self.itermeta(None):
             restart_after = meta.restart_after
             if restart_after is not None:
                 return True
         return False
 
     @property
+    def leaks(self):
+        for meta in self.itermeta(None):
+            leaks = meta.leaks
+            if leaks is not None:
+                return leaks
+        return False
+
+    @property
     def tags(self):
         tags = set()
         for meta in self.itermeta():
             meta_tags = meta.tags
             if atom_reset in meta_tags:
                 tags = meta_tags.copy()
                 tags.remove(atom_reset)
             else: