Currently, the most time consuming operation when creating a new browser

during Telemetry tests consists of sending an "adb force-stop" command
to the device, in order to force kill any previous browsers which did
not close as expected.

Sending a kill -9 to the process is, however, more efficient.
This CL also adds some efficiency improvements to DeviceUtils.KillAll.
The change amounts to a 6-7% speedup when running all of telemetry
unittests.

BUG=379378

Review URL: https://codereview.chromium.org/536343003

Cr-Commit-Position: refs/heads/master@{#294368}

build/android/pylib/device/device_utils.py

@@ -364,20 +364,20 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    pids = self.old_interface.ExtractPid(process_name)
-    if len(pids) == 0:
+    pids = self._GetPidsImpl(process_name)
+    if not pids:
       raise device_errors.CommandFailedError(
           'No process "%s"' % process_name, device=str(self))
 
+    cmd = 'kill -%d %s' % (signum, ' '.join(pids.values()))
+    self._RunShellCommandImpl(cmd, as_root=as_root)
+
     if blocking:
-      total_killed = self.old_interface.KillAllBlocking(
-          process_name, signum=signum, with_su=as_root, timeout_sec=timeout)
-    else:
-      total_killed = self.old_interface.KillAll(
-          process_name, signum=signum, with_su=as_root)
-    if total_killed == 0:
-      raise device_errors.CommandFailedError(
-          'Failed to kill "%s"' % process_name, device=str(self))
+      wait_period = 0.1
+      while self._GetPidsImpl(process_name):
+        time.sleep(wait_period)
+
+    return len(pids)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def StartActivity(self, intent, blocking=False, trace_file_name=None,
@@ -722,6 +722,24 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
+    return self._GetPidsImpl(process_name)
+
+  def _GetPidsImpl(self, process_name):
+    """Implementation of GetPids.
+
+    This is split from GetPids to allow other DeviceUtils methods to call
+    GetPids without spawning a new timeout thread.
+
+    Args:
+      process_name: A string containing the process name to get the PIDs for.
+
+    Returns:
+      A dict mapping process name to PID for each process that contained the
+      provided |process_name|.
+
+    Raises:
+      DeviceUnreachableError on missing device.
+    """
     procs_pids = {}
     for line in self._RunShellCommandImpl('ps'):
       try:

build/android/pylib/device/device_utils_test.py

@@ -553,24 +553,17 @@ class DeviceUtilsKillAllTest(DeviceUtilsOldImplTest):
 
   def testKillAll_nonblocking(self):
     with self.assertCallsSequence([
-        ("adb -s 0123456789abcdef shell 'ps'",
-         'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
-         'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
-              'this.is.a.test.process\r\n'),
         ("adb -s 0123456789abcdef shell 'ps'",
          'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
          'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
               'this.is.a.test.process\r\n'),
         ("adb -s 0123456789abcdef shell 'kill -9 1234'", '')]):
-      self.device.KillAll('this.is.a.test.process', blocking=False)
+      self.assertEquals(1,
+          self.device.KillAll('this.is.a.test.process', blocking=False))
 
   def testKillAll_blocking(self):
     with mock.patch('time.sleep'):
       with self.assertCallsSequence([
-          ("adb -s 0123456789abcdef shell 'ps'",
-           'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
-           'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
-                'this.is.a.test.process\r\n'),
           ("adb -s 0123456789abcdef shell 'ps'",
            'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
            'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
@@ -582,7 +575,8 @@ class DeviceUtilsKillAllTest(DeviceUtilsOldImplTest):
                 'this.is.a.test.process\r\n'),
           ("adb -s 0123456789abcdef shell 'ps'",
            'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n')]):
-        self.device.KillAll('this.is.a.test.process', blocking=True)
+        self.assertEquals(1,
+            self.device.KillAll('this.is.a.test.process', blocking=True))
 
   def testKillAll_root(self):
     with self.assertCallsSequence([
@@ -590,25 +584,20 @@ class DeviceUtilsKillAllTest(DeviceUtilsOldImplTest):
            'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
            'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
                 'this.is.a.test.process\r\n'),
-          ("adb -s 0123456789abcdef shell 'ps'",
-           'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
-           'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
-                'this.is.a.test.process\r\n'),
+          ("adb -s 0123456789abcdef shell 'ls /root'", 'Permission denied\r\n'),
           ("adb -s 0123456789abcdef shell 'su -c kill -9 1234'", '')]):
-      self.device.KillAll('this.is.a.test.process', as_root=True)
+      self.assertEquals(1,
+          self.device.KillAll('this.is.a.test.process', as_root=True))
 
   def testKillAll_sigterm(self):
     with self.assertCallsSequence([
-        ("adb -s 0123456789abcdef shell 'ps'",
-         'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
-         'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
-              'this.is.a.test.process\r\n'),
         ("adb -s 0123456789abcdef shell 'ps'",
          'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
          'u0_a1  1234  174   123456 54321 ffffffff 456789ab '
               'this.is.a.test.process\r\n'),
         ("adb -s 0123456789abcdef shell 'kill -15 1234'", '')]):
-      self.device.KillAll('this.is.a.test.process', signum=signal.SIGTERM)
+      self.assertEquals(1,
+          self.device.KillAll('this.is.a.test.process', signum=signal.SIGTERM))
 
 
 class DeviceUtilsStartActivityTest(DeviceUtilsOldImplTest):

tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py

@@ -205,7 +205,7 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
       self._adb.device().SetProp('socket.relaxsslcheck', 'yes')
 
     # Kill old browser.
-    self._adb.device().ForceStop(self._backend_settings.package)
+    self._KillBrowser()
 
     if self._adb.device().old_interface.CanAccessProtectedFileContents():
       if self.browser_options.profile_dir:
@@ -229,6 +229,13 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
       self._adb.device().RunShellCommand(
           'am set-debug-app --persistent %s' % self._backend_settings.package)
 
+  def _KillBrowser(self):
+    # We use KillAll rather than ForceStop for efficiency reasons.
+    try:
+      self._adb.device().KillAll(self._backend_settings.package, retries=0)
+    except device_errors.CommandFailedError:
+      pass
+
   def _SetUpCommandLine(self):
     def QuoteIfNeeded(arg):
       # Escape 'key=valueA valueB' to 'key="valueA valueB"'
@@ -373,7 +380,7 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
 
   def Close(self):
     super(AndroidBrowserBackend, self).Close()
-    self._adb.device().ForceStop(self._backend_settings.package)
+    self._KillBrowser()
 
     # Restore android.net SSL check
     if self._backend_settings.relax_ssl_check: