Merge "Reduce extra connections made to app" into tm-qpr-dev

3 files changed, 46 insertions, 24 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/resume/MediaResumeListener.kt b/packages/SystemUI/src/com/android/systemui/media/controls/resume/MediaResumeListener.kt
index 92e0c851a462..b0389b50cd7d 100644
--- a/packages/SystemUI/src/com/android/systemui/media/controls/resume/MediaResumeListener.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/controls/resume/MediaResumeListener.kt
@@ -239,6 +239,8 @@ constructor(
                         data.playbackLocation != MediaData.PLAYBACK_CAST_REMOTE)
             if (data.resumeAction == null && !data.hasCheckedForResume && isEligibleForResume) {
                 // 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)
@@ -249,9 +251,6 @@ constructor(
                     backgroundExecutor.execute {
                         tryUpdateResumptionList(key, inf!!.get(0).componentInfo.componentName)
                     }
-                } else {
-                    // No service found
-                    mediaDataManager.setResumeAction(key, null)
                 }
             }
         }
@@ -263,8 +262,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() {
diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/resume/ResumeMediaBrowser.java b/packages/SystemUI/src/com/android/systemui/media/controls/resume/ResumeMediaBrowser.java
index 3493b2453fd6..d460b5b5d782 100644
--- a/packages/SystemUI/src/com/android/systemui/media/controls/resume/ResumeMediaBrowser.java
+++ b/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 =
@@ -202,6 +199,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. */
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/resume/MediaResumeListenerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/resume/MediaResumeListenerTest.kt
index 4dfa6261b868..9ab728949e40 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/resume/MediaResumeListenerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/resume/MediaResumeListenerTest.kt
@@ -313,6 +313,25 @@ class MediaResumeListenerTest : SysuiTestCase() {
     }
 
     @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
         val description = MediaDescription.Builder().setTitle(TITLE).build()
@@ -392,7 +411,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
                 assertThat(result.size).isEqualTo(3)
                 assertThat(result[2].toLong()).isEqualTo(currentTime)
             }
-        verify(sharedPrefsEditor, times(1)).apply()
+        verify(sharedPrefsEditor).apply()
     }
 
     @Test
@@ -432,8 +451,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))
     }
 
@@ -516,7 +535,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
                 assertThat(result.size).isEqualTo(3)
                 assertThat(result[2].toLong()).isEqualTo(currentTime)
             }
-        verify(sharedPrefsEditor, times(1)).apply()
+        verify(sharedPrefsEditor).apply()
     }
 
     @Test