GN: Android perf/tests

infra/bots/recipe_modules/flavor/gn_android_flavor.py

Index: infra/bots/recipe_modules/flavor/gn_android_flavor.py
diff --git a/infra/bots/recipe_modules/flavor/gn_android_flavor.py b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
index 4cbaab38a6e10cfc6b1e1a89d863749a6db5d0e1..fd935307dda09084ce38e402f7d7928933ff9ec2 100644
--- a/infra/bots/recipe_modules/flavor/gn_android_flavor.py
+++ b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
@@ -3,16 +3,39 @@
 # found in the LICENSE file.
 
 import default_flavor
+import subprocess
 
 """GN Android flavor utils, used for building Skia for Android with GN."""
 class GNAndroidFlavorUtils(default_flavor.DefaultFlavorUtils):
+  def __init__(self, m):
+    super(GNAndroidFlavorUtils, self).__init__(m)
+    self._ever_ran_adb = False
+
+    prefix = '/data/local/tmp/'
+    self.device_dirs = default_flavor.DeviceDirs(
+        dm_dir        = prefix + 'dm_out',
+        perf_data_dir = prefix + 'perf',
+        resource_dir  = prefix + 'resources',
+        images_dir    = prefix + 'images',
+        skp_dir       = prefix + 'skps',
+        svg_dir       = prefix + 'svgs',
+        tmp_dir       = prefix + 'tmp')
+
   def supported(self):
     return 'GN_Android' == self.m.vars.builder_cfg.get('extra_config', '')
 
-  def _run(self, title, cmd):
-    path = self.m.vars.default_env['PATH']
-    self.m.vars.default_env = {'PATH': path}
-    self.m.run(self.m.step, title, cmd=cmd, cwd=self.m.vars.skia_dir, env={})
+  def _run(self, *cmd, **kwargs):
+    cmd = map(str, cmd)
+    title = subprocess.list2cmdline(cmd)

[borenet, 2016/09/12 13:10:12]

This generates bad step names, eg. containing symbols that buildbot/etc don't
like.  I recommend just using the first part of the command.

[mtklein, 2016/09/12 14:12:12]

Huh.  What error did you see?

But, done... updated everything to use an explicit name.

[borenet, 2016/09/12 14:32:00]

No error here, but the master refuses to create log links for any steps with
"unusual" symbols in the name.

+    self.m.vars.default_env = {k: v for (k,v)
+                               in self.m.vars.default_env.iteritems()
+                               if k in ['PATH']}

[borenet, 2016/09/12 13:10:12]

Why do we need to overwrite the default_env here?  IMO we should avoid modifying
anything in self.m.vars.

[mtklein, 2016/09/12 14:12:12]

Same deal as in gn_flavor.py and this file previously.  Everything in the
environment is there for GYP or GYP-built binaries, so it's confusing to see
that environment in a GN compile or run.  It's mostly harmless, but by removing
it from the environment we enforce that it's harmless.

I too would like to be able to do this without modifying self.m.vars, but it's
injected into the environment outside my control here.  Can we rework that so
that it's threaded through as env={...} from the outset and then I just sanitize
my own copy?

[borenet, 2016/09/12 14:32:00]

This is fine for now, but the assumption for default_env should be that it
applies for all steps, and this may cause unexpected behavior if something
important and non-GYP-related were added.

+    return self.m.run(self.m.step, title, cmd=cmd,
+                      cwd=self.m.vars.skia_dir, env={}, **kwargs)
+
+  def _adb(self, *cmd, **kwargs):
+    self._ever_ran_adb = True
+    return self._run('adb', *cmd, **kwargs)
 
   def compile(self, unused_target, **kwargs):
     compiler      = self.m.vars.builder_cfg.get('compiler')
@@ -31,6 +54,75 @@ class GNAndroidFlavorUtils(default_flavor.DefaultFlavorUtils):
         'target_cpu': quote(target_arch),
     }.iteritems()))
 
