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

Commit 849d1ec7 authored by Caitlin Cassidy's avatar Caitlin Cassidy
Browse files

[Media] In MediaResumeListener, always disconnect the mediaBrowser when

it changes.

In some places in MediaResumeListener, like #getResumeAction, we were
re-setting the value of `mediaBrowser` without disconnecting the old one
(if it existed), so this might be causing the issue where SysUI is still
bound to an old service.

I haven't been able to repro b/225403871 yet so I'm not sure if this
does fix it but I think this is a good change regardless, and once it's in
some builds we can see if it helps.

Bug: 225403871
Test: MediaResumeListenerTest
Test: manual: verified different media cases still work
Change-Id: Ia123fcdce537ca845c62aa29958426d48e563cf7
parent 1d23f37e
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -64,6 +64,11 @@ class MediaResumeListener @Inject constructor(
    private lateinit var mediaDataManager: MediaDataManager

    private var mediaBrowser: ResumeMediaBrowser? = null
        set(value) {
            // Always disconnect the old browser -- see b/225403871.
            field?.disconnect()
            field = value
        }
    private var currentUserId: Int = context.userId

    @VisibleForTesting
@@ -189,7 +194,6 @@ class MediaResumeListener @Inject constructor(
        if (useMediaResumption) {
            // If this had been started from a resume state, disconnect now that it's live
            if (!key.equals(oldKey)) {
                mediaBrowser?.disconnect()
                mediaBrowser = null
            }
            // If we don't have a resume action, check if we haven't already
@@ -223,7 +227,6 @@ class MediaResumeListener @Inject constructor(
        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(
                object : ResumeMediaBrowser.Callback() {
                    override fun onConnected() {
+91 −31
Original line number Diff line number Diff line
@@ -168,16 +168,7 @@ class MediaResumeListenerTest : SysuiTestCase() {

    @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)
        setUpMbsWithValidResolveInfo()

        whenever(resumeBrowser.testConnection()).thenAnswer {
            callbackCaptor.value.onError()
@@ -213,16 +204,7 @@ class MediaResumeListenerTest : SysuiTestCase() {

    @Test
    fun testOnLoad_checksForResume_hasService() {
        // 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)
        setUpMbsWithValidResolveInfo()

        val description = MediaDescription.Builder().setTitle(TITLE).build()
        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
@@ -288,16 +270,7 @@ class MediaResumeListenerTest : SysuiTestCase() {

    @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)
        setUpMbsWithValidResolveInfo()

        val description = MediaDescription.Builder().setTitle(TITLE).build()
        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
@@ -426,4 +399,91 @@ class MediaResumeListenerTest : SysuiTestCase() {
                }
        verify(sharedPrefsEditor, times(1)).apply()
    }

    @Test
    fun testOnMediaDataLoaded_newKeyDifferent_oldMediaBrowserDisconnected() {
        setUpMbsWithValidResolveInfo()

        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
        executor.runAllReady()

        resumeListener.onMediaDataLoaded(key = "newKey", oldKey = KEY, data)

        verify(resumeBrowser).disconnect()
    }

    @Test
    fun testOnMediaDataLoaded_updatingResumptionListError_mediaBrowserDisconnected() {
        setUpMbsWithValidResolveInfo()

        // Set up mocks to return with an error
        whenever(resumeBrowser.testConnection()).thenAnswer {
            callbackCaptor.value.onError()
        }

        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
        executor.runAllReady()

        // Ensure we disconnect the browser
        verify(resumeBrowser).disconnect()
    }

    @Test
    fun testOnMediaDataLoaded_trackAdded_mediaBrowserDisconnected() {
        setUpMbsWithValidResolveInfo()

        // Set up mocks to return with a track added
        val description = MediaDescription.Builder().setTitle(TITLE).build()
        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
        whenever(resumeBrowser.testConnection()).thenAnswer {
            callbackCaptor.value.addTrack(description, component, resumeBrowser)
        }

        resumeListener.onMediaDataLoaded(key = KEY, oldKey = null, data)
        executor.runAllReady()

        // Ensure we disconnect the browser
        verify(resumeBrowser).disconnect()
    }

    @Test
    fun testResumeAction_oldMediaBrowserDisconnected() {
        setUpMbsWithValidResolveInfo()

        val description = MediaDescription.Builder().setTitle(TITLE).build()
        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
        whenever(resumeBrowser.testConnection()).thenAnswer {
            callbackCaptor.value.addTrack(description, component, resumeBrowser)
        }

        // Load media data that will require us to get the resume action
        val dataCopy = data.copy(resumeAction = null, hasCheckedForResume = false)
        resumeListener.onMediaDataLoaded(KEY, null, dataCopy)
        executor.runAllReady()
        verify(mediaDataManager, times(2)).setResumeAction(eq(KEY), capture(actionCaptor))

        // Set up our factory to return a new browser so we can verify we disconnected the old one
        val newResumeBrowser = mock(ResumeMediaBrowser::class.java)
        whenever(resumeBrowserFactory.create(capture(callbackCaptor), any()))
            .thenReturn(newResumeBrowser)

        // When the resume action is run
        actionCaptor.value.run()

        // Then we disconnect the old one
        verify(resumeBrowser).disconnect()
    }

    /** Sets up mocks to successfully find a MBS that returns valid media. */
    private fun setUpMbsWithValidResolveInfo() {
        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)
    }
}