Merge "[Media] In MediaResumeListener, always disconnect the mediaBrowser when it changes." into tm-dev

packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt

@@ -64,6 +64,11 @@ class MediaResumeListener @Inject constructor(
     private lateinit var mediaDataManager: MediaDataManager
 
     private var mediaBrowser: ResumeMediaBrowser? = null
+        set(value) {
+            // Always disconnect the old browser -- see b/225403871.
+            field?.disconnect()
+            field = value
+        }
     private var currentUserId: Int = context.userId
 
     @VisibleForTesting
@@ -189,7 +194,6 @@ class MediaResumeListener @Inject constructor(
         if (useMediaResumption) {
             // If this had been started from a resume state, disconnect now that it's live
             if (!key.equals(oldKey)) {
-                mediaBrowser?.disconnect()
                 mediaBrowser = null
             }
             // If we don't have a resume action, check if we haven't already
@@ -223,7 +227,6 @@ class MediaResumeListener @Inject constructor(
         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() {

packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt

@@ -168,16 +168,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
 
     @Test
     fun testOnLoad_checksForResume_badService() {
-        // Set up MBS that will allow connection but not return valid media
+        setUpMbsWithValidResolveInfo()
-        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()
@@ -213,16 +204,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
 
     @Test
     fun testOnLoad_checksForResume_hasService() {
-        // Set up mocks to successfully find a MBS that returns valid media
+        setUpMbsWithValidResolveInfo()
-        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)
 
         val description = MediaDescription.Builder().setTitle(TITLE).build()
         val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
@@ -288,16 +270,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
 
     @Test
     fun testGetResumeAction_restarts() {
-        // Set up mocks to successfully find a MBS that returns valid media
+        setUpMbsWithValidResolveInfo()
-        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)
 
         val description = MediaDescription.Builder().setTitle(TITLE).build()
         val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
@@ -426,4 +399,91 @@ class MediaResumeListenerTest : SysuiTestCase() {
                 }
         verify(sharedPrefsEditor, times(1)).apply()
     }
-}
+
+    @Test
+    fun testOnMediaDataLoaded_newKeyDifferent_oldMediaBrowserDisconnected() {
+        setUpMbsWithValidResolveInfo()
+
+        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
+        executor.runAllReady()
+
+        resumeListener.onMediaDataLoaded(key = "newKey", oldKey = KEY, data)
+
+        verify(resumeBrowser).disconnect()
+    }
+
+    @Test
+    fun testOnMediaDataLoaded_updatingResumptionListError_mediaBrowserDisconnected() {
+        setUpMbsWithValidResolveInfo()
+
+        // Set up mocks to return with an error
+        whenever(resumeBrowser.testConnection()).thenAnswer {
+            callbackCaptor.value.onError()
+        }
+
+        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
+        executor.runAllReady()
+
+        // Ensure we disconnect the browser
+        verify(resumeBrowser).disconnect()
+    }
+
+    @Test
+    fun testOnMediaDataLoaded_trackAdded_mediaBrowserDisconnected() {
+        setUpMbsWithValidResolveInfo()
+
+        // Set up mocks to return with a track added
+        val description = MediaDescription.Builder().setTitle(TITLE).build()
+        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
+        whenever(resumeBrowser.testConnection()).thenAnswer {
+            callbackCaptor.value.addTrack(description, component, resumeBrowser)
+        }
+
+        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
+        executor.runAllReady()
+
+        // Ensure we disconnect the browser
+        verify(resumeBrowser).disconnect()
+    }
+
+    @Test
+    fun testResumeAction_oldMediaBrowserDisconnected() {
+        setUpMbsWithValidResolveInfo()
+
+        val description = MediaDescription.Builder().setTitle(TITLE).build()
+        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
+        whenever(resumeBrowser.testConnection()).thenAnswer {
+            callbackCaptor.value.addTrack(description, component, resumeBrowser)
+        }
+
+        // Load media data that will require us to get the resume action
+        val dataCopy = data.copy(resumeAction = null, hasCheckedForResume = false)
+        resumeListener.onMediaDataLoaded(KEY, null, dataCopy)
+        executor.runAllReady()
+        verify(mediaDataManager, times(2)).setResumeAction(eq(KEY), capture(actionCaptor))
+
+        // Set up our factory to return a new browser so we can verify we disconnected the old one
+        val newResumeBrowser = mock(ResumeMediaBrowser::class.java)
+        whenever(resumeBrowserFactory.create(capture(callbackCaptor), any()))
+            .thenReturn(newResumeBrowser)
+
+        // When the resume action is run
+        actionCaptor.value.run()
+
+        // Then we disconnect the old one
+        verify(resumeBrowser).disconnect()
+    }
+
+    /** Sets up mocks to successfully find a MBS that returns valid media. */
+    private fun setUpMbsWithValidResolveInfo() {
+        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)
+    }
+}