Revert "Create a top level ui screenshot description"

This reverts commit 5050d462.

Reason for revert: Failing some bots on L and M (Cannot create shot files java.io.IOException: open failed: EACCES (Permission denied)).

Original change's description:
> Create a top level ui screenshot description
> 
> Merge the screenshot descriptions produced by the instrumentation tests
> into a single description. If the tests are run locally print a link
> to this description, if the are run remotely add a link to the
> description to the json file produced by test_results_presentation
> 
> Change-Id: Ia9dd491460633148283bed66949cec47a410dea3
> Reviewed-on: https://chromium-review.googlesource.com/952972
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Commit-Queue: Anthony Berent <aberent@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543393}

TBR=bauerb@chromium.org,aberent@chromium.org,jbudorick@chromium.org

Change-Id: Iafb4846c02ba54fcbfe5e4c5391bd1e5fbec540e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/964931Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543499}

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

@@ -39,7 +39,6 @@ from py_utils import contextlib_ext
 from py_utils import tempfile_ext
 import tombstones
 
-
 with host_paths.SysPath(
     os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 0):
   import jinja2  # pylint: disable=import-error
@@ -76,6 +75,8 @@ EXTRA_TRACE_FILE = ('org.chromium.base.test.BaseJUnit4ClassRunner.TraceFile')
 _EXTRA_TEST_LIST = (
     'org.chromium.base.test.BaseChromiumAndroidJUnitRunner.TestList')
 
+UI_CAPTURE_DIRS = ['chromium_tests_root', 'UiCapture']
+
 FEATURE_ANNOTATION = 'Feature'
 RENDER_TEST_FEATURE_ANNOTATION = 'RenderTest'
 
