[Android] Reland cleanup of old_interface in build/android/pylib.

Original CL: https://codereview.chromium.org/1047703002/

BUG=267773

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

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

20 files changed, 77 insertions, 74 deletions

diff --git a/build/android/pylib/base/base_test_runner.py b/build/android/pylib/base/base_test_runner.py
index 4e2eae70..1ca0338 100644
--- a/build/android/pylib/base/base_test_runner.py
+++ b/build/android/pylib/base/base_test_runner.py
@@ -26,12 +26,11 @@ NET_TEST_SERVER_PORT_INFO_FILE = 'net-test-server-ports'
 class BaseTestRunner(object):
   """Base class for running tests on a single device."""
 
-  def __init__(self, device_serial, tool, cleanup_test_files=False):
+  def __init__(self, device_serial, tool):
     """
       Args:
         device: Tests will run on the device of this ID.
         tool: Name of the Valgrind tool.
-        cleanup_test_files: Whether or not to cleanup test files on device.
     """
     self.device_serial = device_serial
     self.device = device_utils.DeviceUtils(device_serial)
@@ -45,7 +44,6 @@ class BaseTestRunner(object):
     # starting it in TestServerThread.
     self.test_server_spawner_port = 0
     self.test_server_port = 0
-    self._cleanup_test_files = cleanup_test_files
 
   def _PushTestServerPortInfoToDevice(self):
     """Pushes the latest port information to device."""
@@ -77,8 +75,6 @@ class BaseTestRunner(object):
   def TearDown(self):
     """Run once after all tests are run."""
     self.ShutdownHelperToolsForTestSuite()
-    if self._cleanup_test_files:
-      self.device.old_interface.RemovePushedFiles()
 
   def LaunchTestHttpServer(self, document_root, port=None,
                            extra_config_contents=None):
diff --git a/build/android/pylib/device/device_utils.py b/build/android/pylib/device/device_utils.py
index e432904..6ebef08 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -72,6 +72,17 @@ _CONTROL_CHARGING_COMMANDS = [
   },
 ]
 
+# This must be done in a single shell command.
+_RESTART_ADBD_SCRIPT = """
+    function restart() {
+      stop adbd
+      start adbd
+    }
+
+    restart &
+"""
+
+
 @decorators.WithExplicitTimeoutAndRetries(
     _DEFAULT_TIMEOUT, _DEFAULT_RETRIES)
 def GetAVDs():
@@ -383,6 +394,13 @@ class DeviceUtils(object):
     if wifi:
       timeout_retry.WaitFor(wifi_enabled)
 
+  @decorators.WithTimeoutAndRetriesFromInstance()
+  def RestartAdbd(self, timeout=None, retries=None):
+    with device_temp_file.DeviceTempFile(self.adb) as tmp:
+      self.WriteFile(tmp.name, _RESTART_ADBD_SCRIPT)
+      self.RunShellCommand(['sh', tmp.name], as_root=True, check_return=True)
+    self.adb.WaitForDevice()
+
   REBOOT_DEFAULT_TIMEOUT = 10 * _DEFAULT_TIMEOUT
   REBOOT_DEFAULT_RETRIES = _DEFAULT_RETRIES
 
