Reduce extra connections made to app

When new media data is loaded, update manager immediately, to avoid
extra unnecessary connections if the app sends more updates before we
can start the test connection.
Also improve connection handling in ResumeMediaBrowser to remove
redundant calls to disconnect

Test: atest MediaResumeListenerTest ResumeMediaBrowserTest
Fixes: 271211860
Change-Id: If2c7b9fe761c5b453f699fa3bfa5bb05436d1ac4

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

@@ -233,6 +233,8 @@ constructor(
             // 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)
+                // Set null action to prevent additional attempts to connect
+                mediaDataManager.setResumeAction(key, null)
                 Log.d(TAG, "Checking for service component for " + data.packageName)
                 val pm = context.packageManager
                 val serviceIntent = Intent(MediaBrowserService.SERVICE_INTERFACE)
@@ -243,9 +245,6 @@ constructor(
                     backgroundExecutor.execute {
                         tryUpdateResumptionList(key, inf!!.get(0).componentInfo.componentName)
                     }
-                } else {
-                    // No service found
-                    mediaDataManager.setResumeAction(key, null)
                 }
             }
         }
@@ -257,8 +256,6 @@ 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 =
             mediaBrowserFactory.create(
                 object : ResumeMediaBrowser.Callback() {

packages/SystemUI/src/com/android/systemui/media/controls/resume/ResumeMediaBrowser.java

@@ -85,16 +85,13 @@ public class ResumeMediaBrowser {
      * ResumeMediaBrowser#disconnect will be called automatically with this function.
      */
     public void findRecentMedia() {
-        disconnect();
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = mBrowserFactory.create(
+        MediaBrowser browser = mBrowserFactory.create(
                 mComponentName,
                 mConnectionCallback,
                 rootHints);
-        updateMediaController();
-        mLogger.logConnection(mComponentName, "findRecentMedia");
-        mMediaBrowser.connect();
+        connectBrowser(browser, "findRecentMedia");
     }
 
     private final MediaBrowser.SubscriptionCallback mSubscriptionCallback =
@@ -201,6 +198,21 @@ public class ResumeMediaBrowser {
         }
     };
 
+    /**
+     * Connect using a new media browser. Disconnects the existing browser first, if it exists.
+     * @param browser media browser to connect
+     * @param reason Reason to log for connection
+     */
+    private void connectBrowser(MediaBrowser browser, String reason) {
+        mLogger.logConnection(mComponentName, reason);
+        disconnect();
+        mMediaBrowser = browser;
+        if (browser != null) {
+            browser.connect();
+        }
+        updateMediaController();
+    }
+
     /**
      * Disconnect the media browser. This should be done after callbacks have completed to
      * disconnect from the media browser service.
@@ -222,10 +234,9 @@ public class ResumeMediaBrowser {
      * getting a media update from the app
      */
     public void restart() {
-        disconnect();
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = mBrowserFactory.create(mComponentName,
+        MediaBrowser browser = mBrowserFactory.create(mComponentName,
                 new MediaBrowser.ConnectionCallback() {
                     @Override
                     public void onConnected() {
@@ -265,9 +276,7 @@ public class ResumeMediaBrowser {
                         disconnect();
                     }
                 }, rootHints);
-        updateMediaController();
-        mLogger.logConnection(mComponentName, "restart");
-        mMediaBrowser.connect();
+        connectBrowser(browser, "restart");
     }
 
     @VisibleForTesting
@@ -305,16 +314,13 @@ public class ResumeMediaBrowser {
      * ResumeMediaBrowser#disconnect should be called after this to ensure the connection is closed.
      */
     public void testConnection() {
-        disconnect();
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = mBrowserFactory.create(
+        MediaBrowser browser = mBrowserFactory.create(
                 mComponentName,
                 mConnectionCallback,
                 rootHints);
-        updateMediaController();
-        mLogger.logConnection(mComponentName, "testConnection");
-        mMediaBrowser.connect();
+        connectBrowser(browser, "testConnection");
     }
 
     /** Updates mMediaController based on our current browser values. */

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

@@ -281,6 +281,25 @@ class MediaResumeListenerTest : SysuiTestCase() {
         verify(mediaDataManager, never()).setResumeAction(KEY, null)
     }
 
+    @Test
+    fun testOnLoadTwice_onlyChecksOnce() {
+        // When data is first loaded,
+        setUpMbsWithValidResolveInfo()
+        resumeListener.onMediaDataLoaded(KEY, null, data)
+
+        // We notify the manager to set a null action
+        verify(mediaDataManager).setResumeAction(KEY, null)
+
+        // If we then get another update from the app before the first check completes
+        assertThat(executor.numPending()).isEqualTo(1)
+        var dataWithCheck = data.copy(hasCheckedForResume = true)
+        resumeListener.onMediaDataLoaded(KEY, null, dataWithCheck)
+
+        // We do not try to start another check
+        assertThat(executor.numPending()).isEqualTo(1)
+        verify(mediaDataManager).setResumeAction(KEY, null)
+    }
+
     @Test
     fun testOnUserUnlock_loadsTracks() {
         // Set up mock service to successfully find valid media
@@ -361,7 +380,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
                 assertThat(result.size).isEqualTo(3)
                 assertThat(result[2].toLong()).isEqualTo(currentTime)
             }
-        verify(sharedPrefsEditor, times(1)).apply()
+        verify(sharedPrefsEditor).apply()
     }
 
     @Test
@@ -400,8 +419,8 @@ class MediaResumeListenerTest : SysuiTestCase() {
         resumeListener.userUnlockReceiver.onReceive(mockContext, intent)
 
         // We add its resume controls
-        verify(resumeBrowser, times(1)).findRecentMedia()
-        verify(mediaDataManager, times(1))
+        verify(resumeBrowser).findRecentMedia()
+        verify(mediaDataManager)
             .addResumptionControls(anyInt(), any(), any(), any(), any(), any(), eq(PACKAGE_NAME))
     }
 
@@ -482,7 +501,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
                 assertThat(result.size).isEqualTo(3)
                 assertThat(result[2].toLong()).isEqualTo(currentTime)
             }
-        verify(sharedPrefsEditor, times(1)).apply()
+        verify(sharedPrefsEditor).apply()
     }
 
     @Test