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

build/android/pylib/device/device_utils.py

Index: build/android/pylib/device/device_utils.py
diff --git a/build/android/pylib/device/device_utils.py b/build/android/pylib/device/device_utils.py
index 47884725740151fb9b0acd1bf76f61fb6850d9de..642f89b04fad4d071fbc64f4faa678e12b0973c1 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -600,6 +600,47 @@ class DeviceUtils(object):
     """
     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()