diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt index b09f201e237b8..4cc041074a8d8 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt @@ -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,11 +593,9 @@ 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) - } + 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) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt index 0a1db60e075cb..d428db7b9ddae 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt @@ -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)