Revert of Migrate DeviceUtils.ReadFile to adb_wrapper (patchset #6 id…

…:100001 of https://codereview.chromium.org/775333002/)

Reason for revert:
Broke telemetry android_browser_backend due to missing client change

Original issue's description:
> Migrate DeviceUtils.ReadFile to adb_wrapper
>
> The implementation is based on a simple 'cat' run by shell on the
> device (with optional as_root).
>
> Interface changes:
> - The return value is a string (instead of a list of lines).
> - An exception is raised if the file cannot be read (instead of returning an empty list).
>
> BUG=267773
>
> Committed: https://crrev.com/c75a264a0386df5931a9b6ac8c81337794d034d0
> Cr-Commit-Position: refs/heads/master@{#307672}

TBR=jbudorick@chromium.org,skyostil@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=267773

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

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

build/android/pylib/device/device_utils.py

@@ -67,13 +67,6 @@ def RestartServer():
   pylib.android_commands.AndroidCommands().RestartAdbServer()
 
 
-def _JoinLines(lines):
-  # makes sure that the last line is also terminated, and is more memory
-  # efficient than first appending an end-line to each line and then joining
-  # all of them together.
-  return ''.join(s for line in lines for s in (line, '\n'))
-
-
 class DeviceUtils(object):
 
   _VALID_SHELL_VARIABLE = re.compile('^[a-zA-Z_][a-zA-Z0-9_]*$')
@@ -865,8 +858,7 @@ def PullFile(self, device_path, host_path, timeout=None, retries=None):
     self.adb.Pull(device_path, host_path)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
-  def ReadFile(self, device_path, as_root=False,
-               timeout=None, retries=None):
+  def ReadFile(self, device_path, as_root=False, timeout=None, retries=None):
     """Reads the contents of a file from the device.
 
     Args:
@@ -878,17 +870,22 @@ def ReadFile(self, device_path, as_root=False,
       retries: number of retries
 
     Returns:
-      The contents of |device_path| as a string. Contents are intepreted using
-      universal newlines, so the caller will see them encoded as '\n'. Also,
-      all lines will be terminated.
+      The contents of the file at |device_path| as a list of lines.
 
     Raises:
-      AdbCommandFailedError if the file can't be read.
+      CommandFailedError if the file can't be read.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    return _JoinLines(self.RunShellCommand(
-        ['cat', device_path], as_root=as_root, check_return=True))
+    # TODO(jbudorick) Evaluate whether we want to return a list of lines after
+    # the implementation switch, and if file not found should raise exception.
+    if as_root:
+      if not self.old_interface.CanAccessProtectedFileContents():
+        raise device_errors.CommandFailedError(
+          'Cannot read from %s with root privileges.' % device_path)
+      return self.old_interface.GetProtectedFileContents(device_path)
+    else:
+      return self.old_interface.GetFileContents(device_path)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def WriteFile(self, device_path, contents, as_root=False, force_push=False,

build/android/pylib/device/device_utils_test.py

@@ -273,7 +273,7 @@ def setUp(self):
   def ShellError(self, output=None, exit_code=1):
     def action(cmd, *args, **kwargs):
       raise device_errors.AdbCommandFailedError(
-          ['shell', cmd], output, exit_code, str(self.device))
+          cmd, output, exit_code, str(self.device))
     if output is None:
       output = 'Permission denied\n'
     return action
@@ -1109,33 +1109,56 @@ def testPullFile_doesntExistOnDevice(self):
                                '/test/file/host/path')
 
 
-class DeviceUtilsReadFileTest(DeviceUtilsNewImplTest):
+class DeviceUtilsReadFileTest(DeviceUtilsOldImplTest):
 
   def testReadFile_exists(self):
-    with self.assertCall(
-        self.call.adb.Shell('cat /read/this/test/file'),
-        'this is a test file\r\n'):
-      self.assertEqual('this is a test file\n',
+    with self.assertCallsSequence([
+        ("adb -s 0123456789abcdef shell "
+            "'cat \"/read/this/test/file\" 2>/dev/null'",
+         'this is a test file')]):
+      self.assertEqual(['this is a test file'],
                        self.device.ReadFile('/read/this/test/file'))
 
   def testReadFile_doesNotExist(self):
-    with self.assertCall(
-        self.call.adb.Shell('cat /this/file/does.not.exist'),
-        self.ShellError('/system/bin/sh: cat: /this/file/does.not.exist: '
-                        'No such file or directory')):
-      with self.assertRaises(device_errors.AdbCommandFailedError):
-        self.device.ReadFile('/this/file/does.not.exist')
-
-  def testReadFile_withSU(self):
     with self.assertCalls(
-      (self.call.device.NeedsSU(), True),
-      (self.call.adb.Shell("su -c sh -c 'cat /this/file/can.be.read.with.su'"),
-       'this is a test file\nread with su')):
+        "adb -s 0123456789abcdef shell "
+            "'cat \"/this/file/does.not.exist\" 2>/dev/null'",
+         ''):
+      self.device.ReadFile('/this/file/does.not.exist')
+
+  def testReadFile_asRoot_withRoot(self):
+    self.device.old_interface._privileged_command_runner = (
+        self.device.old_interface.RunShellCommand)
+    self.device.old_interface._protected_file_access_method_initialized = True
+    with self.assertCallsSequence([
+        ("adb -s 0123456789abcdef shell "
+            "'cat \"/this/file/must.be.read.by.root\" 2> /dev/null'",
+         'this is a test file\nread by root')]):
+      self.assertEqual(
+          ['this is a test file', 'read by root'],
+          self.device.ReadFile('/this/file/must.be.read.by.root',
+                               as_root=True))
+
+  def testReadFile_asRoot_withSu(self):
+    self.device.old_interface._privileged_command_runner = (
+        self.device.old_interface.RunShellCommandWithSU)
+    self.device.old_interface._protected_file_access_method_initialized = True
+    with self.assertCallsSequence([
+        ("adb -s 0123456789abcdef shell "
+            "'su -c cat \"/this/file/can.be.read.with.su\" 2> /dev/null'",
+         'this is a test file\nread with su')]):
       self.assertEqual(
-          'this is a test file\nread with su\n',
+          ['this is a test file', 'read with su'],
           self.device.ReadFile('/this/file/can.be.read.with.su',
                                as_root=True))
 
+  def testReadFile_asRoot_rejected(self):
+    self.device.old_interface._privileged_command_runner = None
+    self.device.old_interface._protected_file_access_method_initialized = True
+    with self.assertRaises(device_errors.CommandFailedError):
+      self.device.ReadFile('/this/file/cannot.be.read.by.user',
+                           as_root=True)
+
 
 class DeviceUtilsWriteFileTest(DeviceUtilsNewImplTest):
 

build/android/pylib/flag_changer.py

@@ -7,8 +7,6 @@
 import pylib.android_commands
 import pylib.device.device_utils
 
-from pylib.device import device_errors
-
 
 class FlagChanger(object):
   """Changes the flags Chrome runs with.
@@ -34,10 +32,9 @@ def __init__(self, device, cmdline_file):
     self._cmdline_file = cmdline_file
 
     # Save the original flags.
-    try:
-      self._orig_line = self._device.ReadFile(self._cmdline_file).strip()
-    except device_errors.CommandFailedError:
-      self._orig_line = ''
+    self._orig_line = self._device.ReadFile(self._cmdline_file)
+    if self._orig_line:
+      self._orig_line = self._orig_line[0].strip()
 
     # Parse out the flags into a list to facilitate adding and removing flags.
     self._current_flags = self._TokenizeFlags(self._orig_line)
@@ -107,8 +104,8 @@ def _UpdateCommandLineFile(self):
       self._device.WriteFile(
           self._cmdline_file, cmd_line, as_root=use_root)
       file_contents = self._device.ReadFile(
-          self._cmdline_file, as_root=use_root).rstrip()
-      assert file_contents == cmd_line, (
+          self._cmdline_file, as_root=use_root)
+      assert len(file_contents) == 1 and file_contents[0] == cmd_line, (
           'Failed to set the command line file at %s' % self._cmdline_file)
     else:
       self._device.RunShellCommand('rm ' + self._cmdline_file,

build/android/pylib/instrumentation/test_runner.py

@@ -235,8 +235,10 @@ def TearDownPerfMonitoring(self, test):
           '/data/data/com.google.android.apps.chrome/files/PerfTestData.txt',
           as_root=True)
 
-      if not json_string:
-        raise Exception('Perf file is empty')
+      if json_string:
+        json_string = '\n'.join(json_string)
+      else:
+        raise Exception('Perf file does not exist or is empty')
 
       if self.options.save_perf_json:
         json_local_file = '/tmp/chromium-android-perf-json-' + raw_test_name

build/android/pylib/perf/thermal_throttle.py

@@ -34,7 +34,7 @@ def GetThrottlingTemperature(log_line):
 
   def GetCurrentTemperature(self):
     tempdata = self._device.ReadFile(OmapThrottlingDetector.OMAP_TEMP_FILE)
-    return float(tempdata) / 1000.0
+    return float(tempdata[0]) / 1000.0
 
 
 class ExynosThrottlingDetector(object):

build/android/tombstones.py

@@ -63,8 +63,7 @@ def _GetTombstoneData(device, tombstone_file):
   Returns:
     A list of lines
   """
-  return device.ReadFile(
-      '/data/tombstones/' + tombstone_file, as_root=True).splitlines()
+  return device.ReadFile('/data/tombstones/' + tombstone_file, as_root=True)
 
 
 def _EraseTombstone(device, tombstone_file):

tools/telemetry/telemetry/core/forwarders/android_forwarder.py

@@ -231,7 +231,7 @@ def _FindHostRndisInterface(self):
     """Returns the name of the host-side network interface."""
     interface_list = self._EnumerateHostInterfaces()
     ether_address = self._device.ReadFile(
-        '%s/f_rndis/ethaddr' % self._RNDIS_DEVICE).strip()
+        '%s/f_rndis/ethaddr' % self._RNDIS_DEVICE)[0]
     interface_name = None
     for line in interface_list:
       if not line.startswith((' ', '\t')):
@@ -322,7 +322,7 @@ def _EnableRndis(self):
     self._device.RunShellCommand('rm %s.log' % script_prefix)
     self._device.RunShellCommand('. %s.sh' % script_prefix)
     self._WaitForDevice()
-    result = self._device.ReadFile('%s.log' % script_prefix).splitlines()
+    result = self._device.ReadFile('%s.log' % script_prefix)
     assert any('DONE' in line for line in result), 'RNDIS script did not run!'
 
   def _CheckEnableRndis(self, force):

tools/telemetry/telemetry/core/platform/android_platform_backend.py

@@ -321,7 +321,7 @@ def GetFileContents(self, fname):
     if not self._can_access_protected_file_contents:
       logging.warning('%s cannot be retrieved on non-rooted device.' % fname)
       return ''
-    return self._device.ReadFile(fname, as_root=True)
+    return '\n'.join(self._device.ReadFile(fname, as_root=True))
 
   def GetPsOutput(self, columns, pid=None):
     assert columns == ['pid', 'name'] or columns == ['pid'], \

tools/telemetry/telemetry/core/platform/android_platform_backend_unittest.py

@@ -27,12 +27,12 @@ def tearDown(self):
 
   @benchmark.Disabled('chromeos')
   def testGetCpuStats(self):
-    proc_stat_content = (
+    proc_stat_content = [
         '7702 (.android.chrome) S 167 167 0 0 -1 1077936448 '
         '3247 0 0 0 4 1 0 0 20 0 9 0 5603962 337379328 5867 '
         '4294967295 1074458624 1074463824 3197495984 3197494152 '
         '1074767676 0 4612 0 38136 4294967295 0 0 17 0 0 0 0 0 0 '
-        '1074470376 1074470912 1102155776\n')
+        '1074470376 1074470912 1102155776']
     self._stubs.adb_commands.adb_device.mock_content = proc_stat_content
     old_interface = self._stubs.adb_commands.adb_device.old_interface
     old_interface.can_access_protected_file_contents = True

tools/telemetry/telemetry/core/platform/power_monitor/android_temperature_monitor.py

@@ -2,15 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-from telemetry.core import util
-from telemetry.core.platform import power_monitor
-
-util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'build', 'android')
-try:
-  from pylib.device import device_errors  # pylint: disable=F0401
-except ImportError:
-  device_errors = None
-
+import telemetry.core.platform.power_monitor as power_monitor
 
 _TEMPERATURE_FILE = '/sys/class/thermal/thermal_zone0/temp'
 
@@ -65,8 +57,7 @@ def StopMonitoringPower(self):
     return power_data
 
   def _GetBoardTemperatureCelsius(self):
-    try:
-      contents = self._device.ReadFile(_TEMPERATURE_FILE)
-      return float(contents) if contents else None
-    except device_errors.CommandFailedError:
-      return None
+    contents = self._device.ReadFile(_TEMPERATURE_FILE)
+    if len(contents) > 0:
+      return float(contents[0])
+    return None

tools/telemetry/telemetry/core/platform/power_monitor/android_temperature_monitor_unittest.py

@@ -61,7 +61,7 @@ def testSysfsReadFailed(self):
     mock_power_monitor.ExpectCall('CanMonitorPower').WillReturn(False)
     mock_adb = simple_mock.MockObject()
     mock_device_utils = simple_mock.MockObject()
-    mock_device_utils.ExpectCall('ReadFile', _).WillReturn('')
+    mock_device_utils.ExpectCall('ReadFile', _).WillReturn([])
     setattr(mock_device_utils, 'old_interface', mock_adb)
 
     monitor = android_temperature_monitor.AndroidTemperatureMonitor(

tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper_unittest.py

@@ -20,7 +20,7 @@ def _GetLibrariesMappedIntoProcesses(device, pids):
   libs = set()
   for pid in pids:
     maps_file = '/proc/%d/maps' % pid
-    maps = device.ReadFile(maps_file, as_root=True).splitlines()
+    maps = device.ReadFile(maps_file, as_root=True)
     for map_line in maps:
       lib = re.match(r'.*\s(/.*[.]so)$', map_line)
       if lib: