Handle case where GET_CONTENT is */* + media extra mime type

The original change (and subsequent roll out of the redirect_get_content
flag) caused a few CTS tests to start failing. This was a result of
a missed edge case where the intent type is */*, however, the intent
has EXTRA_MIME_TYPES set to exclusively media mime types. This should
be forwarded to the photopicker (which will have an escape hatch to
send it back to DocumentsUI).

The logic has been updated along with some general test hygiene where
the photopicker activities were just being started on top of one
another and not being closed. This lead to tests not being
deterministic and randomly failing as well.

Bug: 397672317
Bug: 377771195
Flag: com.android.documentsui.flags.redirect_get_content
Test: atest com.android.documentsui.TrampolineActivityTest
Test: atest android.photopicker.cts.PhotoPickerTest
Test: atest android.photopicker.cts.ActionGetContentOnlyTest

Change-Id: I5e54e1d4ba4219bd38f53232581ed12c93f4b5d5

2 files changed, 54 insertions, 30 deletions

diff --git a/src/com/android/documentsui/picker/TrampolineActivity.kt b/src/com/android/documentsui/picker/TrampolineActivity.kt
index 6cb7d37a1..e9e9739a3 100644
--- a/src/com/android/documentsui/picker/TrampolineActivity.kt
+++ b/src/com/android/documentsui/picker/TrampolineActivity.kt
@@ -137,30 +137,32 @@ class TrampolineActivity : AppCompatActivity() {
 }
 
 fun shouldForwardIntentToPhotopicker(intent: Intent): Boolean {
-    if (intent.action != ACTION_GET_CONTENT || !isMediaMimeType(intent.type)) {
+    // Photopicker can only handle `ACTION_GET_CONTENT` intents.
+    if (intent.action != ACTION_GET_CONTENT) {
         return false
     }
 
-    // Intent has type ACTION_GET_CONTENT and is either image/* or video/* with no
-    // additional mime types.
-    if (!intent.hasExtra(Intent.EXTRA_MIME_TYPES)) {
-        return true
+    // Photopicker only handles media mime types (i.e. image/* or video/*), however, it also handles
+    // requests that have type */* with EXTRA_MIME_TYPES that are media mime types. In that scenario
+    // it provides an escape hatch to the user to go back to DocumentsUI.
+    val intentTypeIsMedia = isMediaMimeType(intent.type)
+    if (!intentTypeIsMedia && intent.type != "*/*") {
+        return false
     }
 
     val extraMimeTypes = intent.getStringArrayExtra(Intent.EXTRA_MIME_TYPES)
-    extraMimeTypes?.let {
-        if (it.size == 0) {
-            return false
-        }
 
-        for (mimeType in it) {
-            if (!isMediaMimeType(mimeType)) {
-                return false
-            }
-        }
-    } ?: return false
+    // In the event there were no `EXTRA_MIME_TYPES` this should exclusively be handled by
+    // DocumentsUI and not Photopicker.
+    if (intent.type == "*/*" && extraMimeTypes == null) {
+        return false
+    }
+
+    if (extraMimeTypes == null) {
+        return intentTypeIsMedia
+    }
 
-    return true
+    return extraMimeTypes.isNotEmpty() && extraMimeTypes.none { !isMediaMimeType(it) }
 }
 
 fun isMediaMimeType(mimeType: String?): Boolean {
diff --git a/tests/functional/com/android/documentsui/TrampolineActivityTest.kt b/tests/functional/com/android/documentsui/TrampolineActivityTest.kt
index c2201789b..48e6b7614 100644
--- a/tests/functional/com/android/documentsui/TrampolineActivityTest.kt
+++ b/tests/functional/com/android/documentsui/TrampolineActivityTest.kt
@@ -15,7 +15,10 @@
  */
 package com.android.documentsui
 
+import android.app.Instrumentation
 import android.content.Intent
+import android.content.Intent.ACTION_GET_CONTENT
+import android.content.IntentFilter
 import android.os.Build.VERSION_CODES
 import android.platform.test.annotations.RequiresFlagsEnabled
 import android.platform.test.flag.junit.CheckFlagsRule
@@ -30,6 +33,7 @@ import androidx.test.uiautomator.Until
 import com.android.documentsui.flags.Flags.FLAG_REDIRECT_GET_CONTENT
 import com.android.documentsui.picker.TrampolineActivity
 import java.util.regex.Pattern
+import org.junit.After
 import org.junit.Assert.assertNotNull
 import org.junit.Before
 import org.junit.BeforeClass
@@ -49,15 +53,27 @@ import org.junit.runners.Suite.SuiteClasses
 class TrampolineActivityTest() {
     companion object {
         const val UI_TIMEOUT = 5000L
-        val PHOTOPICKER_PACKAGE_REGEX: Pattern = Pattern.compile(".*photopicker.*")
+        val PHOTOPICKER_PACKAGE_REGEX: Pattern = Pattern.compile(".*(photopicker|media\\.module).*")
         val DOCUMENTSUI_PACKAGE_REGEX: Pattern = Pattern.compile(".*documentsui.*")
 
-        private var device: UiDevice? = null
+        private lateinit var device: UiDevice
+
+        private lateinit var monitor: Instrumentation.ActivityMonitor
 
         @BeforeClass
         @JvmStatic
         fun setUp() {
             device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
+
+            // Monitor to wait for the activity that starts with the `ACTION_GET_CONTENT` intent.
+            val intentFilter = IntentFilter().apply { addAction(ACTION_GET_CONTENT) }
+            monitor =
+                Instrumentation.ActivityMonitor(
+                    intentFilter,
+                    null, // Expected result from startActivityForResult.
+                    true, // Whether to block until activity started or not.
+                )
+            InstrumentationRegistry.getInstrumentation().addMonitor(monitor)
         }
     }
 
@@ -122,7 +138,7 @@ class TrampolineActivityTest() {
                     GetContentIntentData(
                         mimeType = "*/*",
                         extraMimeTypes = arrayOf("image/*", "video/*"),
-                        expectedApp = AppType.DOCUMENTSUI,
+                        expectedApp = AppType.PHOTOPICKER,
                     ),
                     GetContentIntentData(
                         mimeType = "image/*",
@@ -141,7 +157,7 @@ class TrampolineActivityTest() {
         @Before
         fun setUp() {
             val context = InstrumentationRegistry.getInstrumentation().targetContext
-            val intent = Intent(Intent.ACTION_GET_CONTENT)
+            val intent = Intent(ACTION_GET_CONTENT)
             intent.setClass(context, TrampolineActivity::class.java)
             intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
             intent.setType(testData.mimeType)
@@ -150,6 +166,11 @@ class TrampolineActivityTest() {
             context.startActivity(intent)
         }
 
+        @After
+        fun tearDown() {
+            monitor.waitForActivityWithTimeout(UI_TIMEOUT)?.finish()
+        }
+
         @Test
         fun testCorrectAppIsLaunched() {
             val bySelector = when (testData.expectedApp) {
@@ -157,7 +178,7 @@ class TrampolineActivityTest() {
                 else -> By.pkg(DOCUMENTSUI_PACKAGE_REGEX)
             }
 
-            assertNotNull(device?.wait(Until.findObject(bySelector), UI_TIMEOUT))
+            assertNotNull(device.wait(Until.findObject(bySelector), UI_TIMEOUT))
         }
     }
 
@@ -170,21 +191,22 @@ class TrampolineActivityTest() {
         @Test
         fun testReferredGetContentFromPhotopickerShouldNotRedirectBack() {
             val context = InstrumentationRegistry.getInstrumentation().targetContext
-            val intent = Intent(Intent.ACTION_GET_CONTENT)
+            val intent = Intent(ACTION_GET_CONTENT)
             intent.setClass(context, TrampolineActivity::class.java)
             intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
-            intent.setType("image/*")
+            intent.setType("*/*")
+            intent.putExtra(Intent.EXTRA_MIME_TYPES, arrayOf("image/*"))
 
             context.startActivity(intent)
-            val moreButton = device?.wait(Until.findObject(By.desc("More")), UI_TIMEOUT)
+            val moreButton = device.wait(Until.findObject(By.descContains("More")), UI_TIMEOUT)
             moreButton?.click()
 
-            val browseButton = device?.wait(Until.findObject(By.textContains("Browse")), UI_TIMEOUT)
+            val browseButton = device.wait(Until.findObject(By.textContains("Browse")), UI_TIMEOUT)
             browseButton?.click()
 
             assertNotNull(
                 "DocumentsUI has not launched",
-                device?.wait(Until.findObject(By.pkg(DOCUMENTSUI_PACKAGE_REGEX)), UI_TIMEOUT)
+                device.wait(Until.findObject(By.pkg(DOCUMENTSUI_PACKAGE_REGEX)), UI_TIMEOUT)
             )
         }
 
@@ -192,7 +214,7 @@ class TrampolineActivityTest() {
         @SdkSuppress(minSdkVersion = VERSION_CODES.S, maxSdkVersion = VERSION_CODES.S_V2)
         fun testAndroidSWithTakeoverGetContentDisabledShouldNotReferToDocumentsUI() {
             val context = InstrumentationRegistry.getInstrumentation().targetContext
-            val intent = Intent(Intent.ACTION_GET_CONTENT)
+            val intent = Intent(ACTION_GET_CONTENT)
             intent.setClass(context, TrampolineActivity::class.java)
             intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
             intent.setType("image/*")
@@ -201,15 +223,15 @@ class TrampolineActivityTest() {
                 // Disable Photopicker from taking over `ACTION_GET_CONTENT`. In this situation, it
                 // should ALWAYS defer to DocumentsUI regardless if the mimetype satisfies the
                 // conditions.
-                device?.executeShellCommand(
+                device.executeShellCommand(
                     "device_config put mediaprovider take_over_get_content false"
                 )
                 context.startActivity(intent)
                 assertNotNull(
-                    device?.wait(Until.findObject(By.pkg(DOCUMENTSUI_PACKAGE_REGEX)), UI_TIMEOUT)
+                    device.wait(Until.findObject(By.pkg(DOCUMENTSUI_PACKAGE_REGEX)), UI_TIMEOUT)
                 )
             } finally {
-                device?.executeShellCommand(
+                device.executeShellCommand(
                     "device_config delete mediaprovider take_over_get_content"
                 )
             }