[Android] Rework device filtering and add DeviceUtils.HealthyDevices.

BUG=267773

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

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

12 files changed, 138 insertions, 167 deletions

diff --git a/build/android/adb_install_apk.py b/build/android/adb_install_apk.py
index b6da19e..7bc634c 100755
--- a/build/android/adb_install_apk.py
+++ b/build/android/adb_install_apk.py
@@ -11,8 +11,6 @@ import os
 import sys
 
 from pylib import constants
-from pylib.device import adb_wrapper
-from pylib.device import device_filter
 from pylib.device import device_utils
 
 
@@ -72,13 +70,13 @@ def main(argv):
   constants.SetBuildType(options.build_type)
   ValidateInstallAPKOption(parser, options, args)
 
-  devices = adb_wrapper.AdbWrapper.Devices(
-      filters=device_filter.DefaultFilters())
+  devices = device_utils.DeviceUtils.HealthyDevices()
 
   if options.device:
-    if options.device not in [d.GetDeviceSerial() for d in devices]:
+    device_serials = [d.adb.GetDeviceSerial() for d in devices]
+    if options.device not in device_serials:
       raise Exception('Error: %s not in attached devices %s' % (options.device,
-                      ','.join(devices)))
+                      ','.join(device_serials)))
     devices = [options.device]
 
   if not devices:
diff --git a/build/android/adb_reverse_forwarder.py b/build/android/adb_reverse_forwarder.py
index b2da6a7..6cae0cf 100755
--- a/build/android/adb_reverse_forwarder.py
+++ b/build/android/adb_reverse_forwarder.py
@@ -19,7 +19,6 @@ import time
 from pylib import constants
 from pylib import forwarder
 from pylib.device import adb_wrapper
-from pylib.device import device_filter
 from pylib.device import device_utils
 from pylib.utils import run_tests_helper
 
@@ -54,11 +53,10 @@ def main(argv):
     parser.error('Bad port number')
     sys.exit(1)
 
-  devices = adb_wrapper.AdbWrapper.Devices(
-      filters=device_filter.DefaultFilters())
+  devices = device_utils.DeviceUtils.HealthyDevices()
 
   if options.device:
-    if options.device not in [d.GetDeviceSerial() for d in devices]:
+    if options.device not in [str(d) for d in devices]:
       raise Exception('Error: %s not in attached devices %s' % (options.device,
                       ','.join(devices)))
     devices = [options.device]
@@ -67,7 +65,7 @@ def main(argv):
       raise Exception('Error: no connected devices')
     logging.info('No device specified. Defaulting to %s', devices[0])
 
-  device = device_utils.DeviceUtils(devices[0])
+  device = devices[0]
   constants.SetBuildType(options.build_type)
   try:
     forwarder.Forwarder.Map(port_pairs, device)
diff --git a/build/android/host_heartbeat.py b/build/android/host_heartbeat.py
index 11421ce..6a7cdd1 100755
--- a/build/android/host_heartbeat.py
+++ b/build/android/host_heartbeat.py
@@ -12,8 +12,6 @@ This heart beat lets the devices know that they are connected to a host.
 import sys
 import time
 
-from pylib.device import adb_wrapper
-from pylib.device import device_filter
 from pylib.device import device_utils
 
 PULSE_PERIOD = 20
@@ -21,11 +19,10 @@ PULSE_PERIOD = 20
 def main():
   while True:
     try:
-      devices = adb_wrapper.AdbWrapper.Devices(
-          filters=device_filter.DefaultFilters())
+      devices = device_utils.DeviceUtils.HealthyDevices()
       for d in devices:
-        device_utils.DeviceUtils(d).RunShellCommand(
-            ['touch', '/sdcard/host_heartbeat'], check_return=True)
+        d.RunShellCommand(['touch', '/sdcard/host_heartbeat'],
+                          check_return=True)
     except:
       # Keep the heatbeat running bypassing all errors.
       pass
diff --git a/build/android/provision_devices.py b/build/android/provision_devices.py
index e4517b6..ee52c71 100755
--- a/build/android/provision_devices.py
+++ b/build/android/provision_devices.py
@@ -21,11 +21,9 @@ import time
 
 from pylib import constants
 from pylib import device_settings
-from pylib.device import adb_wrapper
 from pylib.device import battery_utils
 from pylib.device import device_blacklist
 from pylib.device import device_errors
-from pylib.device import device_filter
 from pylib.device import device_utils
 from pylib.utils import run_tests_helper
 from pylib.utils import timeout_retry
@@ -55,8 +53,7 @@ def ProvisionDevices(options):
   if options.device is not None:
     devices = [options.device]
   else:
-    devices = adb_wrapper.AdbWrapper.Devices(
-        filters=device_filter.DefaultFilters())
+    devices = device_utils.DeviceUtils.HealthyDevices()
 
   parallel_devices = device_utils.DeviceUtils.parallel(devices)
   parallel_devices.pMap(ProvisionDevice, options)
diff --git a/build/android/pylib/device/adb_wrapper.py b/build/android/pylib/device/adb_wrapper.py
index fa430f5..20787c1 100644
--- a/build/android/pylib/device/adb_wrapper.py
+++ b/build/android/pylib/device/adb_wrapper.py
@@ -12,18 +12,22 @@ import collections
 import errno
 import logging
 import os
+import re
 
 from pylib import cmd_helper
 from pylib import constants
 from pylib.device import decorators
 from pylib.device import device_errors
-from pylib.device import device_filter
 from pylib.utils import timeout_retry
 
 
 _DEFAULT_TIMEOUT = 30
 _DEFAULT_RETRIES = 2
 
+_EMULATOR_RE = re.compile(r'^emulator-[0-9]+$')
+
+_READY_STATE = 'device'
+
 
 def _VerifyLocalFileExists(path):
   """Verifies a local file exists.
@@ -160,22 +164,17 @@ class AdbWrapper(object):
                    cpu_affinity=0)
 
   @classmethod
-  def GetDevices(cls, filters=None, timeout=_DEFAULT_TIMEOUT,
-                 retries=_DEFAULT_RETRIES):
+  def GetDevices(cls, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES):
     """DEPRECATED. Refer to Devices(...) below."""
     # TODO(jbudorick): Remove this function once no more clients are using it.
-    return cls.Devices(filters=filters, timeout=timeout, retries=retries)
+    return cls.Devices(timeout=timeout, retries=retries)
 
   @classmethod
-  def Devices(cls, filters=None, timeout=_DEFAULT_TIMEOUT,
+  def Devices(cls, is_ready=True, timeout=_DEFAULT_TIMEOUT,
               retries=_DEFAULT_RETRIES):
     """Get the list of active attached devices.
 
     Args:
-      filters: (optional) A list of binary functions that take an AdbWrapper
-          instance and a string description. Any device for which all provided
-          filter functions do not return True will not be included in the
-          returned list.
       timeout: (optional) Timeout per try in seconds.
       retries: (optional) Number of retries to attempt.
 
@@ -184,17 +183,8 @@ class AdbWrapper(object):
     """
     output = cls._RunAdbCmd(['devices'], timeout=timeout, retries=retries)
     lines = (line.split() for line in output.splitlines())
-    devices = (AdbWrapper(line[0]) for line in lines if len(line) == 2)
-
-    def matches_all_filters(device):
-      for f in filters or ():
-        if not f(device):
-          logging.info('Device %s failed filter %s', device.GetDeviceSerial(),
-                       f.__name__)
-          return False
-      return True
-
-    return [d for d in devices if matches_all_filters(d)]
+    return [AdbWrapper(line[0]) for line in lines
+            if len(line) == 2 and (not is_ready or line[1] == _READY_STATE)]
 
   def GetDeviceSerial(self):
     """Gets the device serial number associated with this object.
@@ -551,3 +541,15 @@ class AdbWrapper(object):
     if 'cannot' in output:
       raise device_errors.AdbCommandFailedError(
           ['root'], output, device_serial=self._device_serial)
+
+  @property
+  def is_emulator(self):
+    return _EMULATOR_RE.match(self._device_serial)
+
+  @property
+  def is_ready(self):
+    try:
+      return self.GetState() == _READY_STATE
+    except device_errors.CommandFailedError:
+      return False
+
diff --git a/build/android/pylib/device/device_filter.py b/build/android/pylib/device/device_filter.py
deleted file mode 100644
index 8e54b25..0000000
--- a/build/android/pylib/device/device_filter.py
+++ /dev/null
@@ -1,53 +0,0 @@
-# Copyright 2015 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-from pylib.device import device_blacklist
-from pylib.device import device_errors
-
-
-def DefaultFilters():
-  """Returns a list of the most commonly-used device filters.
-
-  These filters match devices that:
-    - are in a "device" state (as opposed to, e.g., "unauthorized" or
-      "emulator")
-    - are not blacklisted.
-
-  Returns:
-    A list of the most commonly-used device filters.
-  """
-  return [DeviceFilter, BlacklistFilter()]
-
-
-def BlacklistFilter():
-  """Returns a filter that matches devices that are not blacklisted.
-
-  Note that this function is not the filter. It creates one when called using
-  the blacklist at that time and returns that.
-
-  Returns:
-    A filter function that matches devices that are not blacklisted.
-  """
-  blacklist = set(device_blacklist.ReadBlacklist())
-  def f(adb):
-    return adb.GetDeviceSerial() not in blacklist
-
-  return f
-
-
-def DeviceFilter(adb):
-  """A filter that matches devices in a "device" state.
-
-  (Basically, this is adb get-state == "device")
-
-  Args:
-    adb: An instance of AdbWrapper.
-  Returns:
-    True if the device is in a "device" state.
-  """
-  try:
-    return adb.GetState() == 'device'
-  except device_errors.CommandFailedError:
-    return False
-
diff --git a/build/android/pylib/device/device_utils.py b/build/android/pylib/device/device_utils.py
index b68c3b2..46aec6f 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -30,7 +30,6 @@ from pylib.device import adb_wrapper
 from pylib.device import decorators
 from pylib.device import device_blacklist
 from pylib.device import device_errors
