diff options
4 files changed, 111 insertions, 64 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt index b0be57668d08..3bf7fb07f8eb 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt @@ -23,7 +23,6 @@ import android.content.Intent import android.content.IntentFilter import android.content.pm.PackageManager import android.media.MediaDescription -import android.media.session.MediaController import android.os.UserHandle import android.provider.Settings import android.service.media.MediaBrowserService @@ -171,6 +170,7 @@ 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 we don't have a resume action, check if we haven't already if (data.resumeAction == null && !data.hasCheckedForResume && !blockedApps.contains(data.packageName) && data.isLocalSession) { @@ -201,7 +201,6 @@ class MediaResumeListener @Inject constructor( */ private fun tryUpdateResumptionList(key: String, componentName: ComponentName) { Log.d(TAG, "Testing if we can connect to $componentName") - mediaBrowser?.disconnect() mediaBrowser = mediaBrowserFactory.create( object : ResumeMediaBrowser.Callback() { override fun onConnected() { @@ -211,7 +210,6 @@ class MediaResumeListener @Inject constructor( override fun onError() { Log.e(TAG, "Cannot resume with $componentName") mediaDataManager.setResumeAction(key, null) - mediaBrowser?.disconnect() mediaBrowser = null } @@ -224,7 +222,6 @@ class MediaResumeListener @Inject constructor( Log.d(TAG, "Can get resumable media from $componentName") mediaDataManager.setResumeAction(key, getResumeAction(componentName)) updateResumptionList(componentName) - mediaBrowser?.disconnect() mediaBrowser = null } }, @@ -262,30 +259,7 @@ class MediaResumeListener @Inject constructor( */ private fun getResumeAction(componentName: ComponentName): Runnable { return Runnable { - mediaBrowser?.disconnect() - mediaBrowser = mediaBrowserFactory.create( - object : ResumeMediaBrowser.Callback() { - override fun onConnected() { - if (mediaBrowser?.token == null) { - Log.e(TAG, "Error after connect") - mediaBrowser?.disconnect() - mediaBrowser = null - return - } - Log.d(TAG, "Connected for restart $componentName") - val controller = MediaController(context, mediaBrowser!!.token) - val controls = controller.transportControls - controls.prepare() - controls.play() - } - - override fun onError() { - Log.e(TAG, "Resume failed for $componentName") - mediaBrowser?.disconnect() - mediaBrowser = null - } - }, - componentName) + mediaBrowser = mediaBrowserFactory.create(null, componentName) mediaBrowser?.restart() } } diff --git a/packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowser.java b/packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowser.java index e453653cf7b5..fecc903326f5 100644 --- a/packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowser.java +++ b/packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowser.java @@ -16,6 +16,7 @@ package com.android.systemui.media; +import android.annotation.Nullable; import android.app.PendingIntent; import android.content.ComponentName; import android.content.Context; @@ -47,7 +48,7 @@ public class ResumeMediaBrowser { private static final String TAG = "ResumeMediaBrowser"; private final Context mContext; - private final Callback mCallback; + @Nullable private final Callback mCallback; private MediaBrowserFactory mBrowserFactory; private MediaBrowser mMediaBrowser; private ComponentName mComponentName; @@ -58,8 +59,8 @@ public class ResumeMediaBrowser { * @param callback used to report media items found * @param componentName Component name of the MediaBrowserService this browser will connect to */ - public ResumeMediaBrowser(Context context, Callback callback, ComponentName componentName, - MediaBrowserFactory browserFactory) { + public ResumeMediaBrowser(Context context, @Nullable Callback callback, + ComponentName componentName, MediaBrowserFactory browserFactory) { mContext = context; mCallback = callback; mComponentName = componentName; @@ -93,18 +94,24 @@ public class ResumeMediaBrowser { List<MediaBrowser.MediaItem> children) { if (children.size() == 0) { Log.d(TAG, "No children found for " + mComponentName); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } } else { // We ask apps to return a playable item as the first child when sending // a request with EXTRA_RECENT; if they don't, no resume controls MediaBrowser.MediaItem child = children.get(0); MediaDescription desc = child.getDescription(); if (child.isPlayable() && mMediaBrowser != null) { - mCallback.addTrack(desc, mMediaBrowser.getServiceComponent(), - ResumeMediaBrowser.this); + if (mCallback != null) { + mCallback.addTrack(desc, mMediaBrowser.getServiceComponent(), + ResumeMediaBrowser.this); + } } else { Log.d(TAG, "Child found but not playable for " + mComponentName); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } } } disconnect(); @@ -113,7 +120,9 @@ public class ResumeMediaBrowser { @Override public void onError(String parentId) { Log.d(TAG, "Subscribe error for " + mComponentName + ": " + parentId); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } disconnect(); } @@ -121,7 +130,9 @@ public class ResumeMediaBrowser { public void onError(String parentId, Bundle options) { Log.d(TAG, "Subscribe error for " + mComponentName + ": " + parentId + ", options: " + options); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } disconnect(); } }; @@ -138,13 +149,20 @@ public class ResumeMediaBrowser { Log.d(TAG, "Service connected for " + mComponentName); if (mMediaBrowser != null && mMediaBrowser.isConnected()) { String root = mMediaBrowser.getRoot(); - if (!TextUtils.isEmpty(root) && mMediaBrowser != null) { - mCallback.onConnected(); - mMediaBrowser.subscribe(root, mSubscriptionCallback); + if (!TextUtils.isEmpty(root)) { + if (mCallback != null) { + mCallback.onConnected(); + } + if (mMediaBrowser != null) { + mMediaBrowser.subscribe(root, mSubscriptionCallback); + } return; } } - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } + disconnect(); } /** @@ -153,7 +171,9 @@ public class ResumeMediaBrowser { @Override public void onConnectionSuspended() { Log.d(TAG, "Connection suspended for " + mComponentName); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } disconnect(); } @@ -163,16 +183,18 @@ public class ResumeMediaBrowser { @Override public void onConnectionFailed() { Log.d(TAG, "Connection failed for " + mComponentName); - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } disconnect(); } }; /** - * Disconnect the media browser. This should be called after restart or testConnection have - * completed to close the connection. + * Disconnect the media browser. This should be done after callbacks have completed to + * disconnect from the media browser service. */ - public void disconnect() { + protected void disconnect() { if (mMediaBrowser != null) { mMediaBrowser.disconnect(); } @@ -183,7 +205,8 @@ public class ResumeMediaBrowser { * Connects to the MediaBrowserService and starts playback. * ResumeMediaBrowser.Callback#onError or ResumeMediaBrowser.Callback#onConnected will be called * depending on whether it was successful. - * ResumeMediaBrowser#disconnect should be called after this to ensure the connection is closed. + * If the connection is successful, the listener should call ResumeMediaBrowser#disconnect after + * getting a media update from the app */ public void restart() { disconnect(); @@ -195,7 +218,10 @@ public class ResumeMediaBrowser { public void onConnected() { Log.d(TAG, "Connected for restart " + mMediaBrowser.isConnected()); if (mMediaBrowser == null || !mMediaBrowser.isConnected()) { - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } + disconnect(); return; } MediaSession.Token token = mMediaBrowser.getSessionToken(); @@ -203,17 +229,26 @@ public class ResumeMediaBrowser { controller.getTransportControls(); controller.getTransportControls().prepare(); controller.getTransportControls().play(); - mCallback.onConnected(); + if (mCallback != null) { + mCallback.onConnected(); + } + // listener should disconnect after media player update } @Override public void onConnectionFailed() { - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } + disconnect(); } @Override public void onConnectionSuspended() { - mCallback.onError(); + if (mCallback != null) { + mCallback.onError(); + } + disconnect(); } }, rootHints); mMediaBrowser.connect(); 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 96d1d94795f6..4e1627ff0343 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt @@ -89,6 +89,7 @@ class MediaResumeListenerTest : SysuiTestCase() { @Mock private lateinit var dumpManager: DumpManager @Captor lateinit var callbackCaptor: ArgumentCaptor<ResumeMediaBrowser.Callback> + @Captor lateinit var actionCaptor: ArgumentCaptor<Runnable> private lateinit var executor: FakeExecutor private lateinit var data: MediaData @@ -224,9 +225,6 @@ class MediaResumeListenerTest : SysuiTestCase() { // But we do not tell it to add new controls verify(mediaDataManager, never()) .addResumptionControls(anyInt(), any(), any(), any(), any(), any(), any()) - - // Finally, make sure the resume browser disconnected - verify(resumeBrowser).disconnect() } @Test @@ -267,4 +265,39 @@ class MediaResumeListenerTest : SysuiTestCase() { verify(mediaDataManager, times(3)).addResumptionControls(anyInt(), any(), any(), any(), any(), any(), eq(PACKAGE_NAME)) } + + @Test + fun testGetResumeAction_restarts() { + // Set up mocks to successfully find a MBS that returns 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) + + val description = MediaDescription.Builder().setTitle(TITLE).build() + val component = ComponentName(PACKAGE_NAME, CLASS_NAME) + whenever(resumeBrowser.testConnection()).thenAnswer { + callbackCaptor.value.addTrack(description, component, resumeBrowser) + } + + // When media data is loaded that has not been checked yet, and does have a MBS + val dataCopy = data.copy(resumeAction = null, hasCheckedForResume = false) + resumeListener.onMediaDataLoaded(KEY, null, dataCopy) + + // Then we test whether the service is valid and set the resume action + executor.runAllReady() + verify(resumeBrowser).testConnection() + verify(mediaDataManager).setResumeAction(eq(KEY), capture(actionCaptor)) + + // When the resume action is run + actionCaptor.value.run() + + // Then we call restart + verify(resumeBrowser).restart() + } }
\ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/ResumeMediaBrowserTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/ResumeMediaBrowserTest.kt index d26229edf71a..dfa7c66b38f9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/ResumeMediaBrowserTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/ResumeMediaBrowserTest.kt @@ -91,8 +91,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserFailed() resumeBrowser.testConnection() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -111,8 +112,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserConnectionNoResults() resumeBrowser.testConnection() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -132,8 +134,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserFailed() resumeBrowser.findRecentMedia() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -143,8 +146,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { whenever(browser.getRoot()).thenReturn(null) resumeBrowser.findRecentMedia() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -163,8 +167,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserConnectionNoResults() resumeBrowser.findRecentMedia() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -173,8 +178,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserConnectionNotPlayable() resumeBrowser.findRecentMedia() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -193,8 +199,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { setupBrowserFailed() resumeBrowser.restart() - // Then it calls onError + // Then it calls onError and disconnects verify(callback).onError() + verify(browser).disconnect() } @Test @@ -202,13 +209,11 @@ public class ResumeMediaBrowserTest : SysuiTestCase() { // When restart is called and we connect successfully setupBrowserConnection() resumeBrowser.restart() + verify(callback).onConnected() // Then it creates a new controller and sends play command verify(transportControls).prepare() verify(transportControls).play() - - // Then it calls onConnected - verify(callback).onConnected() } /** |