Reland of Migrate DeviceUtils.ReadFile to adb_wrapper (try 3)

Original description:

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).

Previous CLs:
- https://codereview.chromium.org/794583004/
- https://codereview.chromium.org/775333002/

BUG=267773

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

Cr-Original-Commit-Position: refs/heads/master@{#313055}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7138148b7108a41bf44a39e8aa52e5954d2c0f52

android/pylib/device/device_utils.py

@@ -91,6 +91,13 @@ def _GetTimeStamp():
   return time.strftime('%Y%m%dT%H%M%S', time.localtime())
 
 
+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_]*$')
@@ -883,7 +890,8 @@ class DeviceUtils(object):
     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:
@@ -895,22 +903,17 @@ class DeviceUtils(object):
       retries: number of retries
 
     Returns:
-      The contents of the file at |device_path| as a list of lines.
+      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.
 
     Raises:
-      CommandFailedError if the file can't be read.
+      AdbCommandFailedError if the file can't be read.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    # 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)
+    return _JoinLines(self.RunShellCommand(
+        ['cat', device_path], as_root=as_root, check_return=True))
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def WriteFile(self, device_path, contents, as_root=False, force_push=False,

android/pylib/device/device_utils_test.py

@@ -1065,56 +1065,33 @@ class DeviceUtilsPullFileTest(DeviceUtilsNewImplTest):
                                '/test/file/host/path')
 
 
-class DeviceUtilsReadFileTest(DeviceUtilsOldImplTest):
+class DeviceUtilsReadFileTest(DeviceUtilsNewImplTest):
 
   def testReadFile_exists(self):
-    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'],
+    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',
                        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(
-        "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.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')):
       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', 'read with su'],
+          'this is a test file\nread with su\n',
           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):
 

android/pylib/flag_changer.py

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

android/pylib/instrumentation/test_runner.py

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

android/pylib/perf/thermal_throttle.py

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

android/tombstones.py

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