-from pylib.device import device_filter
 from pylib.device import intent
 from pylib.device import logcat_monitor
 from pylib.device.commands import install_commands
@@ -1540,6 +1539,18 @@ class DeviceUtils(object):
 
     return self._cache['run_pie']
 
+  def GetClientCache(self, client_name):
+    """Returns client cache."""
+    if client_name not in self._client_caches:
+      self._client_caches[client_name] = {}
+    return self._client_caches[client_name]
+
+  def _ClearCache(self):
+    """Clears all caches."""
+    for client in self._client_caches:
+      self._client_caches[client].clear()
+    self._cache.clear()
+
   @classmethod
   def parallel(cls, devices=None, async=False):
     """Creates a Parallelizer to operate over the provided list of devices.
@@ -1558,8 +1569,7 @@ class DeviceUtils(object):
       A Parallelizer operating over |devices|.
     """
     if not devices:
-      devices = adb_wrapper.AdbWrapper.Devices(
-          filters=device_filter.DefaultFilters())
+      devices = cls.HealthyDevices()
       if not devices:
         raise device_errors.NoDevicesError()
 
@@ -1569,14 +1579,15 @@ class DeviceUtils(object):
     else:
       return parallelizer.SyncParallelizer(devices)
 
-  def GetClientCache(self, client_name):
-    """Returns client cache."""
-    if client_name not in self._client_caches:
-      self._client_caches[client_name] = {}
-    return self._client_caches[client_name]
+  @classmethod
+  def HealthyDevices(cls):
+    blacklist = device_blacklist.ReadBlacklist()
+    def blacklisted(adb):
+      if adb.GetDeviceSerial() in blacklist:
+        logging.warning('Device %s is blacklisted.', adb.GetDeviceSerial())
+        return True
+      return False
+
+    return [cls(adb) for adb in adb_wrapper.AdbWrapper.Devices()
+            if not blacklisted(adb)]
 
-  def _ClearCache(self):
-    """Clears all caches."""
-    for client in self._client_caches:
-      self._client_caches[client].clear()
-    self._cache.clear()
diff --git a/build/android/pylib/device/device_utils_test.py b/build/android/pylib/device/device_utils_test.py
index 3806e26..4301e9d9 100755
--- a/build/android/pylib/device/device_utils_test.py
+++ b/build/android/pylib/device/device_utils_test.py
@@ -1547,30 +1547,6 @@ class DeviceUtilsStrTest(DeviceUtilsTest):
       self.assertEqual('0123456789abcdef', str(self.device))
 
 
-class DeviceUtilsParallelTest(mock_calls.TestCase):
-
-  def testParallel_default(self):
-    test_serials = ['0123456789abcdef', 'fedcba9876543210']
-    with self.assertCalls(
-        (mock.call.pylib.device.device_filter.DefaultFilters(), None),
-        (mock.call.pylib.device.adb_wrapper.AdbWrapper.Devices(filters=None),
-         [_AdbWrapperMock(serial) for serial in test_serials])):
-      parallel_devices = device_utils.DeviceUtils.parallel()
-    for serial, device in zip(test_serials, parallel_devices.pGet(None)):
-      self.assertTrue(
-          isinstance(device, device_utils.DeviceUtils)
-          and serial == str(device),
-          'Expected a DeviceUtils object with serial %s' % serial)
-
-  def testParallel_noDevices(self):
-    with self.assertCalls(
-        (mock.call.pylib.device.device_filter.DefaultFilters(), None),
-        (mock.call.pylib.device.adb_wrapper.AdbWrapper.Devices(filters=None),
-         [])):
-      with self.assertRaises(device_errors.NoDevicesError):
-        device_utils.DeviceUtils.parallel()
-
-
 class DeviceUtilsClientCache(DeviceUtilsTest):
 
   def testClientCache_twoCaches(self):
