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

Commit 1c887bab authored by Westlee Latta's avatar Westlee Latta Committed by Android (Google) Code Review
Browse files

Revert "Cancel notification if media title is invalid" 

This reverts commit 381f6d61.

Reason for revert: This change caused b/278354453 and potentially other  unintended stability issues for 1P partners. The originating team has proposed alternative solutions without the negative impacts in https://b.corp.google.com/issues/274775190#comment23 and DevRel will review the proposal for the next steps.

Change-Id: I28846b7d775be9d724fad4d9a4414b5781ffb951
parent 381f6d61

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

+19 −47
Original line number Diff line number Diff line
@@ -43,7 +43,6 @@ import android.media.session.PlaybackState 
import android.net.Uri

import android.os.Parcelable

import android.os.Process

import android.os.RemoteException

import android.os.UserHandle

import android.provider.Settings

import android.service.notification.StatusBarNotification
@@ -52,7 +51,6 @@ import android.text.TextUtils 
import android.util.Log

import androidx.media.utils.MediaConstants

import com.android.internal.logging.InstanceId

import com.android.internal.statusbar.IStatusBarService

import com.android.keyguard.KeyguardUpdateMonitor

import com.android.systemui.Dumpable

import com.android.systemui.R
@@ -138,8 +136,6 @@ internal val EMPTY_SMARTSPACE_MEDIA_DATA = 
        expiryTimeMs = 0,

    )



const val MEDIA_TITLE_ERROR_MESSAGE = "Invalid media data: title is null or blank."



fun isMediaNotification(sbn: StatusBarNotification): Boolean {

    return sbn.notification.isMediaNotification()

}
@@ -184,7 +180,6 @@ class MediaDataManager( 
    private val logger: MediaUiEventLogger,

    private val smartspaceManager: SmartspaceManager,

    private val keyguardUpdateMonitor: KeyguardUpdateMonitor,

    private val statusBarService: IStatusBarService,

) : Dumpable, BcSmartspaceDataPlugin.SmartspaceTargetListener {



    companion object {
@@ -249,7 +244,6 @@ class MediaDataManager( 
        mediaFlags: MediaFlags,

        logger: MediaUiEventLogger,

        smartspaceManager: SmartspaceManager,

        statusBarService: IStatusBarService,

        keyguardUpdateMonitor: KeyguardUpdateMonitor,

    ) : this(

        context,
@@ -275,7 +269,6 @@ class MediaDataManager( 
        logger,

        smartspaceManager,

        keyguardUpdateMonitor,

        statusBarService,

    )



    private val appChangeReceiver =
@@ -377,21 +370,21 @@ class MediaDataManager( 


    fun onNotificationAdded(key: String, sbn: StatusBarNotification) {

        if (useQsMediaPlayer && isMediaNotification(sbn)) {

            var isNewlyActiveEntry = false

            var logEvent = false

            Assert.isMainThread()

            val oldKey = findExistingEntry(key, sbn.packageName)

            if (oldKey == null) {

                val instanceId = logger.getNewInstanceId()

                val temp = LOADING.copy(packageName = sbn.packageName, instanceId = instanceId)

                mediaEntries.put(key, temp)

                isNewlyActiveEntry = true

                logEvent = true

            } else if (oldKey != key) {

                // Resume -> active conversion; move to new key

                val oldData = mediaEntries.remove(oldKey)!!

                isNewlyActiveEntry = true

                logEvent = true

                mediaEntries.put(key, oldData)

            }

            loadMediaData(key, sbn, oldKey, isNewlyActiveEntry)

            loadMediaData(key, sbn, oldKey, logEvent)

        } else {

            onNotificationRemoved(key)

        }
@@ -474,9 +467,9 @@ class MediaDataManager( 
        key: String,

        sbn: StatusBarNotification,

        oldKey: String?,

        isNewlyActiveEntry: Boolean = false,

        logEvent: Boolean = false

    ) {

        backgroundExecutor.execute { loadMediaDataInBg(key, sbn, oldKey, isNewlyActiveEntry) }

        backgroundExecutor.execute { loadMediaDataInBg(key, sbn, oldKey, logEvent) }

    }



    /** Add a listener for changes in this class */
@@ -600,12 +593,10 @@ class MediaDataManager( 
        }

    }



    private fun removeEntry(key: String, logEvent: Boolean = true) {

    private fun removeEntry(key: String) {

        mediaEntries.remove(key)?.let {

            if (logEvent) {

            logger.logMediaRemoved(it.appUid, it.packageName, it.instanceId)

        }

        }

        notifyMediaDataRemoved(key)

    }
@@ -752,7 +743,7 @@ class MediaDataManager( 
        key: String,

        sbn: StatusBarNotification,

        oldKey: String?,

        isNewlyActiveEntry: Boolean = false,

        logEvent: Boolean = false

    ) {

        val token =

            sbn.notification.extras.getParcelable(
@@ -766,34 +757,6 @@ class MediaDataManager( 
        val metadata = mediaController.metadata

        val notif: Notification = sbn.notification



        // Song name

        var song: CharSequence? = metadata?.getString(MediaMetadata.METADATA_KEY_DISPLAY_TITLE)

        if (song == null) {

            song = metadata?.getString(MediaMetadata.METADATA_KEY_TITLE)

        }

        if (song == null) {

            song = HybridGroupManager.resolveTitle(notif)

        }

        // Media data must have a title.

        if (song.isNullOrBlank()) {

            try {

                statusBarService.onNotificationError(

                    sbn.packageName,

                    sbn.tag,

                    sbn.id,

                    sbn.uid,

                    sbn.initialPid,

                    MEDIA_TITLE_ERROR_MESSAGE,

                    sbn.user.identifier

                )

            } catch (e: RemoteException) {

                Log.e(TAG, "cancelNotification failed: $e")

            }

            // Only add log for media removed if active media is updated with invalid title.

            foregroundExecutor.execute { removeEntry(key, !isNewlyActiveEntry) }

            return

        }



        val appInfo =

            notif.extras.getParcelable(

                Notification.EXTRA_BUILDER_APPLICATION_INFO,
@@ -822,6 +785,15 @@ class MediaDataManager( 
        // App Icon

        val smallIcon = sbn.notification.smallIcon



        // Song name

        var song: CharSequence? = metadata?.getString(MediaMetadata.METADATA_KEY_DISPLAY_TITLE)

        if (song == null) {

            song = metadata?.getString(MediaMetadata.METADATA_KEY_TITLE)

        }

        if (song == null) {

            song = HybridGroupManager.resolveTitle(notif)

        }



        // Explicit Indicator

        var isExplicit = false

        if (mediaFlags.isExplicitIndicatorEnabled()) {
@@ -893,7 +865,7 @@ class MediaDataManager( 
        val instanceId = currentEntry?.instanceId ?: logger.getNewInstanceId()

        val appUid = appInfo?.uid ?: Process.INVALID_UID



        if (isNewlyActiveEntry) {

        if (logEvent) {

            logSingleVsMultipleMediaAdded(appUid, sbn.packageName, instanceId)

            logger.logActiveMediaAdded(appUid, sbn.packageName, instanceId, playbackLocation)

        } else if (playbackLocation != currentEntry?.playbackLocation) {

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

+11 −129
Original line number Diff line number Diff line
@@ -40,7 +40,6 @@ import android.testing.TestableLooper.RunWithLooper 
import androidx.media.utils.MediaConstants

import androidx.test.filters.SmallTest

import com.android.internal.logging.InstanceId

import com.android.internal.statusbar.IStatusBarService

import com.android.keyguard.KeyguardUpdateMonitor

import com.android.systemui.InstanceIdSequenceFake

import com.android.systemui.R
@@ -131,7 +130,6 @@ class MediaDataManagerTest : SysuiTestCase() { 
    @Mock lateinit var activityStarter: ActivityStarter

    @Mock lateinit var smartspaceManager: SmartspaceManager

    @Mock lateinit var keyguardUpdateMonitor: KeyguardUpdateMonitor

    @Mock lateinit var statusBarService: IStatusBarService

    lateinit var smartspaceMediaDataProvider: SmartspaceMediaDataProvider

    @Mock lateinit var mediaSmartspaceTarget: SmartspaceTarget

    @Mock private lateinit var mediaRecommendationItem: SmartspaceAction
@@ -194,8 +192,7 @@ class MediaDataManagerTest : SysuiTestCase() { 
                mediaFlags = mediaFlags,

                logger = logger,

                smartspaceManager = smartspaceManager,

                keyguardUpdateMonitor = keyguardUpdateMonitor,

                statusBarService = statusBarService,

                keyguardUpdateMonitor = keyguardUpdateMonitor

            )

        verify(tunerService)

            .addTunable(capture(tunableCaptor), eq(Settings.Secure.MEDIA_CONTROLS_RECOMMENDATION))
@@ -520,136 +517,19 @@ class MediaDataManagerTest : SysuiTestCase() { 
    }



    @Test

    fun testOnNotificationAdded_emptyTitle_notLoaded() {

        // GIVEN that the manager has a notification with an empty title.

    fun testOnNotificationRemoved_emptyTitle_notConverted() {

        // GIVEN that the manager has a notification with a resume action and empty title.

        whenever(controller.metadata)

            .thenReturn(

                metadataBuilder

                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_EMPTY_TITLE)

                    .build()

            )

        mediaDataManager.onNotificationAdded(KEY, mediaNotification)



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

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

        verify(statusBarService)

            .onNotificationError(

                eq(PACKAGE_NAME),

                eq(mediaNotification.tag),

                eq(mediaNotification.id),

                eq(mediaNotification.uid),

                eq(mediaNotification.initialPid),

                eq(MEDIA_TITLE_ERROR_MESSAGE),

                eq(mediaNotification.user.identifier)

            )

        verify(listener, never())

            .onMediaDataLoaded(

                eq(KEY),

                eq(null),

                capture(mediaDataCaptor),

                eq(true),

                eq(0),

                eq(false)

            )

        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())

        verify(logger, never()).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), any())

    }



    @Test

    fun testOnNotificationAdded_blankTitle_notLoaded() {

        // GIVEN that the manager has a notification with a blank title.

        whenever(controller.metadata)

            .thenReturn(

                metadataBuilder

                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_BLANK_TITLE)

                    .build()

            )

        mediaDataManager.onNotificationAdded(KEY, mediaNotification)



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

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

        verify(statusBarService)

            .onNotificationError(

                eq(PACKAGE_NAME),

                eq(mediaNotification.tag),

                eq(mediaNotification.id),

                eq(mediaNotification.uid),

                eq(mediaNotification.initialPid),

                eq(MEDIA_TITLE_ERROR_MESSAGE),

                eq(mediaNotification.user.identifier)

            )

        verify(listener, never())

            .onMediaDataLoaded(

                eq(KEY),

                eq(null),

                capture(mediaDataCaptor),

                eq(true),

                eq(0),

                eq(false)

            )

        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())

        verify(logger, never()).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), any())

    }



    @Test

    fun testOnNotificationUpdated_invalidTitle_logMediaRemoved() {

        addNotificationAndLoad()

        val data = mediaDataCaptor.value



        verify(listener)

            .onMediaDataLoaded(

                eq(KEY),

                eq(null),

                capture(mediaDataCaptor),

                eq(true),

                eq(0),

                eq(false)

            )



        reset(listener)

        whenever(controller.metadata)

            .thenReturn(

                metadataBuilder

                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_BLANK_TITLE)

                    .build()

            )

        mediaDataManager.onNotificationAdded(KEY, mediaNotification)

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

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

        verify(statusBarService)

            .onNotificationError(

                eq(PACKAGE_NAME),

                eq(mediaNotification.tag),

                eq(mediaNotification.id),

                eq(mediaNotification.uid),

                eq(mediaNotification.initialPid),

                eq(MEDIA_TITLE_ERROR_MESSAGE),

                eq(mediaNotification.user.identifier)

            )

        verify(listener, never())

            .onMediaDataLoaded(

                eq(KEY),

                eq(null),

                capture(mediaDataCaptor),

                eq(true),

                eq(0),

                eq(false)

            )

        verify(logger).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), eq(data.instanceId))

    }



    @Test

    fun testOnNotificationRemoved_emptyTitle_notConverted() {

        // GIVEN that the manager has a notification with a resume action and empty title.

        addNotificationAndLoad()

        val data = mediaDataCaptor.value

        val instanceId = data.instanceId

        assertThat(data.resumption).isFalse()

        mediaDataManager.onMediaDataLoaded(

            KEY,

            null,

            data.copy(song = SESSION_EMPTY_TITLE, resumeAction = Runnable {})

        )

        mediaDataManager.onMediaDataLoaded(KEY, null, data.copy(resumeAction = Runnable {}))



        // WHEN the notification is removed

        reset(listener)
@@ -674,15 +554,17 @@ class MediaDataManagerTest : SysuiTestCase() { 
    @Test

    fun testOnNotificationRemoved_blankTitle_notConverted() {

        // GIVEN that the manager has a notification with a resume action and blank title.

        whenever(controller.metadata)

            .thenReturn(

                metadataBuilder

                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_BLANK_TITLE)

                    .build()

            )

        addNotificationAndLoad()

        val data = mediaDataCaptor.value

        val instanceId = data.instanceId

        assertThat(data.resumption).isFalse()

        mediaDataManager.onMediaDataLoaded(

            KEY,

            null,

            data.copy(song = SESSION_BLANK_TITLE, resumeAction = Runnable {})

        )

        mediaDataManager.onMediaDataLoaded(KEY, null, data.copy(resumeAction = Runnable {}))



        // WHEN the notification is removed

        reset(listener)