@@ -125,6 +126,7 @@ class LocalDeviceInstrumentationTestRun(
     super(LocalDeviceInstrumentationTestRun, self).__init__(
         env, test_instance)
     self._flag_changers = {}
+    self._ui_capture_dir = dict()
     self._replace_package_contextmanager = None
 
   #override
@@ -251,8 +253,21 @@ class LocalDeviceInstrumentationTestRun(
         valgrind_tools.SetChromeTimeoutScale(
             dev, self._test_instance.timeout_scale)
 
+      @trace_event.traced
+      def setup_ui_capture_dir(dev):
+        # Make sure the UI capture directory exists and is empty by deleting
+        # and recreating it.
+        # TODO (aberent) once DeviceTempDir exists use it here.
+        self._ui_capture_dir[dev] = posixpath.join(
+            dev.GetExternalStoragePath(),
+            *UI_CAPTURE_DIRS)
+
+        if dev.PathExists(self._ui_capture_dir[dev]):
+          dev.RunShellCommand(['rm', '-rf', self._ui_capture_dir[dev]])
+        dev.RunShellCommand(['mkdir', self._ui_capture_dir[dev]])
+
       steps += [set_debug_app, edit_shared_prefs, push_test_data,
-                create_flag_changer]
+                create_flag_changer, setup_ui_capture_dir]
 
       def bind_crash_handler(step, dev):
         return lambda: crash_handler.RetryOnSystemCrash(step, dev)
@@ -357,11 +372,7 @@ class LocalDeviceInstrumentationTestRun(
         device.adb, suffix='.png', dir=device.GetExternalStoragePath())
     extras[EXTRA_SCREENSHOT_FILE] = screenshot_device_file.name
 
-    # Set up the screenshot directory. This needs to be done for each test so
-    # that we only get screenshots created by that test.
-    ui_capture_dir = device_temp_file.NamedDeviceTemporaryDirectory(
-        device.adb)
-    extras[EXTRA_UI_CAPTURE_DIR] = ui_capture_dir.name
+    extras[EXTRA_UI_CAPTURE_DIR] = self._ui_capture_dir[device]
 
     if self._env.trace_output:
       trace_device_file = device_temp_file.DeviceTempFile(
@@ -439,7 +450,6 @@ class LocalDeviceInstrumentationTestRun(
         time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
         device.serial)
 
-    with ui_capture_dir:
     with self._env.output_manager.ArchivedTempfile(
         stream_name, 'logcat') as logcat_file:
       try:
@@ -447,8 +457,7 @@ class LocalDeviceInstrumentationTestRun(
             device.adb,
             filter_specs=local_device_environment.LOGCAT_FILTERS,
             output_file=logcat_file.name,
-              transform_func=self._test_instance.MaybeDeobfuscateLines
-              ) as logmon:
+            transform_func=self._test_instance.MaybeDeobfuscateLines) as logmon:
           with _LogTestEndpoints(device, test_name):
             with contextlib_ext.Optional(
                 trace_event.trace(test_name),
@@ -497,9 +506,9 @@ class LocalDeviceInstrumentationTestRun(
 
     def handle_render_test_data():
       if _IsRenderTest(test):
-          # Render tests do not cause test failure by default. So we have to
-          # check to see if any failure images were generated even if the test
-          # does not fail.
+        # Render tests do not cause test failure by default. So we have to check
+        # to see if any failure images were generated even if the test does not
+        # fail.
         try:
           self._ProcessRenderTestResults(
               device, render_tests_device_output_dir, results)
@@ -509,7 +518,7 @@ class LocalDeviceInstrumentationTestRun(
 
     def pull_ui_screen_captures():
       screenshots = []
-        for filename in device.ListDirectory(ui_capture_dir.name):
+      for filename in device.ListDirectory(self._ui_capture_dir[device]):
         if filename.endswith('.json'):
           screenshots.append(pull_ui_screenshot(filename))
       if screenshots:
@@ -524,7 +533,7 @@ class LocalDeviceInstrumentationTestRun(
           result.SetLink('ui screenshot', json_archive.Link())
 
     def pull_ui_screenshot(filename):
-        source_dir = ui_capture_dir.name
+      source_dir = self._ui_capture_dir[device]
       json_path = posixpath.join(source_dir, filename)
       json_data = json.loads(device.ReadFile(json_path))
       image_file_path = posixpath.join(source_dir, json_data['location'])

build/android/pylib/results/presentation/test_results_presentation.py

@@ -6,9 +6,7 @@
 
 import argparse
 import collections
-import contextlib
 import json
-import logging
 import tempfile
 import os
 import sys
@@ -363,56 +361,6 @@ def upload_to_google_bucket(html, bucket, dest):
         authenticated_link=True)
 
 
-def ui_screenshot_set(json_path):
-  with open(json_path) as json_file:
-    json_object = json.loads(json_file.read())
-  if not 'per_iteration_data' in json_object:
-    # This will be reported as an error by result_details, no need to duplicate.
-    return None
-  ui_screenshots = []
-  for testsuite_run in json_object['per_iteration_data']:
-    for _, test_runs in testsuite_run.iteritems():
-      for test_run in test_runs:
-        if 'ui screenshot' in test_run['links']:
-          screenshot_link = test_run['links']['ui screenshot']
-          if screenshot_link.startswith('file:'):
-            with contextlib.closing(urllib.urlopen(screenshot_link)) as f:
-              test_screenshots = json.load(f)
-          else:
-            # Assume anything that isn't a file link is a google storage link
-            screenshot_string = google_storage_helper.read_from_link(
-                screenshot_link)
-            if not screenshot_string:
-              logging.error('Bad screenshot link %s', screenshot_link)
-              continue
-            test_screenshots = json.loads(
-                screenshot_string)
-          ui_screenshots.extend(test_screenshots)
-
-  if ui_screenshots:
-    return json.dumps(ui_screenshots)
-  return None
-
-
-def upload_screenshot_set(json_path, test_name, bucket, builder_name,
-                          build_number):
-  screenshot_set = ui_screenshot_set(json_path)
-  if not screenshot_set:
-    return None
-  dest = google_storage_helper.unique_name(
-    'screenshots_%s_%s_%s' % (test_name, builder_name, build_number),
-    suffix='.json')
-  with tempfile.NamedTemporaryFile(suffix='.json') as temp_file:
-    temp_file.write(screenshot_set)
-    temp_file.flush()
-    return google_storage_helper.upload(
-        name=dest,
-        filepath=temp_file.name,
-        bucket='%s/json' % bucket,
-        content_type='application/json',
-        authenticated_link=True)
-
-
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--json-file', help='Path of json file.')
@@ -507,28 +455,16 @@ def main():
       'Result details link do not match. The link returned by get_url_link'
       ' should be the same as that returned by upload.')
 
-  ui_screenshot_link = upload_screenshot_set(json_file, args.test_name,
-      args.bucket, builder_name, build_number)
-
-
   if args.output_json:
     with open(json_file) as original_json_file:
       json_object = json.load(original_json_file)
       json_object['links'] = {
           'result_details (logcats, flakiness links)': result_details_link
       }
-
-      if ui_screenshot_link:
-        json_object['links']['ui screenshots'] = ui_screenshot_link
-
       with open(args.output_json, 'w') as f:
         json.dump(json_object, f)
   else:
-    print 'Result Details: %s' % result_details_link
-
-    if ui_screenshot_link:
-      print 'UI Screenshots %s' % ui_screenshot_link
-
+    print result_details_link
 
 if __name__ == '__main__':
   sys.exit(main())

build/android/pylib/utils/google_storage_helper.py

@@ -13,7 +13,6 @@ import logging
 import os
 import sys
 import time
-import urlparse
 
 from pylib.constants import host_paths
 from pylib.utils import decorators
@@ -63,15 +62,6 @@ def upload(name, filepath, bucket, gs_args=None, command_args=None,
   return get_url_link(name, bucket, authenticated_link)
 
 
-@decorators.NoRaiseException(default_return_value='')
-def read_from_link(link):
-  # Note that urlparse returns the path with an initial '/', so we only need to
-  # add one more after the 'gs;'
-  gs_path = 'gs:/%s' % urlparse.urlparse(link).path
-  cmd = [_GSUTIL_PATH, '-q', 'cat', gs_path]
-  return cmd_helper.GetCmdOutput(cmd)
-
-
 @decorators.NoRaiseException(default_return_value=False)
 def exists(name, bucket):
   bucket = _format_bucket_name(bucket)

build/android/test_runner.py

@@ -937,16 +937,6 @@ def RunTestsInPlatformMode(args):
         results_detail_file.flush()
       logging.critical('TEST RESULTS: %s', results_detail_file.Link())
 
-      ui_screenshots = test_results_presentation.ui_screenshot_set(
-          json_file.name)
-      if ui_screenshots:
-        with out_manager.ArchivedTempfile(
-            'ui_screenshots.json',
-            'ui_capture',
-            output_manager.Datatype.JSON) as ui_screenshot_file:
-          ui_screenshot_file.write(ui_screenshots)
-        logging.critical('UI Screenshots: %s', ui_screenshot_file.Link())
-
   if args.command == 'perf' and (args.steps or args.single_step):
     return 0
 

chrome/android/javatests/src/org/chromium/chrome/browser/test/ScreenShooter.java

@@ -10,7 +10,6 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import android.annotation.SuppressLint;
 import android.app.Instrumentation;
 import android.content.res.Configuration;
 import android.graphics.Point;
@@ -94,7 +93,6 @@ import java.util.Map;
  * }
  * </pre>
  */
-@SuppressLint("SetWorldReadable")
 public class ScreenShooter extends TestWatcher {
     private static final String SCREENSHOT_DIR =
             "org.chromium.base.test.util.Screenshooter.ScreenshotDir";
@@ -206,9 +204,6 @@ public class ScreenShooter extends TestWatcher {
             File shotFile = File.createTempFile(shotName, IMAGE_SUFFIX, new File(mBaseDir));
             assertTrue("Screenshot " + shotName, mDevice.takeScreenshot(shotFile));
             writeImageDescription(shotFile, filters, tags, metadata);
-            // Set as world readable so that the test runner can read it from /data/local/tmp
-            // without having to run as root
-            shotFile.setReadable(true, false);
         } catch (IOException e) {
             fail("Cannot create shot files " + e.toString());
         }
@@ -236,12 +231,8 @@ public class ScreenShooter extends TestWatcher {
         String jsonFileName =
                 shotFileName.substring(0, shotFileName.length() - IMAGE_SUFFIX.length())
                 + JSON_SUFFIX;
-        File descriptionFile = new File(mBaseDir, jsonFileName);
-        try (FileWriter fileWriter = new FileWriter(descriptionFile)) {
+        try (FileWriter fileWriter = new FileWriter(new File(mBaseDir, jsonFileName));) {
             fileWriter.write(imageDescription.toString());
         }
-        // Set as world readable so that the test runner can read it from /data/local/tmp without
-        // having to run as root
-        descriptionFile.setReadable(true, false);
     }
 }