Move screenshot capture to Java-side.

build/android/pylib/local/device/local_device_instrumentation_test_run.py

Index: build/android/pylib/local/device/local_device_instrumentation_test_run.py
diff --git a/build/android/pylib/local/device/local_device_instrumentation_test_run.py b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
index c95fe6d6e9bf81d10465469fd425d253e45a56a7..fc454208ad19f440973104408338dbb1750b8857 100644
--- a/build/android/pylib/local/device/local_device_instrumentation_test_run.py
+++ b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
@@ -386,22 +386,33 @@ class LocalDeviceInstrumentationTestRun(
               self._test_instance.gs_results_bucket) as screenshot_host_dir:
         screenshot_host_dir = (
             self._test_instance.screenshot_dir or screenshot_host_dir)
+
         if screenshot_host_dir:
-          file_name = '%s-%s.png' % (
-              test_display_name,
-              time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()))
-          screenshot_file = device.TakeScreenshot(
-              os.path.join(screenshot_host_dir, file_name))
-          logging.info(
-              'Saved screenshot for %s to %s.',
-              test_display_name, screenshot_file)
-          if self._test_instance.gs_results_bucket:
-            link = google_storage_helper.upload(
-                google_storage_helper.unique_name('screenshot', device=device),
-                screenshot_file,
-                bucket=self._test_instance.gs_results_bucket + '/screenshots')
-            for result in results:
-              result.SetLink('post_test_screenshot', link)
+          screenshot_device_file = posixpath.join(
+              device.GetExternalStoragePath(),
+              'chromium_tests_root',

[jbudorick, 2017/05/05 01:21:58]

If I don't like hard-coding things, I *really* don't like hard-coding things in
multiple places.

This depends on the implementation of getIsolatedTestFilePath referring to
*exactly* <external storage>/chromium_tests_root *and* on
ScreenshotOnFailureStatement concatenating the provided --screenshot-dir to that
path. Can we reduce the number of places in which this is hard-coded? Maybe we
can have the script tell the Statement exactly where it should write the
screenshot?

[mikecase (-- gone --), 2017/05/05 18:09:19]

Changed --screenshot-dir arg to just --screenshot-path so fewer things have to
be hard-coded.

Currently, other things depend on getIsolatedTestFilePath referring to exactly
<external storage>/chromium_tests_root....at least I think so. Like, we push the
data to the device to chromium_tests_root, and then use UrlUtils to find the
test data.

+              instrumentation_test_instance.FAILURE_SCREENSHOT_SUBDIR,
+              'failure.png')
+          screenshot_host_file = os.path.join(
+              screenshot_host_dir,
+              '%s-%s.png' % (
+                  test_display_name,
+                  time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime())))
+          if device.FileExists(screenshot_device_file):
+            device.PullFile(screenshot_device_file, screenshot_host_file)
+            device.RemovePath(screenshot_device_file)

[jbudorick, 2017/05/05 01:21:58]

We should try to remove the file even if pulling it fails for some reason. Maybe
a try/finally here?

[mikecase (-- gone --), 2017/05/05 18:09:19]

sounds good. Done

+
+            logging.info(
+                'Saved screenshot for %s to %s.',
+                test_display_name, screenshot_host_file)
+            if self._test_instance.gs_results_bucket:
+              link = google_storage_helper.upload(
+                  google_storage_helper.unique_name(
+                      'screenshot', device=device),
+                  screenshot_host_file,
+                  bucket=self._test_instance.gs_results_bucket + '/screenshots')

[jbudorick, 2017/05/05 01:21:58]

nit: '%s/screenshots' % self._test_instance.gs_results_bucket

+              for result in results:
+                result.SetLink('post_test_screenshot', link)
 
       logging.info('detected failure in %s. raw output:', test_display_name)
       for l in output: