Merge "Cancel notification if media title is invalid" into udc-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
321f474e52
@@ -43,6 +43,7 @@ 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,6 +53,7 @@ import android.util.Log
|
||||
import android.util.Pair as APair
|
||||
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
|
||||
@@ -137,6 +139,8 @@ 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()
|
||||
}
|
||||
@@ -181,6 +185,7 @@ class MediaDataManager(
|
||||
private val logger: MediaUiEventLogger,
|
||||
private val smartspaceManager: SmartspaceManager,
|
||||
private val keyguardUpdateMonitor: KeyguardUpdateMonitor,
|
||||
private val statusBarService: IStatusBarService,
|
||||
) : Dumpable, BcSmartspaceDataPlugin.SmartspaceTargetListener {
|
||||
|
||||
companion object {
|
||||
@@ -252,6 +257,7 @@ class MediaDataManager(
|
||||
mediaFlags: MediaFlags,
|
||||
logger: MediaUiEventLogger,
|
||||
smartspaceManager: SmartspaceManager,
|
||||
statusBarService: IStatusBarService,
|
||||
keyguardUpdateMonitor: KeyguardUpdateMonitor,
|
||||
) : this(
|
||||
context,
|
||||
@@ -277,6 +283,7 @@ class MediaDataManager(
|
||||
logger,
|
||||
smartspaceManager,
|
||||
keyguardUpdateMonitor,
|
||||
statusBarService,
|
||||
)
|
||||
|
||||
private val appChangeReceiver =
|
||||
@@ -378,21 +385,21 @@ class MediaDataManager(
|
||||
|
||||
fun onNotificationAdded(key: String, sbn: StatusBarNotification) {
|
||||
if (useQsMediaPlayer && isMediaNotification(sbn)) {
|
||||
var logEvent = false
|
||||
var isNewlyActiveEntry = 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)
|
||||
logEvent = true
|
||||
isNewlyActiveEntry = true
|
||||
} else if (oldKey != key) {
|
||||
// Resume -> active conversion; move to new key
|
||||
val oldData = mediaEntries.remove(oldKey)!!
|
||||
logEvent = true
|
||||
isNewlyActiveEntry = true
|
||||
mediaEntries.put(key, oldData)
|
||||
}
|
||||
loadMediaData(key, sbn, oldKey, logEvent)
|
||||
loadMediaData(key, sbn, oldKey, isNewlyActiveEntry)
|
||||
} else {
|
||||
onNotificationRemoved(key)
|
||||
}
|
||||
@@ -475,9 +482,9 @@ class MediaDataManager(
|
||||
key: String,
|
||||
sbn: StatusBarNotification,
|
||||
oldKey: String?,
|
||||
logEvent: Boolean = false
|
||||
isNewlyActiveEntry: Boolean = false,
|
||||
) {
|
||||
backgroundExecutor.execute { loadMediaDataInBg(key, sbn, oldKey, logEvent) }
|
||||
backgroundExecutor.execute { loadMediaDataInBg(key, sbn, oldKey, isNewlyActiveEntry) }
|
||||
}
|
||||
|
||||
/** Add a listener for changes in this class */
|
||||
@@ -601,9 +608,11 @@ class MediaDataManager(
|
||||
}
|
||||
}
|
||||
|
||||
private fun removeEntry(key: String) {
|
||||
private fun removeEntry(key: String, logEvent: Boolean = true) {
|
||||
mediaEntries.remove(key)?.let {
|
||||
logger.logMediaRemoved(it.appUid, it.packageName, it.instanceId)
|
||||
if (logEvent) {
|
||||
logger.logMediaRemoved(it.appUid, it.packageName, it.instanceId)
|
||||
}
|
||||
}
|
||||
notifyMediaDataRemoved(key)
|
||||
}
|
||||
@@ -751,7 +760,7 @@ class MediaDataManager(
|
||||
key: String,
|
||||
sbn: StatusBarNotification,
|
||||
oldKey: String?,
|
||||
logEvent: Boolean = false
|
||||
isNewlyActiveEntry: Boolean = false,
|
||||
) {
|
||||
val token =
|
||||
sbn.notification.extras.getParcelable(
|
||||
@@ -765,6 +774,34 @@ 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,
|
||||
@@ -793,15 +830,6 @@ 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()) {
|
||||
@@ -873,7 +901,7 @@ class MediaDataManager(
|
||||
val instanceId = currentEntry?.instanceId ?: logger.getNewInstanceId()
|
||||
val appUid = appInfo?.uid ?: Process.INVALID_UID
|
||||
|
||||
if (logEvent) {
|
||||
if (isNewlyActiveEntry) {
|
||||
logSingleVsMultipleMediaAdded(appUid, sbn.packageName, instanceId)
|
||||
logger.logActiveMediaAdded(appUid, sbn.packageName, instanceId, playbackLocation)
|
||||
} else if (playbackLocation != currentEntry?.playbackLocation) {
|
||||
|
||||
@@ -40,6 +40,7 @@ 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
|
||||
@@ -130,6 +131,7 @@ 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
|
||||
@@ -192,7 +194,8 @@ class MediaDataManagerTest : SysuiTestCase() {
|
||||
mediaFlags = mediaFlags,
|
||||
logger = logger,
|
||||
smartspaceManager = smartspaceManager,
|
||||
keyguardUpdateMonitor = keyguardUpdateMonitor
|
||||
keyguardUpdateMonitor = keyguardUpdateMonitor,
|
||||
statusBarService = statusBarService,
|
||||
)
|
||||
verify(tunerService)
|
||||
.addTunable(capture(tunableCaptor), eq(Settings.Secure.MEDIA_CONTROLS_RECOMMENDATION))
|
||||
@@ -517,19 +520,136 @@ class MediaDataManagerTest : SysuiTestCase() {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testOnNotificationRemoved_emptyTitle_notConverted() {
|
||||
// GIVEN that the manager has a notification with a resume action and empty title.
|
||||
fun testOnNotificationAdded_emptyTitle_notLoaded() {
|
||||
// GIVEN that the manager has a notification with an 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(resumeAction = Runnable {}))
|
||||
mediaDataManager.onMediaDataLoaded(
|
||||
KEY,
|
||||
null,
|
||||
data.copy(song = SESSION_EMPTY_TITLE, resumeAction = Runnable {})
|
||||
)
|
||||
|
||||
// WHEN the notification is removed
|
||||
reset(listener)
|
||||
@@ -554,17 +674,15 @@ 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(resumeAction = Runnable {}))
|
||||
mediaDataManager.onMediaDataLoaded(
|
||||
KEY,
|
||||
null,
|
||||
data.copy(song = SESSION_BLANK_TITLE, resumeAction = Runnable {})
|
||||
)
|
||||
|
||||
// WHEN the notification is removed
|
||||
reset(listener)
|
||||
|
||||
Reference in New Issue
Block a user