Bug 1333114 - Enable leak checking for web-platform-tests, r=Ms2ger draft
authorJames Graham <james@hoppipolla.co.uk>
Mon, 30 Jan 2017 08:18:36 -0800
changeset 482793 6ea4cacabb5218dc2d2a1f08eef85d646d4390e8
parent 482792 e6594ae4c90319ba69251ceb83fdd63cf092d7f6
child 482794 d92abed83b40fa7042bb97cc40c72cffe50eb055
push id45167
push userbmo:james@hoppipolla.co.uk
push dateMon, 13 Feb 2017 13:31:27 +0000
reviewersMs2ger
bugs1333114
milestone54.0a1
Bug 1333114 - Enable leak checking for web-platform-tests, r=Ms2ger Enable gecko leak checking for web-platform-tests. Allows disabling leak checks for a specific test using a 'leaks' key in test metadata. 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/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]
 
+Disabiling 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):
+        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
 
@@ -122,17 +125,17 @@ class Browser(object):
 
     def log_crash(self, process, test):
         """Return a list of dictionaries containing information about crashes that happend
         in the browser, or an empty list if no crashes occurred"""
         self.logger.crash(process, test)
 
 
 class NullBrowser(Browser):
-    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
@@ -54,17 +54,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,
@@ -98,17 +99,18 @@ def update_properties():
 
 class FirefoxBrowser(Browser):
     used_ports = set()
     init_timeout = 60
     shutdown_timeout = 60
 
     def __init__(self, logger, binary, prefs_root, debug_info=None,
                  symbols_path=None, stackwalk_binary=None, certutil_binary=None,
-                 ca_certificate_path=None, e10s=False, stackfix_dir=None):
+                 ca_certificate_path=None, e10s=False, stackfix_dir=None,
+                 leak_report=True):
         Browser.__init__(self, logger)
         self.binary = binary
         self.prefs_root = prefs_root
         self.marionette_port = None
         self.runner = None
         self.debug_info = debug_info
         self.profile = None
         self.symbols_path = symbols_path
@@ -116,20 +118,26 @@ class FirefoxBrowser(Browser):
         self.ca_certificate_path = ca_certificate_path
         self.certutil_binary = certutil_binary
         self.e10s = e10s
         if self.symbols_path and stackfix_dir:
             self.stack_fixer = get_stack_fixer_function(stackfix_dir,
                                                         self.symbols_path)
         else:
             self.stack_fixer = None
+        self.leak_report_file = None
+        self.leak_report = leak_report
 
-    def start(self):
-        self.marionette_port = get_free_port(2828, exclude=self.used_ports)
-        self.used_ports.add(self.marionette_port)
+    def settings(self, test):
+        return {"check_leaks": self.leak_report 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()
 
@@ -139,16 +147,24 @@ class FirefoxBrowser(Browser):
                                       "marionette.defaultPrefs.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_report 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()
 
@@ -188,16 +204,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:
@@ -214,16 +248,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
@@ -153,21 +153,29 @@ 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)
+        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()
@@ -178,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:
@@ -437,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:
--- 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: