Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4053be92 authored by Beth Thibodeau's avatar Beth Thibodeau
Browse files

Only allow local sessions to be resumable 

It is possible for an app to be resumable locally, but also post
notifications when playing remotely that are not resumable long-term.
This change makes it so that only local sessions will be used for resume
players, and remote sessions will be removed when the underlying notification
is removed.

Bug: 185681682
Test: manual - start playback on Spotify web, observe notification on
phone, then close app on phone and observe no resume player
Test: atest MediaDataManagerTest MediaResumeListenerTest

Change-Id: I0a6ff2bab234a33298f84fd6d99b7d537d3d5e34
parent c28f00cb

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

+2 −2
Original line number Diff line number Diff line
@@ -586,7 +586,7 @@ class MediaDataManager( 
        }



        val isLocalSession = mediaController.playbackInfo?.playbackType ==

            MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL ?: true

            MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL

        val isPlaying = mediaController.playbackState?.let { isPlayingState(it.state) } ?: null



        foregroundExecutor.execute {
@@ -724,7 +724,7 @@ class MediaDataManager( 
        Assert.isMainThread()

        val removed = mediaEntries.remove(key)

        if (useMediaResumption && removed?.resumeAction != null &&

                !isBlockedFromResume(removed.packageName)) {

                !isBlockedFromResume(removed.packageName) && removed?.isLocalSession == true) {

            Log.d(TAG, "Not removing $key because resumable")

            // Move to resume key (aka package name) if that key doesn't already exist.

            val resumeAction = getResumeMediaAction(removed.resumeAction!!)

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

+1 −1
Original line number Diff line number Diff line
@@ -173,7 +173,7 @@ class MediaResumeListener @Inject constructor( 
            mediaBrowser?.disconnect()

            // If we don't have a resume action, check if we haven't already

            if (data.resumeAction == null && !data.hasCheckedForResume &&

                    !blockedApps.contains(data.packageName)) {

                    !blockedApps.contains(data.packageName) && data.isLocalSession) {

                // TODO also check for a media button receiver intended for restarting (b/154127084)

                Log.d(TAG, "Checking for service component for " + data.packageName)

                val pm = context.packageManager

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

+25 −0
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ class MediaDataManagerTest : SysuiTestCase() { 
    @JvmField @Rule val mockito = MockitoJUnit.rule()

    @Mock lateinit var mediaControllerFactory: MediaControllerFactory

    @Mock lateinit var controller: MediaController

    @Mock lateinit var playbackInfo: MediaController.PlaybackInfo

    lateinit var session: MediaSession

    lateinit var metadataBuilder: MediaMetadata.Builder

    lateinit var backgroundExecutor: FakeExecutor
@@ -118,6 +119,9 @@ class MediaDataManagerTest : SysuiTestCase() { 
            putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_TITLE)

        }

        whenever(mediaControllerFactory.create(eq(session.sessionToken))).thenReturn(controller)

        whenever(controller.playbackInfo).thenReturn(playbackInfo)

        whenever(playbackInfo.playbackType).thenReturn(

                MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL)



        // This is an ugly hack for now. The mediaSessionBasedFilter is one of the internal

        // listeners in the internal processing pipeline. It receives events, but ince it is a
@@ -229,6 +233,27 @@ class MediaDataManagerTest : SysuiTestCase() { 
        verify(listener).onMediaDataRemoved(eq(KEY_2))

    }



    @Test

    fun testOnNotificationRemoved_withResumption_butNotLocal() {

        // GIVEN that the manager has a notification with a resume action, but is not local

        whenever(controller.metadata).thenReturn(metadataBuilder.build())

        whenever(playbackInfo.playbackType).thenReturn(

                MediaController.PlaybackInfo.PLAYBACK_TYPE_REMOTE)

        mediaDataManager.onNotificationAdded(KEY, mediaNotification)

        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)

        assertThat(foregroundExecutor.runAllReady()).isEqualTo(1)

        verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor))

        val data = mediaDataCaptor.value

        val dataRemoteWithResume = data.copy(resumeAction = Runnable {}, isLocalSession = false)

        mediaDataManager.onMediaDataLoaded(KEY, null, dataRemoteWithResume)



        // WHEN the notification is removed

        mediaDataManager.onNotificationRemoved(KEY)



        // THEN the media data is removed

        verify(listener).onMediaDataRemoved(eq(KEY))

    }



    @Test

    fun testAppBlockedFromResumption() {

        // GIVEN that the manager has a notification with a resume action

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

+10 −0
Original line number Diff line number Diff line
@@ -181,6 +181,16 @@ class MediaResumeListenerTest : SysuiTestCase() { 
        verify(mediaDataManager).setResumeAction(KEY, null)

    }



    @Test

    fun testOnLoad_remotePlayback_doesNotCheck() {

        // When media data is loaded that has not been checked yet, and is not local

        val dataRemote = data.copy(isLocalSession = false)

        resumeListener.onMediaDataLoaded(KEY, null, dataRemote)



        // Then we do not take action

        verify(mediaDataManager, never()).setResumeAction(any(), any())

    }



    @Test

    fun testOnLoad_checksForResume_hasService() {

        // Set up mocks to successfully find a MBS that returns valid media