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

Commit 8651185e authored by Beth Thibodeau's avatar Beth Thibodeau
Browse files

Handle media load events while testing for resume 

An app can send additional notifications, resulting in onMediaDataLoaded calls,
while we are waiting for the callback from its MediaBrowserService to
return. Previously this would result in disconnecting and then starting a new
attempt to connect to the MBS.

Instead,

1. notify MediaDataManager immediately when we initiate the test
connection, so that if we get additional onMediaDataLoaded calls while testing,
it will not try again

2. only disconnect the mediaBrowser in onMediaDataLoaded when the key
changes, since this is only relevant when resuming via the MBS. Otherwise
we might prematurely disconnect while waiting for the callback result.

Bug: 189702977
Test: manual - checked repro with test app
Test: atest MediaResumeListenerTest
Change-Id: I55cf2be6e62ef72c777899575d9caab78eda31be
parent 10de71a3

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

+7 −3
Original line number Original line Diff line number Diff line
@@ -163,8 +163,10 @@ class MediaResumeListener @Inject constructor( 
    ) {

    ) {

        if (useMediaResumption) {

        if (useMediaResumption) {

            // If this had been started from a resume state, disconnect now that it's live

            // If this had been started from a resume state, disconnect now that it's live

            if (!key.equals(oldKey)) {

                mediaBrowser?.disconnect()

                mediaBrowser?.disconnect()

                mediaBrowser = null

                mediaBrowser = null

            }

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

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

            if (data.resumeAction == null && !data.hasCheckedForResume && data.isLocalSession) {

            if (data.resumeAction == null && !data.hasCheckedForResume && data.isLocalSession) {

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

                // TODO also check for a media button receiver intended for restarting (b/154127084)
@@ -194,6 +196,9 @@ class MediaResumeListener @Inject constructor( 
     */

     */

    private fun tryUpdateResumptionList(key: String, componentName: ComponentName) {

    private fun tryUpdateResumptionList(key: String, componentName: ComponentName) {

        Log.d(TAG, "Testing if we can connect to $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?.disconnect()

        mediaBrowser = mediaBrowserFactory.create(

        mediaBrowser = mediaBrowserFactory.create(

                object : ResumeMediaBrowser.Callback() {

                object : ResumeMediaBrowser.Callback() {

                    override fun onConnected() {

                    override fun onConnected() {
@@ -202,7 +207,6 @@ class MediaResumeListener @Inject constructor( 




                    override fun onError() {

                    override fun onError() {

                        Log.e(TAG, "Cannot resume with $componentName")

                        Log.e(TAG, "Cannot resume with $componentName")

                        mediaDataManager.setResumeAction(key, null)

                        mediaBrowser = null

                        mediaBrowser = null

                    }

                    }

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

+31 −3
Original line number Original line Diff line number Diff line
@@ -44,6 +44,7 @@ import org.junit.Test 
import org.junit.runner.RunWith

import org.junit.runner.RunWith

import org.mockito.ArgumentCaptor

import org.mockito.ArgumentCaptor

import org.mockito.ArgumentMatchers.anyInt

import org.mockito.ArgumentMatchers.anyInt

import org.mockito.ArgumentMatchers.isNotNull

import org.mockito.Captor

import org.mockito.Captor

import org.mockito.Mock

import org.mockito.Mock

import org.mockito.Mockito

import org.mockito.Mockito
@@ -182,6 +183,31 @@ class MediaResumeListenerTest : SysuiTestCase() { 
        verify(mediaDataManager).setResumeAction(KEY, null)

        verify(mediaDataManager).setResumeAction(KEY, null)

    }

    }





    @Test

    fun testOnLoad_checksForResume_badService() {

        // Set up MBS that will allow connection but not return 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)



        whenever(resumeBrowser.testConnection()).thenAnswer {

            callbackCaptor.value.onError()

        }



        // When media data is loaded that has not been checked yet, and does not have a MBS

        resumeListener.onMediaDataLoaded(KEY, null, data)

        executor.runAllReady()



        // Then we report back to the manager

        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))

    }



    @Test

    @Test

    fun testOnLoad_remotePlayback_doesNotCheck() {

    fun testOnLoad_remotePlayback_doesNotCheck() {

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

        // When media data is loaded that has not been checked yet, and is not local
@@ -217,10 +243,11 @@ class MediaResumeListenerTest : SysuiTestCase() { 




        // Then we test whether the service is valid

        // Then we test whether the service is valid

        executor.runAllReady()

        executor.runAllReady()

        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))

        verify(resumeBrowser).testConnection()

        verify(resumeBrowser).testConnection()





        // And since it is, we report back to the manager

        // And since it is, we send info to the manager

        verify(mediaDataManager).setResumeAction(eq(KEY), any())

        verify(mediaDataManager).setResumeAction(eq(KEY), isNotNull())





        // But we do not tell it to add new controls

        // But we do not tell it to add new controls

        verify(mediaDataManager, never())

        verify(mediaDataManager, never())
@@ -291,8 +318,9 @@ class MediaResumeListenerTest : SysuiTestCase() { 




        // Then we test whether the service is valid and set the resume action

        // Then we test whether the service is valid and set the resume action

        executor.runAllReady()

        executor.runAllReady()

        verify(mediaDataManager).setResumeAction(eq(KEY), eq(null))

        verify(resumeBrowser).testConnection()

        verify(resumeBrowser).testConnection()

        verify(mediaDataManager).setResumeAction(eq(KEY), capture(actionCaptor))

        verify(mediaDataManager, times(2)).setResumeAction(eq(KEY), capture(actionCaptor))





        // When the resume action is run

        // When the resume action is run

        actionCaptor.value.run()

        actionCaptor.value.run()