@@ -1597,6 +1573,57 @@ class DeviceUtilsClientCache(DeviceUtilsTest):
     self.assertEqual(client_cache_one, {})
     self.assertEqual(client_cache_two, {})
 
+
+class DeviceUtilsParallelTest(mock_calls.TestCase):
+
+  def testParallel_default(self):
+    test_serials = ['0123456789abcdef', 'fedcba9876543210']
+    with self.assertCall(
+        mock.call.pylib.device.device_utils.DeviceUtils.HealthyDevices(),
+        [device_utils.DeviceUtils(s) for s in test_serials]):
+      parallel_devices = device_utils.DeviceUtils.parallel()
+    for serial, device in zip(test_serials, parallel_devices.pGet(None)):
+      self.assertTrue(isinstance(device, device_utils.DeviceUtils))
+      self.assertEquals(serial, device.adb.GetDeviceSerial())
+
+  def testParallel_noDevices(self):
+    with self.assertCall(
+        mock.call.pylib.device.device_utils.DeviceUtils.HealthyDevices(), []):
+      with self.assertRaises(device_errors.NoDevicesError):
+        device_utils.DeviceUtils.parallel()
+
+
+class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase):
+
+  def _createAdbWrapperMock(self, serial, is_ready=True):
+    adb = _AdbWrapperMock(serial)
+    adb.is_ready = is_ready
+    return adb
+
+  def testHealthyDevices_default(self):
+    test_serials = ['0123456789abcdef', 'fedcba9876543210']
+    with self.assertCalls(
+        (mock.call.pylib.device.device_blacklist.ReadBlacklist(), []),
+        (mock.call.pylib.device.adb_wrapper.AdbWrapper.Devices(),
+         [self._createAdbWrapperMock(s) for s in test_serials])):
+      devices = device_utils.DeviceUtils.HealthyDevices()
+    for serial, device in zip(test_serials, devices):
+      self.assertTrue(isinstance(device, device_utils.DeviceUtils))
+      self.assertEquals(serial, device.adb.GetDeviceSerial())
+
+  def testHealthyDevices_blacklisted(self):
+    test_serials = ['0123456789abcdef', 'fedcba9876543210']
+    with self.assertCalls(
+        (mock.call.pylib.device.device_blacklist.ReadBlacklist(),
+         ['fedcba9876543210']),
+        (mock.call.pylib.device.adb_wrapper.AdbWrapper.Devices(),
+         [self._createAdbWrapperMock(s) for s in test_serials])):
+      devices = device_utils.DeviceUtils.HealthyDevices()
+    self.assertEquals(1, len(devices))
+    self.assertTrue(isinstance(devices[0], device_utils.DeviceUtils))
+    self.assertEquals('0123456789abcdef', devices[0].adb.GetDeviceSerial())
+
+
 if __name__ == '__main__':
   logging.getLogger().setLevel(logging.DEBUG)
   unittest.main(verbosity=2)
diff --git a/build/android/pylib/local/device/local_device_environment.py b/build/android/pylib/local/device/local_device_environment.py
index 8346762..0d02ca3 100644
--- a/build/android/pylib/local/device/local_device_environment.py
+++ b/build/android/pylib/local/device/local_device_environment.py
@@ -5,7 +5,6 @@
 from pylib.base import environment
 from pylib.device import adb_wrapper
 from pylib.device import device_errors
-from pylib.device import device_filter
 from pylib.device import device_utils
 from pylib.utils import parallelizer
 
@@ -14,26 +13,24 @@ class LocalDeviceEnvironment(environment.Environment):
 
   def __init__(self, args, _error_func):
     super(LocalDeviceEnvironment, self).__init__()
-    self._device = args.test_device
+    self._device_serial = args.test_device
     self._devices = []
     self._max_tries = 1 + args.num_retries
     self._tool_name = args.tool
 
   #override
   def SetUp(self):
-    available_devices = adb_wrapper.AdbWrapper.Devices(
-        filters=device_filter.DefaultFilters())
+    available_devices = device_utils.DeviceUtils.HealthyDevices()
     if not available_devices:
       raise device_errors.NoDevicesError