diff --git a/build/android/pylib/gtest/test_options.py b/build/android/pylib/gtest/test_options.py
index 5099ba6..58cd82b 100644
--- a/build/android/pylib/gtest/test_options.py
+++ b/build/android/pylib/gtest/test_options.py
@@ -8,7 +8,6 @@ import collections
 
 GTestOptions = collections.namedtuple('GTestOptions', [
     'tool',
-    'cleanup_test_files',
     'gtest_filter',
     'run_disabled',
     'test_arguments',
diff --git a/build/android/pylib/gtest/test_package_exe.py b/build/android/pylib/gtest/test_package_exe.py
index e607cad..ee95b5c 100644
--- a/build/android/pylib/gtest/test_package_exe.py
+++ b/build/android/pylib/gtest/test_package_exe.py
@@ -6,6 +6,7 @@
 
 import logging
 import os
+import posixpath
 import sys
 import tempfile
 
@@ -119,14 +120,19 @@ class TestPackageExecutable(TestPackage):
 
   #override
   def GetAllTests(self, device):
-    cmd = '%s %s/%s --gtest_list_tests' % (self.tool.GetTestWrapper(),
-        constants.TEST_EXECUTABLE_DIR, self.suite_name)
-    lib_path = '%s/%s_deps' % (constants.TEST_EXECUTABLE_DIR, self.suite_name)
-    (exit_code, output) = device.old_interface.GetAndroidToolStatusAndOutput(
-        cmd, lib_path=lib_path)
-    if exit_code != 0:
-      raise Exception(
-          'Failed to start binary:\n%s' % '\n'.join(output))
+    lib_path = posixpath.join(
+        constants.TEST_EXECUTABLE_DIR, '%s_deps' % self.suite_name)
+
+    cmd = []
+    for wrapper in (device.GetDevicePieWrapper(), self.tool.GetTestWrapper()):
+      if wrapper:
+        cmd.append(wrapper)
+    cmd.extend([
+        posixpath.join(constants.TEST_EXECUTABLE_DIR, self.suite_name),
+        '--gtest_list_tests'])
+
+    output = device.RunShellCommand(
+        cmd, check_return=True, env={'LD_LIBRARY_PATH': lib_path})
     return gtest_test_instance.ParseGTestListTests(output)
 
   #override
diff --git a/build/android/pylib/gtest/test_runner.py b/build/android/pylib/gtest/test_runner.py
index 4bb9737..0fc6c12 100644
--- a/build/android/pylib/gtest/test_runner.py
+++ b/build/android/pylib/gtest/test_runner.py
@@ -49,8 +49,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
       test_package: An instance of TestPackage class.
     """
 
-    super(TestRunner, self).__init__(device, test_options.tool,
-                                     test_options.cleanup_test_files)
+    super(TestRunner, self).__init__(device, test_options.tool)
 
     self.test_package = test_package
     self.test_package.tool = self.tool
diff --git a/build/android/pylib/host_driven/setup.py b/build/android/pylib/host_driven/setup.py
index 285bcd4..b2ed348 100644
--- a/build/android/pylib/host_driven/setup.py
+++ b/build/android/pylib/host_driven/setup.py
@@ -195,7 +195,6 @@ def InstrumentationSetup(host_driven_test_root, official_build,
   def TestRunnerFactory(device, shard_index):
     return test_runner.HostDrivenTestRunner(
         device, shard_index,
-        instrumentation_options.tool,
-        instrumentation_options.cleanup_test_files)
+        instrumentation_options.tool)
 
   return (TestRunnerFactory, available_tests)
diff --git a/build/android/pylib/host_driven/test_case.py b/build/android/pylib/host_driven/test_case.py
index 8c372cd..a7c6a18 100644
--- a/build/android/pylib/host_driven/test_case.py
+++ b/build/android/pylib/host_driven/test_case.py
@@ -50,8 +50,6 @@ class HostDrivenTestCase(object):
       instrumentation_options: An InstrumentationOptions object.
     """
     class_name = self.__class__.__name__
-    self.adb = None
-    self.cleanup_test_files = False
     self.device = None
     self.device_id = ''
     self.has_forwarded_ports = False
@@ -67,15 +65,12 @@ class HostDrivenTestCase(object):
 
   # TODO(bulach): make ports_to_forward not optional and move the Forwarder
   # mapping here.
-  def SetUp(self, device, shard_index,
-            cleanup_test_files, ports_to_forward=None):
+  def SetUp(self, device, shard_index, ports_to_forward=None):
     if not ports_to_forward:
       ports_to_forward = []
     self.device_id = device
     self.shard_index = shard_index
     self.device = device_utils.DeviceUtils(self.device_id)
-    self.adb = self.device.old_interface
-    self.cleanup_test_files = cleanup_test_files
     if ports_to_forward:
       self.ports_to_forward = ports_to_forward
 
diff --git a/build/android/pylib/host_driven/test_runner.py b/build/android/pylib/host_driven/test_runner.py
index 865be20..5e175bc 100644
--- a/build/android/pylib/host_driven/test_runner.py
+++ b/build/android/pylib/host_driven/test_runner.py
@@ -47,23 +47,27 @@ class HostDrivenTestRunner(base_test_runner.BaseTestRunner):
   result, rather than being re-raised on the main thread.
   """
 
+  # TODO(jbudorick): Remove cleanup_test_files once it's no longer used.
+  # pylint: disable=unused-argument
   #override
-  def __init__(self, device, shard_index, tool, cleanup_test_files):
+  def __init__(self, device, shard_index, tool, cleanup_test_files=None):
     """Creates a new HostDrivenTestRunner.
 
     Args:
       device: Attached android device.
       shard_index: Shard index.
       tool: Name of the Valgrind tool.
-      cleanup_test_files: Whether or not to cleanup test files on device.
+      cleanup_test_files: Deprecated.
     """
 
-    super(HostDrivenTestRunner, self).__init__(device, tool, cleanup_test_files)
+    super(HostDrivenTestRunner, self).__init__(device, tool)
 
     # The shard index affords the ability to create unique port numbers (e.g.
     # DEFAULT_PORT + shard_index) if the test so wishes.
     self.shard_index = shard_index
 
+  # pylint: enable=unused-argument
+
   #override
   def RunTest(self, test):
     """Sets up and runs a test case.
@@ -82,7 +86,7 @@ class HostDrivenTestRunner(base_test_runner.BaseTestRunner):
     exception_raised = False
 
     try:
-      test.SetUp(str(self.device), self.shard_index, self._cleanup_test_files)
+      test.SetUp(str(self.device), self.shard_index)
     except Exception:
       logging.exception(
           'Caught exception while trying to run SetUp() for test: ' +
diff --git a/build/android/pylib/instrumentation/test_options.py b/build/android/pylib/instrumentation/test_options.py
index 4a16b11..792010b 100644
--- a/build/android/pylib/instrumentation/test_options.py
+++ b/build/android/pylib/instrumentation/test_options.py
@@ -8,7 +8,6 @@ import collections
 
 InstrumentationOptions = collections.namedtuple('InstrumentationOptions', [
     'tool',
-    'cleanup_test_files',
     'annotations',
     'exclude_annotations',
     'test_filter',
diff --git a/build/android/pylib/instrumentation/test_runner.py b/build/android/pylib/instrumentation/test_runner.py
index f3983fd..adc424b0 100644
--- a/build/android/pylib/instrumentation/test_runner.py
+++ b/build/android/pylib/instrumentation/test_runner.py
@@ -48,8 +48,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
       test_pkg: A TestPackage object.
       additional_flags: A list of additional flags to add to the command line.
     """
-    super(TestRunner, self).__init__(device, test_options.tool,
-                                     test_options.cleanup_test_files)
+    super(TestRunner, self).__init__(device, test_options.tool)
     self._lighttp_port = constants.LIGHTTPD_RANDOM_PORT_FIRST + shard_index
     self._logcat_monitor = None
 
diff --git a/build/android/pylib/linker/setup.py b/build/android/pylib/linker/setup.py
index ff21f10..5776f5af 100644
--- a/build/android/pylib/linker/setup.py
+++ b/build/android/pylib/linker/setup.py
@@ -40,7 +40,6 @@ def Setup(args, _devices):
                  if t.qualified_name in filtered_test_names]
 
   def TestRunnerFactory(device, _shard_index):
-    return test_runner.LinkerTestRunner(
-        device, args.tool, args.cleanup_test_files)
+    return test_runner.LinkerTestRunner(device, args.tool)
 
   return (TestRunnerFactory, all_tests)
diff --git a/build/android/pylib/linker/test_runner.py b/build/android/pylib/linker/test_runner.py
index 3680f83..b6803e4 100644
--- a/build/android/pylib/linker/test_runner.py
+++ b/build/android/pylib/linker/test_runner.py
@@ -49,16 +49,14 @@ class LinkerTestRunner(base_test_runner.BaseTestRunner):
   """
 
   #override
-  def __init__(self, device, tool, cleanup_test_files):
+  def __init__(self, device, tool):
     """Creates a new LinkerTestRunner.
 
     Args:
       device: Attached android device.
       tool: Name of the Valgrind tool.
-      cleanup_test_files: Whether or not to cleanup test files on device.
     """
-
-    super(LinkerTestRunner, self).__init__(device, tool, cleanup_test_files)
+    super(LinkerTestRunner, self).__init__(device, tool)
 
   #override
   def InstallTestPackage(self):
@@ -68,8 +66,7 @@ class LinkerTestRunner(base_test_runner.BaseTestRunner):
     if not os.path.exists(apk_path):
       raise Exception('%s not found, please build it' % apk_path)
 
-    package_name = apk_helper.GetPackageName(apk_path)
-    self.device.old_interface.ManagedInstall(apk_path, package_name)
+    self.device.Install(apk_path)
 
   #override
   def RunTest(self, test):
diff --git a/build/android/pylib/perf/perf_control.py b/build/android/pylib/perf/perf_control.py
index 97fa4a7..b6a0989e 100644
--- a/build/android/pylib/perf/perf_control.py
+++ b/build/android/pylib/perf/perf_control.py
@@ -28,7 +28,7 @@ class PerfControl(object):
 
   def SetHighPerfMode(self):
     """Sets the highest stable performance mode for the device."""
-    if not self._device.old_interface.IsRootEnabled():
+    if not self._device.HasRoot():
       message = 'Need root for performance mode. Results may be NOISY!!'
       logging.warning(message)
       # Add an additional warning at exit, such that it's clear that any results
@@ -58,13 +58,13 @@ class PerfControl(object):
     self._ForceAllCpusOnline(True)
     self._SetScalingGovernorInternal('performance')
     if not self._AllCpusAreOnline():
-      if not self._device.old_interface.IsRootEnabled():
+      if not self._device.HasRoot():
         raise RuntimeError('Need root to force CPUs online.')
       raise RuntimeError('Failed to force CPUs online.')
 
   def SetDefaultPerfMode(self):
     """Sets the performance mode for the device to its default mode."""
-    if not self._device.old_interface.IsRootEnabled():
+    if not self._device.HasRoot():
       return
     product_model = self._device.product_model
     if 'Nexus 5' == product_model:
diff --git a/build/android/pylib/perf/test_runner.py b/build/android/pylib/perf/test_runner.py
index da66d07..20f0fd0 100644
--- a/build/android/pylib/perf/test_runner.py
+++ b/build/android/pylib/perf/test_runner.py
@@ -170,7 +170,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
       tests: a dict mapping test_name to command.
       flaky_tests: a list of flaky test_name.
     """
-    super(TestRunner, self).__init__(device, None, 'Release')
+    super(TestRunner, self).__init__(device, None)
     self._options = test_options
     self._shard_index = shard_index
     self._max_shard = max_shard
@@ -235,7 +235,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
     try:
       logging.warning('Unmapping device ports')
       forwarder.Forwarder.UnmapAllDevicePorts(self.device)
-      self.device.old_interface.RestartAdbdOnDevice()
+      self.device.RestartAdbd()
     except Exception as e:
       logging.error('Exception when tearing down device %s', e)
 
diff --git a/build/android/pylib/screenshot.py b/build/android/pylib/screenshot.py
index e21d756..2704631 100644
--- a/build/android/pylib/screenshot.py
+++ b/build/android/pylib/screenshot.py
@@ -5,6 +5,7 @@
 import os
 import signal
 import tempfile
+import time
 
 from pylib import cmd_helper
 
@@ -82,10 +83,13 @@ class VideoRecorder(object):
     Returns:
       Output video file name on the host.
     """
-    host_file_name = host_file or ('screen-recording-%s.mp4' %
-                                   self._device.old_interface.GetTimestamp())
+    # TODO(jbudorick): Merge filename generation with the logic for doing so in
+    # DeviceUtils.
+    host_file_name = (
+        host_file
+        or 'screen-recording-%s.mp4' % time.strftime('%Y%m%dT%H%M%S',
+                                                     time.localtime()))
     host_file_name = os.path.abspath(host_file_name)
-    self._device.old_interface.EnsureHostDirectory(host_file_name)
     self._device.PullFile(self._device_file, host_file_name)
     self._device.RunShellCommand('rm -f "%s"' % self._device_file)
     return host_file_name
diff --git a/build/android/pylib/uiautomator/test_options.py b/build/android/pylib/uiautomator/test_options.py
index b383b20..3f5f950 100644
--- a/build/android/pylib/uiautomator/test_options.py
+++ b/build/android/pylib/uiautomator/test_options.py
@@ -8,7 +8,6 @@ import collections
 
 UIAutomatorOptions = collections.namedtuple('UIAutomatorOptions', [
     'tool',
-    'cleanup_test_files',
     'annotations',
     'exclude_annotations',
     'test_filter',
diff --git a/build/android/pylib/uiautomator/test_runner.py b/build/android/pylib/uiautomator/test_runner.py
index d7a4bdf..296bd47 100644
--- a/build/android/pylib/uiautomator/test_runner.py
+++ b/build/android/pylib/uiautomator/test_runner.py
@@ -26,7 +26,6 @@ class TestRunner(instr_test_runner.TestRunner):
     # Create an InstrumentationOptions object to pass to the super class
     instrumentation_options = instr_test_options.InstrumentationOptions(
         test_options.tool,
-        test_options.cleanup_test_files,
         test_options.annotations,
         test_options.exclude_annotations,
         test_options.test_filter,
diff --git a/build/android/pylib/utils/test_environment.py b/build/android/pylib/utils/test_environment.py
index 4d88a45..f55a4ac 100644
--- a/build/android/pylib/utils/test_environment.py
+++ b/build/android/pylib/utils/test_environment.py
@@ -34,13 +34,14 @@ def CleanupLeftoverProcesses():
   """Clean up the test environment, restarting fresh adb and HTTP daemons."""
   _KillWebServers()
   device_utils.RestartServer()
-  p = device_utils.DeviceUtils.parallel()
-  p.old_interface.RestartAdbdOnDevice()
-  try:
-    p.EnableRoot()
-  except device_errors.CommandFailedError as e:
-    # TODO(jbudorick) Handle this exception appropriately after interface
-    #                 conversions are finished.
-    logging.error(str(e))
-  p.WaitUntilFullyBooted()
+
+  def cleanup_device(d):
+    d.RestartAdbd()
+    try:
+      d.EnableRoot()
+    except device_errors.CommandFailedError as e:
+      logging.error(str(e))
+    d.WaitUntilFullyBooted()
+
+  device_utils.DeviceUtils.parallel().pMap(cleanup_device)
 
diff --git a/build/android/test_runner.py b/build/android/test_runner.py
index 87f4cec..d07bb82 100755
--- a/build/android/test_runner.py
+++ b/build/android/test_runner.py
@@ -175,9 +175,6 @@ def AddRemoteDeviceOptions(parser):
 def AddDeviceOptions(parser):
   """Adds device options to |parser|."""
   group = parser.add_argument_group(title='Device Options')
-  group.add_argument('-c', dest='cleanup_test_files',
-                     help='Cleanup test files on the device after run',
-                     action='store_true')
   group.add_argument('--tool',
                      dest='tool',
                      help=('Run the test under a tool '
@@ -372,7 +369,6 @@ def ProcessInstrumentationOptions(args):
   # TODO(jbudorick): Get rid of InstrumentationOptions.
   return instrumentation_test_options.InstrumentationOptions(
       args.tool,
-      args.cleanup_test_files,
       args.annotations,
       args.exclude_annotations,
       args.test_filter,
@@ -437,7 +433,6 @@ def ProcessUIAutomatorOptions(args):
 
   return uiautomator_test_options.UIAutomatorOptions(
       args.tool,
-      args.cleanup_test_files,
       args.annotations,
       args.exclude_annotations,
       args.test_filter,
@@ -636,7 +631,6 @@ def _RunGTests(args, devices):
     # into the gtest code.
     gtest_options = gtest_test_options.GTestOptions(
         args.tool,
-        args.cleanup_test_files,
         args.test_filter,
         args.run_disabled,
         args.test_arguments,
diff --git a/build/android/update_verification.py b/build/android/update_verification.py
index fe89567..9539d94 100755
--- a/build/android/update_verification.py
+++ b/build/android/update_verification.py
@@ -17,7 +17,7 @@ from pylib.device import device_utils
 
 def _SaveAppData(device, package_name, from_apk=None, data_dir=None):
   def _BackupAppData(data_dir=None):
-    device.old_interface.Adb().SendCommand('backup %s' % package_name)
+    device.adb.Backup(package_name)
     backup_file = os.path.join(os.getcwd(), 'backup.ab')
     assert os.path.exists(backup_file), 'Backup failed.'
     if data_dir:
@@ -29,8 +29,7 @@ def _SaveAppData(device, package_name, from_apk=None, data_dir=None):
 
   if from_apk:
     logging.info('Installing %s...', from_apk)
-    # TODO(jbudorick) Switch to AdbWrapper.Install on the impl switch.
-    output = device.old_interface.Install(from_apk, reinstall=True)
+    output = device.Install(from_apk, reinstall=True)
     if 'Success' not in output:
       raise Exception('Unable to install %s. output: %s' % (from_apk, output))
 
@@ -42,14 +41,13 @@ def _SaveAppData(device, package_name, from_apk=None, data_dir=None):
 def _VerifyAppUpdate(device, to_apk, app_data, from_apk=None):
   def _RestoreAppData():
     assert os.path.exists(app_data), 'Backup file does not exist!'
-    device.old_interface.Adb().SendCommand('restore %s' % app_data)
+    device.adb.Restore(app_data)
     # It seems restore command is not synchronous.
     time.sleep(15)
 
   if from_apk:
     logging.info('Installing %s...', from_apk)
-    # TODO(jbudorick) Switch to AdbWrapper.Install on the impl switch.
-    output = device.old_interface.Install(from_apk, reinstall=True)
+    output = device.Install(from_apk, reinstall=True)
     if 'Success' not in output:
       raise Exception('Unable to install %s. output: %s' % (from_apk, output))
 
@@ -59,8 +57,7 @@ def _VerifyAppUpdate(device, to_apk, app_data, from_apk=None):
 
   logging.info('Verifying that %s cannot be installed side-by-side...',
                to_apk)
-  # TODO(jbudorick) Switch to AdbWrapper.Install on the impl switch.
-  output = device.old_interface.Install(to_apk)
+  output = device.Install(to_apk)
   if 'INSTALL_FAILED_ALREADY_EXISTS' not in output:
     if 'Success' in output:
       raise Exception('Package name has changed! output: %s' % output)
@@ -68,8 +65,7 @@ def _VerifyAppUpdate(device, to_apk, app_data, from_apk=None):
       raise Exception(output)
 
   logging.info('Verifying that %s can be overinstalled...', to_apk)
-  # TODO(jbudorick) Switch to AdbWrapper.Install on the impl switch.
-  output = device.old_interface.Install(to_apk, reinstall=True)
+  output = device.adb.Install(to_apk, reinstall=True)
   if 'Success' not in output:
     raise Exception('Unable to install %s.\n output: %s' % (to_apk, output))
   logging.info('Successfully updated to the new apk. Please verify that the '