Fix instrumentation/gtest tests on R+

Fixes instrumentation and gtest-based tests failing on Android R and
above due to more restrictive storage permissions. This is achieved
by applying the legacy storage permissions using app compatibility
flags.

Bug: 1173699

Change-Id: I351894dc4dc9ef1542d720c3cf4d367a73eb5a84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2693225
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#855384}

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

@@ -297,8 +297,11 @@ def Run(self, test, device, flags=None, **kwargs):
         extras[_EXTRA_TEST] = test[0]
     # pylint: enable=redefined-variable-type
 
+    # We need to use GetAppWritablePath here instead of GetExternalStoragePath
+    # since we will not have yet applied legacy storage permission workarounds
+    # on R+.
     stdout_file = device_temp_file.DeviceTempFile(
-        device.adb, dir=device.GetExternalStoragePath(), suffix='.gtest_out')
+        device.adb, dir=device.GetAppWritablePath(), suffix='.gtest_out')
     extras[_EXTRA_STDOUT_FILE] = stdout_file.name
 
     if self._wait_for_java_debugger:
@@ -442,6 +445,11 @@ def __init__(self, env, test_instance):
     assert isinstance(test_instance, gtest_test_instance.GtestTestInstance)
     super(LocalDeviceGtestRun, self).__init__(env, test_instance)
 
+    if self._test_instance.apk_helper:
+      self._installed_packages = [
+          self._test_instance.apk_helper.GetPackageName()
+      ]
+
     # pylint: disable=redefined-variable-type
     if self._test_instance.apk:
       self._delegate = _ApkDelegate(self._test_instance, env.tool)

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

@@ -17,6 +17,7 @@
 import time
 
 from devil import base_error
+from devil.android import apk_helper
 from devil.android import crash_handler
 from devil.android import device_errors
 from devil.android import device_temp_file
@@ -271,10 +272,18 @@ def incremental_install_helper_internal(d, apk_path=None):
       steps.extend(
           install_helper(apk) for apk in self._test_instance.additional_apks)
 
+      # We'll potentially need the package names later for setting app
+      # compatibility workarounds.
+      for apk in (self._test_instance.additional_apks +
+                  [self._test_instance.test_apk]):
+        self._installed_packages.append(apk_helper.GetPackageName(apk))
+
       # The apk under test needs to be installed last since installing other
       # apks after will unintentionally clear the fake module directory.
       # TODO(wnwen): Make this more robust, fix crbug.com/1010954.
       if self._test_instance.apk_under_test:
+        self._installed_packages.append(
+            apk_helper.GetPackageName(self._test_instance.apk_under_test))
         permissions = self._test_instance.apk_under_test.GetPermissions()
         if self._test_instance.apk_under_test_incremental_install_json:
           steps.append(
@@ -863,9 +872,12 @@ def _GetTestsFromRunner(self):
                  self._test_instance.junit4_runner_class)
     def list_tests(d):
       def _run(dev):
+        # We need to use GetAppWritablePath instead of GetExternalStoragePath
+        # here because we will not have applied legacy storage workarounds on R+
+        # yet.
         with device_temp_file.DeviceTempFile(
             dev.adb, suffix='.json',
-            dir=dev.GetExternalStoragePath()) as dev_test_list_json:
+            dir=dev.GetAppWritablePath()) as dev_test_list_json:
           junit4_runner_class = self._test_instance.junit4_runner_class
           test_package = self._test_instance.test_package
           extras = {

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

@@ -52,6 +52,8 @@ class LocalDeviceTestRun(test_run.TestRun):
   def __init__(self, env, test_instance):
     super(LocalDeviceTestRun, self).__init__(env, test_instance)
     self._tools = {}
+    # This is intended to be filled by a child class.
+    self._installed_packages = []
     env.SetPreferredAbis(test_instance.GetPreferredAbis())
 
   #override
@@ -62,6 +64,10 @@ def RunTests(self, results):
 
     @local_device_environment.handle_shard_failures
     def run_tests_on_device(dev, tests, results):
+      # This is performed here instead of during setup because restarting the
+      # device clears app compatibility flags, which will happen if a device
+      # needs to be recovered.
+      SetAppCompatibilityFlagsIfNecessary(self._installed_packages, dev)
       consecutive_device_errors = 0
       for test in tests:
         if exit_now.isSet():
@@ -282,5 +288,27 @@ def _ShouldShard(self):
     raise NotImplementedError
 
 
+def SetAppCompatibilityFlagsIfNecessary(packages, device):
+  """Sets app compatibility flags on the given packages and device.
+
+  Args:
+    packages: A list of strings containing package names to apply flags to.
+    device: A DeviceUtils instance to apply the flags on.
+  """
+
+  def set_flag_for_packages(flag, enable):
+    enable_str = 'enable' if enable else 'disable'
+    for p in packages:
+      cmd = ['am', 'compat', enable_str, flag, p]
+      device.RunShellCommand(cmd)
+
+  sdk_version = device.build_version_sdk
+  if sdk_version >= version_codes.R:
+    # These flags are necessary to use the legacy storage permissions on R+.
+    # See crbug.com/1173699 for more information.
+    set_flag_for_packages('DEFAULT_SCOPED_STORAGE', False)
+    set_flag_for_packages('FORCE_ENABLE_SCOPED_STORAGE', False)
+
+
 class NoTestsError(Exception):
   """Error for when no tests are found."""

net/test/android/javatests/AndroidManifest.xml

@@ -15,7 +15,16 @@
 
         <uses-library android:name="android.test.runner" />
 
+        <!-- The android:requestLegacyExternalStorage setting is necessary to
+             work on Q+. If the target SDK version of this APK is ever updated
+             to 30 or higher, that workaround will stop working and the more
+             complicated one used by the test APKs (using app compatibility
+             changes) will need to be used instead. Note that this setting is
+             usually applied at the application level, but those do not
+             propagate to child services and the service is the thing that
+             actually reads from external storage. -->
         <service android:name="org.chromium.net.test.EmbeddedTestServerService"
+                android:requestLegacyExternalStorage="true"
                 android:exported="true"
                 tools:ignore="ExportedService">
             <intent-filter>