[chromedriver] Kill browser processes when timeout in py tests

ChromeDriver python tests sometimes encounter timeout while launching
browsers. Currently, ChromeDriver would try to re-launch browser again
with max retry of 3 for all tests. However, the original browser might
still be launching after the retry/retries. This change is to
terminate/kill the original browser before the retry.

There are two benefits:
1. If the original browser is terminated/killed before retry, then
we will not see the log from it later in ChromeDriver log.
This can avoid confusion.
2. It reduces resource consumption with less concurrent browsers running.
This is in hope to reduce test flakiness.

Change-Id: Ie31c155398e1c4ca19d3697e6f25f853323191ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586972
Commit-Queue: Shengfa Lin <shengfa@google.com>
Reviewed-by: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836800}

chrome/test/chromedriver/client/chromedriver.py

@@ -5,6 +5,7 @@
 import platform
 import sys
 import util
+import psutil
 
 import command_executor
 from command_executor import Command
@@ -127,21 +128,38 @@ class ChromeDriver(object):
   retry_count = 0
   retried_tests = []
 
-  def __init__(self, *args, **kwargs):
+  def __init__(self, server_url, server_pid, **kwargs):
     try:
-      self._InternalInit(*args, **kwargs)
+      self._InternalInit(server_url, **kwargs)
     except Exception as e:
       if not e.message.startswith('timed out'):
         raise
       else:
+        # Kill ChromeDriver child processes recursively
+        # (i.e. browsers and their child processes etc)
+        # when there is a timeout for launching browser
+        if server_pid:
+          processes = psutil.Process(server_pid).children(recursive=True)
+          if len(processes):
+            print 'Terminating ', len(processes), ' processes'
+            for p in processes:
+              p.terminate()
+
+            gone, alive = psutil.wait_procs(processes, timeout=3)
+            if len(alive):
+              print 'Killing ', len(alive), ' processes'
+              for p in alive:
+                p.kill()
+
         if ChromeDriver.retry_count < MAX_RETRY_COUNT:
           ChromeDriver.retry_count = ChromeDriver.retry_count + 1
           ChromeDriver.retried_tests.append(kwargs.get('test_name'))
-          self._InternalInit(*args, **kwargs)
+          self._InternalInit(server_url, **kwargs)
         else:
           raise
 
-  def _InternalInit(self, server_url, chrome_binary=None, android_package=None,
+  def _InternalInit(self, server_url,
+      chrome_binary=None, android_package=None,
       android_activity=None, android_process=None,
       android_use_running_app=None, chrome_switches=None,
       chrome_extensions=None, chrome_log_path=None,

chrome/test/chromedriver/server/server.py

@@ -67,6 +67,7 @@ class Server(object):
         chromedriver_args.extend([arg])
 
     self._process = subprocess.Popen(chromedriver_args)
+    self._pid = self._process.pid
     self._host = '127.0.0.1'
     self._port = port
     self._url = 'http://%s:%d' % (self._host, port)
@@ -99,6 +100,9 @@ class Server(object):
   def GetUrl(self):
     return self._url
 
+  def GetPid(self):
+    return self._pid
+
   def GetHost(self):
     return self._host
 

chrome/test/chromedriver/test/run_py_tests.py

@@ -305,9 +305,12 @@ class ChromeDriverBaseTest(unittest.TestCase):
       except:
         pass
 
-  def CreateDriver(self, server_url=None, download_dir=None, **kwargs):
+  def CreateDriver(self, server_url=None, server_pid=None,
+                   download_dir=None, **kwargs):
     if server_url is None:
       server_url = _CHROMEDRIVER_SERVER_URL
+    if server_pid is None:
+      server_pid = _CHROMEDRIVER_SERVER_PID
 
     if (not _ANDROID_PACKAGE_KEY and 'debugger_address' not in kwargs and
           '_MINIDUMP_PATH' in globals() and _MINIDUMP_PATH):
@@ -328,7 +331,7 @@ class ChromeDriverBaseTest(unittest.TestCase):
         android_activity = constants.PACKAGE_INFO[_ANDROID_PACKAGE_KEY].activity
         android_process = '%s:main' % android_package
 
-    driver = chromedriver.ChromeDriver(server_url,
+    driver = chromedriver.ChromeDriver(server_url, server_pid,
                                        chrome_binary=_CHROME_BINARY,
                                        android_package=android_package,
                                        android_activity=android_activity,
@@ -4509,7 +4512,8 @@ class ChromeDriverLogTest(ChromeDriverBaseTest):
         _CHROMEDRIVER_BINARY, log_path=tmp_log_path)
     try:
       driver = chromedriver.ChromeDriver(
-          chromedriver_server.GetUrl(), chrome_binary=_CHROME_BINARY,
+          chromedriver_server.GetUrl(), chromedriver_server.GetPid(),
+          chrome_binary=_CHROME_BINARY,
           experimental_options={ self.UNEXPECTED_CHROMEOPTION_CAP : 1 })
       driver.Quit()
     except chromedriver.ChromeDriverException, e:
@@ -4724,6 +4728,7 @@ class LaunchDesktopTest(ChromeDriverBaseTest):
       exception_raised = False
       try:
         driver = chromedriver.ChromeDriver(_CHROMEDRIVER_SERVER_URL,
+                                           _CHROMEDRIVER_SERVER_PID,
                                            chrome_binary=path,
                                            test_name=self.id())
       except Exception as e:
@@ -4747,6 +4752,7 @@ class LaunchDesktopTest(ChromeDriverBaseTest):
     try:
       driver = chromedriver.ChromeDriver(
           _CHROMEDRIVER_SERVER_URL,
+          _CHROMEDRIVER_SERVER_PID,
           chrome_binary=os.path.join(temp_dir, 'this_file_should_not_exist'),
           test_name=self.id())
     except Exception as e:
@@ -5035,6 +5041,10 @@ if __name__ == '__main__':
   global chromedriver_server
   chromedriver_server = server.Server(_CHROMEDRIVER_BINARY, options.log_path,
                                       replayable=options.replayable)
+
+  global _CHROMEDRIVER_SERVER_PID
+  _CHROMEDRIVER_SERVER_PID = chromedriver_server.GetPid()
+
   global _CHROMEDRIVER_SERVER_URL
   _CHROMEDRIVER_SERVER_URL = chromedriver_server.GetUrl()