-    if self._device:
-      if self._device not in available_devices:
+    if self._device_serial:
+      self._devices = [d for d in available_devices
+                       if d.adb.GetDeviceSerial == self._device_serial]
+      if not self._devices:
         raise device_errors.DeviceUnreachableError(
-            'Could not find device %r' % self._device)
-      self._devices = [device_utils.DeviceUtils(self._device)]
+            'Could not find device %r' % self._device_serial)
     else:
-      self._devices = [
-          device_utils.DeviceUtils(s)
-          for s in available_devices]
+      self._devices = available_devices
 
   @property
   def devices(self):
diff --git a/build/android/screenshot.py b/build/android/screenshot.py
index 4c1e247..c48a255 100755
--- a/build/android/screenshot.py
+++ b/build/android/screenshot.py
@@ -12,8 +12,7 @@ import os
 import sys
 
 from pylib import screenshot
-from pylib.device import adb_wrapper
-from pylib.device import device_filter
+from pylib.device import device_errors
 from pylib.device import device_utils
 
 def _PrintMessage(heading, eol='\n'):
@@ -67,22 +66,25 @@ def main():
 
   (options, args) = parser.parse_args()
 
+  if len(args) > 1:
+    parser.error('Too many positional arguments.')
+  host_file = args[0] if args else options.file
+
   if options.verbose:
     logging.getLogger().setLevel(logging.DEBUG)
 
-  devices = adb_wrapper.AdbWrapper.Devices(
-      filters=device_filter.DefaultFilters())
-
-  if not options.device and len(devices) > 1:
-    parser.error('Multiple devices are attached. '
-                 'Please specify device serial number with --device.')
-  elif not options.device and len(devices) == 1:
-    options.device = devices[0]
+  devices = device_utils.DeviceUtils.HealthyDevices()
 
-  if len(args) > 1:
-    parser.error('Too many positional arguments.')
-  host_file = args[0] if args else options.file
-  device = device_utils.DeviceUtils(options.device)
+  if not options.device:
+    if len(devices) > 1:
+      parser.error('Multiple devices are attached. '
+                   'Please specify device serial number with --device.')
+    elif len(devices) == 1:
+      device = devices[0]
+    else:
+      raise device_errors.NoDevicesError()
+  else:
+    device = device_utils.DeviceUtils(options.device)
 
   if options.video:
     _CaptureVideo(device, host_file, options)
diff --git a/build/android/tombstones.py b/build/android/tombstones.py
index 3f35086..c83a584 100755
--- a/build/android/tombstones.py
+++ b/build/android/tombstones.py
@@ -21,7 +21,6 @@ import optparse
 
 from pylib.device import adb_wrapper
 from pylib.device import device_errors
-from pylib.device import device_filter
 from pylib.device import device_utils
 from pylib.utils import run_tests_helper
 
@@ -234,20 +233,19 @@ def main():
   options, _ = parser.parse_args()
 
   if options.device:
-    devices = [options.device]
+    devices = [device_utils.DeviceUtils(options.device)]
   else:
-    devices = adb_wrapper.AdbWrapper.Devices(
-        filters=device_filter.DefaultFilters())
+    devices = device_utils.DeviceUtils.HealthyDevices()
 
   # This must be done serially because strptime can hit a race condition if
   # used for the first time in a multithreaded environment.
   # http://bugs.python.org/issue7980
   tombstones = []
-  for adb in devices:
-    device = device_utils.DeviceUtils(adb)
+  for device in devices:
     tombstones += _GetTombstonesForDevice(device, options)
 
   _ResolveTombstones(options.jobs, tombstones)
 
+
 if __name__ == '__main__':
   sys.exit(main())
diff --git a/build/android/update_verification.py b/build/android/update_verification.py
index 29589f3..45c6e98 100755
--- a/build/android/update_verification.py
+++ b/build/android/update_verification.py
@@ -12,8 +12,6 @@ import shutil
 import sys
 import time
 
-from pylib.device import adb_wrapper
-from pylib.device import device_filter
 from pylib.device import device_utils
 
 def _SaveAppData(device, package_name, from_apk=None, data_dir=None):
@@ -109,11 +107,10 @@ def main():
     parser.print_help(sys.stderr)
     parser.error('Unknown arguments: %s.' % args)
 
-  devices = adb_wrapper.AdbWrapper.Devices(
-      filters=device_filter.DefaultFilters())
+  devices = device_utils.DeviceUtils.HealthyDevices()
   if len(devices) != 1:
     parser.error('Exactly 1 device must be attached.')
-  device = device_utils.DeviceUtils(devices[0])
+  device = devices[0]
 
   if options.from_apk:
     assert os.path.isfile(options.from_apk)