Bug 1665366 [wpt PR 25567] - Reland "[webdriver] Close old windows at the end of each test as well as beginning", a=testonly

Automatic update from web-platform-tests
Reland "[webdriver] Close old windows at the end of each test as well as beginning" (#25567)

This is a reland of #24879 (which was reverted in #25544). It updates the code
to only close the windows if the user hasn't asked us to pause after a test has run.

From the original PR:

----

Previously, we closed old windows at the start of each test. This was nice in terms
of defensiveness (don't assume the last run left the world in a good state), but made
it hard to find problematic tests that left dialogs open (since they wouldn't throw until
the next test).

Instead, this patch does both - close both at the start and end of a test. This should
improve the blaming situation, whilst still being defensive.

There are potential performance implications to this patch, however test runs are
inconclusive. Full runs of Chrome and Safari show +- 2%, which is possibly within
margin of error. Running locally, some directories showed a ~2% slowdown, whilst
others had little or no difference.
--

wpt-commits: ed3c2cd80ae2766813412695306c31716f3c82a7
wpt-pr: 25567

testing/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py

@@ -37,6 +37,12 @@ def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
         executor_kwargs["webdriver_binary"] = kwargs.get("webdriver_binary")
         executor_kwargs["webdriver_args"] = kwargs.get("webdriver_args")
 
+    # By default the executor may try to cleanup windows after a test (to best
+    # associate any problems with the test causing them). If the user might
+    # want to view the results, however, the executor has to skip that cleanup.
+    if kwargs["pause_after_test"] or kwargs["pause_on_unexpected"]:
+        executor_kwargs["cleanup_after_test"] = False
+
     return executor_kwargs
 
 

testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executorwebdriver.py

@@ -369,7 +369,8 @@ class WebDriverTestharnessExecutor(TestharnessExecutor):
 
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
                  close_after_done=True, capabilities=None, debug_info=None,
-                 supports_eager_pageload=True, **kwargs):
+                 supports_eager_pageload=True, cleanup_after_test=True,
+                 **kwargs):
         """WebDriver-based executor for testharness.js tests"""
         TestharnessExecutor.__init__(self, logger, browser, server_config,
                                      timeout_multiplier=timeout_multiplier,
@@ -383,6 +384,7 @@ class WebDriverTestharnessExecutor(TestharnessExecutor):
         self.close_after_done = close_after_done
         self.window_id = str(uuid.uuid4())
         self.supports_eager_pageload = supports_eager_pageload
+        self.cleanup_after_test = cleanup_after_test
 
     def is_alive(self):
         return self.protocol.is_alive()
@@ -409,7 +411,10 @@ class WebDriverTestharnessExecutor(TestharnessExecutor):
     def do_testharness(self, protocol, url, timeout):
         format_map = {"url": strip_server(url)}
 
+        # The previous test may not have closed its old windows (if something
+        # went wrong or if cleanup_after_test was False), so clean up here.
         parent_window = protocol.testharness.close_old_windows()
+
         # Now start the test harness
         protocol.base.execute_script("window.open('about:blank', '%s', 'noopener')" % self.window_id)
         test_window = protocol.testharness.get_test_window(self.window_id,
@@ -446,6 +451,14 @@ class WebDriverTestharnessExecutor(TestharnessExecutor):
             done, rv = handler(result)
             if done:
                 break
+
+        # Attempt to cleanup 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
+        # have to leave the window(s) open.
+        if self.cleanup_after_test:
+            protocol.testharness.close_old_windows()
+
         return rv
 
     def wait_for_load(self, protocol):

testing/web-platform/tests/tools/wptrunner/wptrunner/tests/browsers/test_webkitgtk.py

@@ -39,6 +39,8 @@ def test_webkitgtk_certificate_domain_list(product):
     kwargs["webkit_port"] = "gtk"
     kwargs["binary"] = None
     kwargs["webdriver_binary"] = None
+    kwargs["pause_after_test"] = False
+    kwargs["pause_on_unexpected"] = False
     with ConfigBuilder(browser_host="example.net",
                        alternate_hosts={"alt": "example.org"},
                        subdomains={"a", "b"},