[Android] Switch to DeviceUtils versions of GetPid, TakeScreenshot, and GetIoStats.

build/android/pylib/device/device_utils.py

 # Copyright 2014 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.
 
 """Provides a variety of device interactions based on adb.
 
 Eventually, this will be based on adb_wrapper.
 """
 # pylint: disable=W0613
 
@@ skipping 582 matching lines @@
     Args:
       property_name: A string containing the name of the property to set on
                      the device.
       value: A string containing the value to set to the property on the
              device.
       timeout: Same as for |IsOnline|.
       retries: Same as for |IsOnline|.
     """
     self.old_interface.system_properties[property_name] = value
 
+  @decorators.WithTimeoutAndRetriesFromInstance()
+  def GetPid(self, process_name, timeout=None, retries=None):

[frankf, 2014/07/11 20:22:53]

this should be plural to match the return value

[jbudorick, 2014/07/14 16:15:11]

Done.

+    """Returns the PIDs of processes with the given name.
+
+    Note that the |process_name| is often the package name.

[frankf, 2014/07/11 20:29:07]

Also isn't this the app package name and not the process name?

[jbudorick, 2014/07/14 16:15:11]

Not necessarily, though it can be, e.g. these can both appear in the name field
of a ps line:

  /data/local/tmp/forwarder/device_forwarder
  com.google.android.apps.chrome

+
+    Args:
+      process_name: A string containing the process name to get the PIDs for.
+      timeout: Same as for |IsOnline|.

[frankf, 2014/07/11 20:22:53]

I'd change all these to:

timeout: timeout in <some unit>
retries: number of retries

same amount of duplication plus it's self-contained

[jbudorick, 2014/07/14 16:15:11]

I don't really have an objection to this, but...

https://codereview.chromium.org/285143002/#msg14

[frankf, 2014/07/14 17:04:10]

Yea, the spirit of that comment was to cut down on boilerplate, but you still
have boiler plate so you might as well have the actual arg description. If
someone is using generated pydoc, then it's easier for them.

On 2014/07/14 16:15:11, jbudorick wrote:

[jbudorick, 2014/07/15 15:52:56]

Makes sense.

+      retries: Same as for |IsOnline|.
+    Returns:
+      A list containing the PIDs of processes with the given name.

[frankf, 2014/07/11 20:22:53]

are there any assumptions on the ordering here? For example for Chrome, does it
list the browser process first?

[jbudorick, 2014/07/14 16:15:11]

I don't think that the interface should guarantee ordering, but I'm open to
arguments to the contrary.

Currently, AndroidCommands puts exact matches at the front of the list. Other
matches are added in the order in which they appear in the output of ps, which
appears to be in numerical order by PID.

As for browser vs renderer processes, no, there are definitely no guarantees
there as far as the scripts are concerned.

[frankf, 2014/07/14 17:04:10]

For example, in monkey test runner, we take the first pid as the browser
process, and there might be other instances. So either provide the guarantee or
come up with a mechanism to get just the browser process and not renderers.

On 2014/07/14 16:15:11, jbudorick wrote:

[jbudorick, 2014/07/15 15:52:56]

Ah, ok.

Switched to having GetPids return a dict mapping process name to PID.

+    """

[frankf, 2014/07/11 20:22:53]

you should add raises section to all methods

[jbudorick, 2014/07/14 16:15:11]

Done.

+    return self.old_interface.ExtractPid(process_name)
+
+  @decorators.WithTimeoutAndRetriesFromInstance()
+  def TakeScreenshot(self, host_path=None, timeout=None, retries=None):
+    """Takes a screenshot of the device.
+
+    Args:
+      host_path: A string containing the path on the host to save the
+                 screenshot to. If None, a file name will be generated.

[frankf, 2014/07/11 20:22:53]

is this a temp file?

[jbudorick, 2014/07/14 16:15:11]

No.

The AndroidCommands implementation uses ${PWD}/screenshot-${timestamp}.png when
it generates a file name. That is not guaranteed to stay the same in the impl
change, but it won't be a temp file.

[frankf, 2014/07/14 17:04:10]

So my point was: is the user expected to clean this up? I guess in general it's
good to delete the file even if it's in /tmp...

On 2014/07/14 16:15:11, jbudorick wrote:

[jbudorick, 2014/07/15 15:52:56]

Yes, the user is expected to clean up the screenshot file on the host if they
don't want it (although if they don't want the screenshot file, there's really
no point in calling this function).

+      timeout: Same as for |IsOnline|.
+      retries: Same as for |IsOnline|.
+    Returns:
+      The name of the file on the host to which the screenshot was saved.
+    """
+    return self.old_interface.TakeScreenshot(host_path)
+
+  @decorators.WithTimeoutAndRetriesFromInstance()
+  def GetIOStats(self, timeout=None, retries=None):
+    """Gets cumulative disk IO stats since boot for all processes.
+
+    Args:
+      timeout: Same as for |IsOnline|.
+      retries: Same as for |IsOnline|.
+    Returns:
+      A dict containing |num_reads|, |num_writes|, |read_ms|, and |write_ms|.
+    """
+    return self.old_interface.GetIoStats()
+
   def __str__(self):
     """Returns the device serial."""
     return self.old_interface.GetDevice()
 
   @staticmethod
   def parallel(devices=None, async=False):
     """Creates a Parallelizer to operate over the provided list of devices.
 
     If |devices| is either |None| or an empty list, the Parallelizer will
     operate over all attached devices.
 
     Args:
       devices: A list of either DeviceUtils instances or objects from
                from which DeviceUtils instances can be constructed. If None,
                all attached devices will be used.
       async: If true, returns a Parallelizer that runs operations
              asynchronously.
     Returns:
       A Parallelizer operating over |devices|.
     """
     if not devices or len(devices) == 0:
       devices = pylib.android_commands.GetAttachedDevices()
     parallelizer_type = (parallelizer.Parallelizer if async
                          else parallelizer.SyncParallelizer)
     return parallelizer_type([
         d if isinstance(d, DeviceUtils) else DeviceUtils(d)
         for d in devices])