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

Commit fc384f5a authored by Beth Thibodeau's avatar Beth Thibodeau
Browse files

Add check for null token when updating state 

MediaController requires a non-null token as input, which updateState
did not check before calling. The token is required to be non-null
already for active media, but it can be null for resume players, so it's
possible for users to hit a race condition between the notification
removal (token set to null) and a final PlaybackState update (attempting
to use the token).

Also added an annotation to the MediaControllerFactory method so that Kotlin
code is aware of this requirement.

Fixes: 231733532
Test: atest MediaDataManagerTest
Change-Id: Id5e4a52cba74fafa04331a663cd4ce870fe920a2
parent 133c2347

packages/SystemUI/src/com/android/systemui/media/MediaControllerFactory.java

+2 −1
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.media;



import android.annotation.NonNull;

import android.content.Context;

import android.media.session.MediaController;

import android.media.session.MediaSession;
@@ -39,7 +40,7 @@ public class MediaControllerFactory { 
     *

     * @param token The token for the session. This value must never be null.

     */

    public MediaController create(MediaSession.Token token) {

    public MediaController create(@NonNull MediaSession.Token token) {

        return new MediaController(mContext, token);

    }

}

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

+5 −0
Original line number Diff line number Diff line
@@ -509,6 +509,11 @@ class MediaDataManager( 
     */

    private fun updateState(key: String, state: PlaybackState) {

        mediaEntries.get(key)?.let {

            val token = it.token

            if (token == null) {

                if (DEBUG) Log.d(TAG, "State updated, but token was null")

                return

            }

            val actions = createActionsFromState(it.packageName,

                    mediaControllerFactory.create(it.token), UserHandle(it.userId))

            val data = it.copy(

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

+3 −2
Original line number Diff line number Diff line
@@ -161,8 +161,9 @@ class MediaTimeoutListener @Inject constructor( 
                destroyed = false

                mediaController?.unregisterCallback(this)

                field = value

                mediaController = if (field.token != null) {

                    mediaControllerFactory.create(field.token)

                val token = field.token

                mediaController = if (token != null) {

                    mediaControllerFactory.create(token)

                } else {

                    null

                }

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

+20 −0
Original line number Diff line number Diff line
@@ -978,6 +978,26 @@ class MediaDataManagerTest : SysuiTestCase() { 
                anyBoolean())

    }



    @Test

    fun testPlaybackStateChange_keyHasNullToken_doesNothing() {

        // When we get an update that sets the data's token to null

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

        addNotificationAndLoad()

        val data = mediaDataCaptor.value

        assertThat(data.resumption).isFalse()

        mediaDataManager.onMediaDataLoaded(KEY, null, data.copy(token = null))



        // And then get a state update

        val state = PlaybackState.Builder().build()

        val callbackCaptor = argumentCaptor<(String, PlaybackState) -> Unit>()

        verify(mediaTimeoutListener).stateCallback = capture(callbackCaptor)



        // Then no changes are made

        callbackCaptor.value.invoke(KEY, state)

        verify(listener, never()).onMediaDataLoaded(eq(KEY), any(), any(), anyBoolean(), anyInt(),

            anyBoolean())

    }



    /**

     * Helper function to add a media notification and capture the resulting MediaData

     */