From 962ebd80c103972ea721b20e1dc7a9debbd7d108 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Mon, 6 Mar 2023 13:41:37 -0600 Subject: [PATCH] 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 --- .../controls/resume/MediaResumeListener.kt | 7 ++-- .../controls/resume/ResumeMediaBrowser.java | 36 +++++++++++-------- .../resume/MediaResumeListenerTest.kt | 27 +++++++++++--- 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 2d10b823f784e..25d95a54b043b 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 @@ -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() { 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 3493b2453fd62..d460b5b5d7823 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 = @@ -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. */ 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 136ace1737959..a6be7e8eada9c 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 @@ -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