Merge r225740 - [GTK] WebDriver: run-webdriver-tests is leaking a Dum…

…pRenderTree directory in tmp

https://bugs.webkit.org/show_bug.cgi?id=180426

Reviewed by Michael Catanzaro.

This happens when running the tests with Xvfb driver, because _setup_environ_for_test() is called twice and the
DTR temp directory is created there every time. Only the last directory created is cleaned up by the driver
destructor. The DRT temp directory is only needed for layout tests, so we could even avoid its creation by
moving it to the start() method like other drivers do (included the base driver implementation). Since API and
WebDriver tests don't call start(), the directory is not even created, and the required env vars are not set
either in that case. Weston driver was behaving differently for some reason, it's now consistent with all other
drivers.

* Scripts/webkitpy/port/headlessdriver.py:
(HeadlessDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(HeadlessDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/waylanddriver.py:
(WaylandDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(WaylandDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/westondriver.py:
(WestonDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(WestonDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
(WestonDriver.stop): Do not delete the temp directory, it's done by the parent class.
(WestonDriver._ensure_driver_tmpdir_subdirectory): Deleted.
* Scripts/webkitpy/port/westondriver_unittest.py:
(WestonDriverTest.make_driver):
(WestonDriverTest.test_stop):
* Scripts/webkitpy/port/xorgdriver.py:
(XorgDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(XorgDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/xvfbdriver.py:
(XvfbDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(XvfbDriver._start): Use Port._driver_tempdir() to create the driver temp directory.

Tools/ChangeLog

@@ -1,3 +1,39 @@
+2017-12-11  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebDriver: run-webdriver-tests is leaking a DumpRenderTree directory in tmp
+        https://bugs.webkit.org/show_bug.cgi?id=180426
+
+        Reviewed by Michael Catanzaro.
+
+        This happens when running the tests with Xvfb driver, because _setup_environ_for_test() is called twice and the
+        DTR temp directory is created there every time. Only the last directory created is cleaned up by the driver
+        destructor. The DRT temp directory is only needed for layout tests, so we could even avoid its creation by
+        moving it to the start() method like other drivers do (included the base driver implementation). Since API and
+        WebDriver tests don't call start(), the directory is not even created, and the required env vars are not set
+        either in that case. Weston driver was behaving differently for some reason, it's now consistent with all other
+        drivers.
+
+        * Scripts/webkitpy/port/headlessdriver.py:
+        (HeadlessDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (HeadlessDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/waylanddriver.py:
+        (WaylandDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (WaylandDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/westondriver.py:
+        (WestonDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (WestonDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        (WestonDriver.stop): Do not delete the temp directory, it's done by the parent class.
+        (WestonDriver._ensure_driver_tmpdir_subdirectory): Deleted.
+        * Scripts/webkitpy/port/westondriver_unittest.py:
+        (WestonDriverTest.make_driver):
+        (WestonDriverTest.test_stop):
+        * Scripts/webkitpy/port/xorgdriver.py:
+        (XorgDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (XorgDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/xvfbdriver.py:
+        (XvfbDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (XvfbDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+
 2017-12-01  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: auto-install pytest instead of importing it from wpt tools directory

Tools/Scripts/webkitpy/port/waylanddriver.py

@@ -50,13 +50,14 @@ def _setup_environ_for_test(self):
         self._port._copy_value_from_environ_if_set(driver_environment, 'WAYLAND_SOCKET')
         driver_environment['GDK_BACKEND'] = 'wayland'
         driver_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
-        driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return driver_environment
 
     def _start(self, pixel_tests, per_test_args):
         super(WaylandDriver, self).stop()
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())

Tools/Scripts/webkitpy/port/westondriver.py

@@ -55,7 +55,6 @@ def __init__(self, *args, **kwargs):
         self._xvfbdriver = XvfbDriver(*args, **kwargs)
 
     def _setup_environ_for_test(self):
-        self._driver_directory = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
         driver_environment = self._port.setup_environ_for_server(self._server_name)
         driver_environment['DISPLAY'] = ":%d" % self._xvfbdriver._xvfb_run(driver_environment)
         weston_socket = 'WKTesting-weston-%032x' % random.getrandbits(128)
@@ -69,10 +68,11 @@ def _setup_environ_for_test(self):
         time.sleep(self._startup_delay_secs)
 
         driver_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        # Currently on WebKit2, there is no API for setting the application cache directory.
-        # Each worker should have its own and it should be cleaned afterwards, when the worker stops.
-        driver_environment['XDG_CACHE_HOME'] = self._ensure_driver_tmpdir_subdirectory('appcache')
-        driver_environment['DUMPRENDERTREE_TEMP'] = self._ensure_driver_tmpdir_subdirectory('drt-temp')
+        if self._driver_tempdir is not None:
+            # Currently on WebKit2, there is no API for setting the application cache directory.
+            # Each worker should have its own and it should be cleaned afterwards, when the worker stops.
+            driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+            driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
         driver_environment['WAYLAND_DISPLAY'] = weston_socket
         driver_environment['GDK_BACKEND'] = 'wayland'
         if driver_environment.get('DISPLAY'):
@@ -81,6 +81,7 @@ def _setup_environ_for_test(self):
 
     def _start(self, pixel_tests, per_test_args):
         self.stop()
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())
@@ -92,11 +93,4 @@ def stop(self):
             # The Weston process is terminated instead of killed, giving the Weston a chance to clean up after itself.
             self._weston_process.terminate()
             self._weston_process = None
-        if getattr(self, '_driver_directory', None):
-            self._port.host.filesystem.rmtree(str(self._driver_directory))
 
-    def _ensure_driver_tmpdir_subdirectory(self, subdirectory):
-        assert getattr(self, '_driver_directory', None)
-        directory_path = self._port.host.filesystem.join(str(self._driver_directory), subdirectory)
-        self._port.host.filesystem.maybe_make_directory(directory_path)
-        return directory_path

Tools/Scripts/webkitpy/port/westondriver_unittest.py

@@ -50,8 +50,8 @@ def _xvfb_run(self, environment):
 
 
 class WestonDriverTest(unittest.TestCase):
-    def make_driver(self, filesystem=None):
-        port = Port(MockSystemHost(log_executive=True, filesystem=filesystem), 'westondrivertestport', options=MockOptions(configuration='Release'))
+    def make_driver(self):
+        port = Port(MockSystemHost(log_executive=True), 'westondrivertestport', options=MockOptions(configuration='Release'))
         port._config.build_directory = lambda configuration: "/mock_build"
         port._server_process_constructor = MockServerProcess
 
@@ -85,13 +85,10 @@ class FakeWestonProcess(object):
             def terminate(self):
                 _log.info("MOCK FakeWestonProcess.terminate")
 
-        filesystem = MockFileSystem(dirs=['/tmp/weston-driver-directory'])
-        driver = self.make_driver(filesystem)
+        driver = self.make_driver()
         driver._weston_process = FakeWestonProcess()
-        driver._driver_directory = '/tmp/weston-driver-directory'
 
         expected_logs = "MOCK FakeWestonProcess.terminate\n"
         OutputCapture().assert_outputs(self, driver.stop, [], expected_logs=expected_logs)
 
         self.assertIsNone(driver._weston_process)
-        self.assertFalse(driver._port._filesystem.exists(driver._driver_directory))

Tools/Scripts/webkitpy/port/xorgdriver.py

@@ -50,18 +50,19 @@ def _setup_environ_for_test(self):
         self._port._copy_value_from_environ_if_set(server_environment, 'XAUTHORITY')
         server_environment['GDK_BACKEND'] = 'x11'
         server_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        server_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
-        # Currently on WebKit2, there is no API for setting the application
-        # cache directory. Each worker should have it's own and it should be
-        # cleaned afterwards, so we set it to inside the temporary folder by
-        # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
-        server_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            server_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            # Currently on WebKit2, there is no API for setting the application
+            # cache directory. Each worker should have it's own and it should be
+            # cleaned afterwards, so we set it to inside the temporary folder by
+            # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
+            server_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return server_environment
 
     def _start(self, pixel_tests, per_test_args):
         super(XorgDriver, self).stop()
 
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
 
         self._crashed_process_name = None
         self._crashed_pid = None

Tools/Scripts/webkitpy/port/xvfbdriver.py

@@ -99,19 +99,19 @@ def _setup_environ_for_test(self):
         # We must do this here because the DISPLAY number depends on _worker_number
         environment['DISPLAY'] = ":%d" % display_id
         environment['GDK_BACKEND'] = 'x11'
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
-        environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
         environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-
-        # Currently on WebKit2, there is no API for setting the application
-        # cache directory. Each worker should have it's own and it should be
-        # cleaned afterwards, so we set it to inside the temporary folder by
-        # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
-        environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            # Currently on WebKit2, there is no API for setting the application
+            # cache directory. Each worker should have it's own and it should be
+            # cleaned afterwards, so we set it to inside the temporary folder by
+            # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
+            environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return environment
 
     def _start(self, pixel_tests, per_test_args):
         self.stop()
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())