Handle media load events while testing for resume

An app can send additional notifications, resulting in onMediaDataLoaded calls,
while we are waiting for the callback from its MediaBrowserService to
return. Previously this would result in disconnecting and then starting a new
attempt to connect to the MBS.

Instead,

1. notify MediaDataManager immediately when we initiate the test
connection, so that if we get additional onMediaDataLoaded calls while testing,
it will not try again

2. only disconnect the mediaBrowser in onMediaDataLoaded when the key
changes, since this is only relevant when resuming via the MBS. Otherwise
we might prematurely disconnect while waiting for the callback result.

Bug: 189702977
Test: manual - checked repro with test app
Test: atest MediaResumeListenerTest
Change-Id: I55cf2be6e62ef72c777899575d9caab78eda31be

2 files changed, 38 insertions, 6 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
index 9aeb63d724e3..0da84fbac600 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
@@ -163,8 +163,10 @@ class MediaResumeListener @Inject constructor(
     ) {
         if (useMediaResumption) {
             // If this had been started from a resume state, disconnect now that it's live
-            mediaBrowser?.disconnect()
-            mediaBrowser = null
+            if (!key.equals(oldKey)) {
+                mediaBrowser?.disconnect()
+                mediaBrowser = null
+            }
             // If we don't have a resume action, check if we haven't already
             if (data.resumeAction == null && !data.hasCheckedForResume && data.isLocalSession) {
                 // TODO also check for a media button receiver intended for restarting (b/154127084)
@@ -194,6 +196,9 @@ class MediaResumeListener @Inject constructor(
      */
     private fun tryUpdateResumptionList(key: String, componentName: ComponentName) {
         Log.d(TAG, "Testing if we can connect to $componentName")
+        // Set null action to prevent additional attempts to connect
+        mediaDataManager.setResumeAction(key, null)
+        mediaBrowser?.disconnect()
         mediaBrowser = mediaBrowserFactory.create(
                 object : ResumeMediaBrowser.Callback() {
                     override fun onConnected() {
@@ -202,7 +207,6 @@ class MediaResumeListener @Inject constructor(
 
                     override fun onError() {
                         Log.e(TAG, "Cannot resume with $componentName")
-                        mediaDataManager.setResumeAction(key, null)
                         mediaBrowser = null
                     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt
index 4e1627ff0343..150f4545bd43 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt
@@ -44,6 +44,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
 import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.ArgumentMatchers.isNotNull
 import org.mockito.Captor
 import org.mockito.Mock
 import org.mockito.Mockito
@@ -183,6 +184,31 @@ class MediaResumeListenerTest : SysuiTestCase() {
     }
 
     @Test
+    fun testOnLoad_checksForResume_badService() {
+        // Set up MBS that will allow connection but not return valid media
+        val pm = mock(PackageManager::class.java)
+        whenever(mockContext.packageManager).thenReturn(pm)
+        val resolveInfo = ResolveInfo()
+        val serviceInfo = ServiceInfo()
+        serviceInfo.packageName = PACKAGE_NAME
+        resolveInfo.serviceInfo = serviceInfo
+        resolveInfo.serviceInfo.name = CLASS_NAME
+        val resumeInfo = listOf(resolveInfo)
+        whenever(pm.queryIntentServices(any(), anyInt())).thenReturn(resumeInfo)
+
+        whenever(resumeBrowser.testConnection()).thenAnswer {
+            callbackCaptor.value.onError()
+        }
+
+        // When media data is loaded that has not been checked yet, and does not have a MBS
+        resumeListener.onMediaDataLoaded(KEY, null, data)
+        executor.runAllReady()
+
+        // Then we report back to the manager
+        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))
+    }
+
+    @Test
     fun testOnLoad_remotePlayback_doesNotCheck() {
         // When media data is loaded that has not been checked yet, and is not local
         val dataRemote = data.copy(isLocalSession = false)
@@ -217,10 +243,11 @@ class MediaResumeListenerTest : SysuiTestCase() {
 
         // Then we test whether the service is valid
         executor.runAllReady()
+        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))
         verify(resumeBrowser).testConnection()
 
-        // And since it is, we report back to the manager
-        verify(mediaDataManager).setResumeAction(eq(KEY), any())
+        // And since it is, we send info to the manager
+        verify(mediaDataManager).setResumeAction(eq(KEY), isNotNull())
 
         // But we do not tell it to add new controls
         verify(mediaDataManager, never())
@@ -291,8 +318,9 @@ class MediaResumeListenerTest : SysuiTestCase() {
 
         // Then we test whether the service is valid and set the resume action
         executor.runAllReady()
+        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))
         verify(resumeBrowser).testConnection()
-        verify(mediaDataManager).setResumeAction(eq(KEY), capture(actionCaptor))
+        verify(mediaDataManager, times(2)).setResumeAction(eq(KEY), capture(actionCaptor))
 
         // When the resume action is run
         actionCaptor.value.run()