-    self._run('fetch-gn', [self.m.vars.skia_dir.join('bin', 'fetch-gn')])
-    self._run('gn gen', ['gn', 'gen', self.out_dir, '--args=' + gn_args])
-    self._run('ninja', ['ninja', '-C', self.out_dir])
+    self._run(self.m.vars.skia_dir.join('bin', 'fetch-gn'))
+    self._run('gn', 'gen', self.out_dir, '--args=' + gn_args)
+    self._run('ninja', '-C', self.out_dir)
+
+  def install(self):
+    self._adb('reboot')
+    self._adb('wait-for-usb-device')
+    self._adb('shell', 'rm', '-rf', '/data/local/tmp/*')    # TEMPORARY

[borenet, 2016/09/12 13:10:12]

Do we really need this?  It's going to require us to re-copy all of the SKPs,
SkImages, etc which is very slow.

[mtklein, 2016/09/12 14:12:12]

Not sure, but we definitely don't need it in the long term.

The file copying doesn't really take too long compared to the run times of these
bots.  You have to have really good timing to catch the resources or skps
copying, but yeah, the images can take a couple minutes.  

Let's land this as-is then follow up with a separate CL to re-enable the
caching?  That way it's independently revertible.

[borenet, 2016/09/12 14:32:00]

SGTM

+    self._adb('shell', 'mkdir', '-p', '/data/local/tmp/resources')
+
+  def cleanup_steps(self):
+    if self._ever_ran_adb:
+      self._adb('shell', 'rm', '-rf', '/data/local/tmp/*')  # TEMPORARY
+      self._adb('reboot')                                   # TEMPORARY
+      self._adb('kill-server')
+
+  def step(self, name, cmd, env=None, **kwargs):
+    app = self.m.vars.skia_out.join(self.m.vars.configuration, cmd[0])
+    self._adb('push', app, '/data/local/tmp')
+
+    sh = '%s.sh' % cmd[0]
+    self.m.run.writefile(self.m.vars.tmp_dir.join(sh),
+        'set -x; /data/local/tmp/%s; echo $? >/data/local/tmp/rc' %
+        subprocess.list2cmdline(map(str, cmd)))
+    self._adb('push', self.m.vars.tmp_dir.join(sh), '/data/local/tmp')
+
+    self._adb('logcat', '-c')
+    self._adb('shell', 'sh', '/data/local/tmp/' + sh)
+    self._adb('logcat', '-d')
+
+    self.m.python.inline('check %s rc' % sh, """
+    import subprocess
+    import sys
+    sys.exit(int(subprocess.check_output(['adb', 'shell', 'cat',
+                                          '/data/local/tmp/rc'])))
+    """)
+
+  def copy_file_to_device(self, host, device):
+    self._adb('push', host, device)
+
+  def copy_directory_contents_to_device(self, host, device):
+    # Copy the tree, avoiding hidden directories and resolving symlinks.
+    self.m.python.inline('push %s/* %s' % (host, device), """
+    import os
+    import subprocess
+    import sys
+    host   = sys.argv[1]
+    device = sys.argv[2]
+    for d, _, fs in os.walk(host):
+      p = os.path.relpath(d, host)
+      if p != '.' and p.startswith('.'):
+        continue
+      for f in fs:
+        print os.path.join(p,f)
+        subprocess.check_call(['adb', 'push',
+                               os.path.realpath(os.path.join(host, p, f)),
+                               os.path.join(device, p, f)])
+    """, args=[host, device], cwd=self.m.vars.skia_dir)
+
+  def copy_directory_contents_to_host(self, device, host):
+    self._adb('pull', device, host)
+
+  def read_file_on_device(self, path):
+    return self._adb('shell', 'cat', path,
+                     stdout=self.m.raw_io.output()).stdout
+
+  def remove_file_on_device(self, path):
+    self._adb('shell', 'rm', '-f', path)
+
+  def create_clean_device_dir(self, path):
+    self._adb('shell', 'rm', '-rf', path)
+    self._adb('shell', 'mkdir', '-p', path)