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

Commit 45e22250 authored by Jernej Virag's avatar Jernej Virag Committed by Android (Google) Code Review
Browse files

Merge "Fix cancellation of MediaDataLoader jobs" into main

parents a571aa74 46a06145
Loading
Loading
Loading
Loading
+44 −8
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.flags.Flags.MEDIA_RESUME_PROGRESS
import com.android.systemui.flags.fakeFeatureFlagsClassic
import com.android.systemui.graphics.ImageLoader
import com.android.systemui.graphics.imageLoader
import com.android.systemui.kosmos.testDispatcher
import com.android.systemui.kosmos.testScope
@@ -45,12 +46,18 @@ import com.android.systemui.res.R
import com.android.systemui.statusbar.SbnBuilder
import com.android.systemui.testKosmos
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.kotlin.any
import org.mockito.kotlin.mock
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

private const val KEY = "KEY"
@@ -88,12 +95,13 @@ class MediaDataLoaderTest : SysuiTestCase() {
            mediaControllerFactory,
            mediaFlags,
            kosmos.imageLoader,
            statusBarManager
            statusBarManager,
        )

    @Before
    fun setUp() {
        mediaControllerFactory.setControllerForToken(session.sessionToken, mediaController)
        whenever(mediaController.metadata).then { metadataBuilder.build() }
    }

    @Test
@@ -115,7 +123,7 @@ class MediaDataLoaderTest : SysuiTestCase() {
                        0,
                        0,
                        AudioAttributes.Builder().build(),
                        null
                        null,
                    )
                )
            whenever(mediaController.metadata)
@@ -126,7 +134,7 @@ class MediaDataLoaderTest : SysuiTestCase() {
                        .putBitmap(MediaMetadata.METADATA_KEY_ALBUM_ART, albumArt)
                        .putLong(
                            MediaConstants.METADATA_KEY_IS_EXPLICIT,
                            MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT
                            MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT,
                        )
                        .build()
                )
@@ -161,12 +169,12 @@ class MediaDataLoaderTest : SysuiTestCase() {
            val extras = Bundle()
            extras.putInt(
                MediaConstants.DESCRIPTION_EXTRAS_KEY_COMPLETION_STATUS,
                MediaConstants.DESCRIPTION_EXTRAS_VALUE_COMPLETION_STATUS_PARTIALLY_PLAYED
                MediaConstants.DESCRIPTION_EXTRAS_VALUE_COMPLETION_STATUS_PARTIALLY_PLAYED,
            )
            extras.putDouble(MediaConstants.DESCRIPTION_EXTRAS_KEY_COMPLETION_PERCENTAGE, 0.3)
            extras.putLong(
                MediaConstants.METADATA_KEY_IS_EXPLICIT,
                MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT
                MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT,
            )

            val description =
@@ -189,7 +197,7 @@ class MediaDataLoaderTest : SysuiTestCase() {
                    session.sessionToken,
                    APP_NAME,
                    intent,
                    PACKAGE_NAME
                    PACKAGE_NAME,
                )
            assertThat(result).isNotNull()
            assertThat(result?.appName).isEqualTo(APP_NAME)
@@ -372,9 +380,37 @@ class MediaDataLoaderTest : SysuiTestCase() {
            assertThat(result).isNotNull()
        }

    @OptIn(ExperimentalCoroutinesApi::class)
    @Test
    fun testLoadMediaDataInBg_cancelMultipleScheduledTasks() =
        testScope.runTest {
            val mockImageLoader = mock<ImageLoader>()
            val mediaDataLoader =
                MediaDataLoader(
                    context,
                    testDispatcher,
                    testScope,
                    mediaControllerFactory,
                    mediaFlags,
                    mockImageLoader,
                    statusBarManager,
                )
            metadataBuilder.putString(
                MediaMetadata.METADATA_KEY_ALBUM_ART_URI,
                "content://album_art_uri",
            )

            testScope.launch { mediaDataLoader.loadMediaData(KEY, createMediaNotification()) }
            testScope.launch { mediaDataLoader.loadMediaData(KEY, createMediaNotification()) }
            testScope.launch { mediaDataLoader.loadMediaData(KEY, createMediaNotification()) }
            testScope.advanceUntilIdle()

            verify(mockImageLoader, times(1)).loadBitmap(any(), anyInt(), anyInt(), anyInt())
        }

    private fun createMediaNotification(
        mediaSession: MediaSession? = session,
        applicationInfo: ApplicationInfo? = null
        applicationInfo: ApplicationInfo? = null,
    ): StatusBarNotification =
        SbnBuilder().run {
            setPkg(PACKAGE_NAME)
@@ -385,7 +421,7 @@ class MediaDataLoaderTest : SysuiTestCase() {
                    val bundle = Bundle()
                    bundle.putParcelable(
                        Notification.EXTRA_BUILDER_APPLICATION_INFO,
                        applicationInfo
                        applicationInfo,
                    )
                    it.addExtras(bundle)
                }
+34 −27
Original line number Diff line number Diff line
@@ -42,7 +42,6 @@ import android.service.notification.StatusBarNotification
import android.support.v4.media.MediaMetadataCompat
import android.text.TextUtils
import android.util.Log
import android.util.Pair
import androidx.media.utils.MediaConstants
import com.android.app.tracing.coroutines.traceCoroutine
import com.android.systemui.dagger.SysUISingleton
@@ -70,6 +69,7 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.ensureActive

/** Loads media information from media style [StatusBarNotification] classes. */
@@ -85,7 +85,7 @@ constructor(
    private val imageLoader: ImageLoader,
    private val statusBarManager: StatusBarManager,
) {
    private val mediaProcessingJobs = ConcurrentHashMap<JobKey, Job>()
    private val mediaProcessingJobs = ConcurrentHashMap<String, Job>()

    private val artworkWidth: Int =
        context.resources.getDimensionPixelSize(
@@ -97,7 +97,7 @@ constructor(
    private val themeText =
        com.android.settingslib.Utils.getColorAttr(
                context,
                com.android.internal.R.attr.textColorPrimary
                com.android.internal.R.attr.textColorPrimary,
            )
            .defaultColor

@@ -112,11 +112,14 @@ constructor(
     * load will be cancelled.
     */
    suspend fun loadMediaData(key: String, sbn: StatusBarNotification): MediaDataLoaderResult? {
        logD(TAG) { "Loading media data for $key..." }
        val jobKey = JobKey(key, sbn)
        val loadMediaJob = backgroundScope.async { loadMediaDataInBackground(key, sbn) }
        loadMediaJob.invokeOnCompletion { mediaProcessingJobs.remove(jobKey) }
        val existingJob = mediaProcessingJobs.put(jobKey, loadMediaJob)
        loadMediaJob.invokeOnCompletion {
            // We need to make sure we're removing THIS job after cancellation, not
            // a job that we created later.
            mediaProcessingJobs.remove(key, loadMediaJob)
        }
        val existingJob = mediaProcessingJobs.put(key, loadMediaJob)
        logD(TAG) { "Loading media data for $key... / existing job: $existingJob" }
        existingJob?.cancel("New processing job incoming.")
        return loadMediaJob.await()
    }
@@ -128,10 +131,15 @@ constructor(
        sbn: StatusBarNotification,
    ): MediaDataLoaderResult? =
        traceCoroutine("MediaDataLoader#loadMediaData") {
            // We have apps spamming us with quick notification updates which can cause
            // us to spend significant CPU time loading duplicate data. This debounces
            // those requests at the cost of a bit of latency.
            delay(DEBOUNCE_DELAY_MS)

            val token =
                sbn.notification.extras.getParcelable(
                    Notification.EXTRA_MEDIA_SESSION,
                    MediaSession.Token::class.java
                    MediaSession.Token::class.java,
                )
            if (token == null) {
                Log.i(TAG, "Token was null, not loading media info")
@@ -144,7 +152,7 @@ constructor(
            val appInfo =
                notification.extras.getParcelable(
                    Notification.EXTRA_BUILDER_APPLICATION_INFO,
                    ApplicationInfo::class.java
                    ApplicationInfo::class.java,
                ) ?: getAppInfoFromPackage(sbn.packageName)

            // App name
@@ -240,7 +248,7 @@ constructor(
                playbackLocation = playbackLocation,
                isPlaying = isPlaying,
                appUid = appUid,
                isExplicit = isExplicit
                isExplicit = isExplicit,
            )
        }

@@ -258,7 +266,7 @@ constructor(
        token: MediaSession.Token,
        appName: String,
        appIntent: PendingIntent,
        packageName: String
        packageName: String,
    ): MediaDataLoaderResult? {
        val mediaData =
            backgroundScope.async {
@@ -270,7 +278,7 @@ constructor(
                    token,
                    appName,
                    appIntent,
                    packageName
                    packageName,
                )
            }
        return mediaData.await()
@@ -286,7 +294,7 @@ constructor(
        token: MediaSession.Token,
        appName: String,
        appIntent: PendingIntent,
        packageName: String
        packageName: String,
    ): MediaDataLoaderResult? =
        traceCoroutine("MediaDataLoader#loadMediaDataForResumption") {
            if (desc.title.isNullOrBlank()) {
@@ -338,14 +346,14 @@ constructor(
                appUid = appUid,
                isExplicit = isExplicit,
                resumeAction = resumeAction,
                resumeProgress = progress
                resumeProgress = progress,
            )
        }

    private fun createActionsFromState(
        packageName: String,
        controller: MediaController,
        user: UserHandle
        user: UserHandle,
    ): MediaButton? {
        if (!mediaFlags.areMediaSessionActionsEnabled(packageName, user)) {
            return null
@@ -368,7 +376,7 @@ constructor(
     */
    private fun getDeviceInfoForRemoteCast(
        key: String,
        sbn: StatusBarNotification
        sbn: StatusBarNotification,
    ): MediaDeviceData? {
        val extras = sbn.notification.extras
        val deviceName = extras.getCharSequence(Notification.EXTRA_MEDIA_REMOTE_DEVICE, null)
@@ -388,7 +396,7 @@ constructor(
                deviceDrawable,
                deviceName,
                deviceIntent,
                showBroadcastButton = false
                showBroadcastButton = false,
            )
        }
        return null
@@ -439,7 +447,7 @@ constructor(
                listOf(
                    ContentResolver.SCHEME_CONTENT,
                    ContentResolver.SCHEME_ANDROID_RESOURCE,
                    ContentResolver.SCHEME_FILE
                    ContentResolver.SCHEME_FILE,
                )
        ) {
            Log.w(TAG, "Invalid album art uri $uri")
@@ -451,7 +459,7 @@ constructor(
            source,
            artworkWidth,
            artworkHeight,
            allocator = ImageDecoder.ALLOCATOR_SOFTWARE
            allocator = ImageDecoder.ALLOCATOR_SOFTWARE,
        )
    }

@@ -459,7 +467,7 @@ constructor(
        uri: Uri,
        userId: Int,
        appUid: Int,
        packageName: String
        packageName: String,
    ): Bitmap? {
        try {
            val ugm = UriGrantsManager.getService()
@@ -468,7 +476,7 @@ constructor(
                packageName,
                ContentProvider.getUriWithoutUserId(uri),
                Intent.FLAG_GRANT_READ_URI_PERMISSION,
                ContentProvider.getUserIdFromUri(uri, userId)
                ContentProvider.getUserIdFromUri(uri, userId),
            )
            return loadBitmapFromUri(uri)
        } catch (e: SecurityException) {
@@ -488,21 +496,20 @@ constructor(
                .loadDrawable(context),
            action,
            context.getString(R.string.controls_media_resume),
            context.getDrawable(R.drawable.ic_media_play_container)
            context.getDrawable(R.drawable.ic_media_play_container),
        )
    }

    private data class JobKey(val key: String, val sbn: StatusBarNotification) :
        Pair<String, StatusBarNotification>(key, sbn)

    companion object {
        private const val TAG = "MediaDataLoader"
        private val ART_URIS =
            arrayOf(
                MediaMetadata.METADATA_KEY_ALBUM_ART_URI,
                MediaMetadata.METADATA_KEY_ART_URI,
                MediaMetadata.METADATA_KEY_DISPLAY_ICON_URI
                MediaMetadata.METADATA_KEY_DISPLAY_ICON_URI,
            )

        private const val DEBOUNCE_DELAY_MS = 200L
    }

    /** Returned data from loader. */
@@ -523,6 +530,6 @@ constructor(
        val appUid: Int,
        val isExplicit: Boolean,
        val resumeAction: Runnable? = null,
        val resumeProgress: Double? = null
        val resumeProgress: Double? = null,
    )
}
+80 −79

File changed.

Preview size limit exceeded, changes collapsed.

+79 −77

File changed.

Preview size limit exceeded, changes collapsed.