Merge "Require title for media controls" into tm-qpr-dev am: 92e6c550a1

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21489153

Change-Id: I8801a9db537396cb4e3b5b0d0a9c4fa162ad1c29
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

@@ -665,7 +665,7 @@ class MediaDataManager(
         appIntent: PendingIntent,
         packageName: String
     ) {
-        if (TextUtils.isEmpty(desc.title)) {
+        if (desc.title.isNullOrBlank()) {
             Log.e(TAG, "Description incomplete")
             // Delete the placeholder entry
             mediaEntries.remove(packageName)
@@ -1405,6 +1405,13 @@ class MediaDataManager(
     /** Set the given [MediaData] as a resume state player and notify listeners */
     private fun convertToResumePlayer(key: String, data: MediaData) {
         if (DEBUG) Log.d(TAG, "Converting $key to resume")
+        // Resumption controls must have a title.
+        if (data.song.isNullOrBlank()) {
+            Log.e(TAG, "Description incomplete")
+            notifyMediaDataRemoved(key)
+            logger.logMediaRemoved(data.appUid, data.packageName, data.instanceId)
+            return
+        }
         // Move to resume key (aka package name) if that key doesn't already exist.
         val resumeAction = data.resumeAction?.let { getResumeMediaAction(it) }
         val actions = resumeAction?.let { listOf(resumeAction) } ?: emptyList()

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

@@ -93,6 +93,8 @@ private const val SYSTEM_PACKAGE_NAME = "com.android.systemui"
 private const val APP_NAME = "SystemUI"
 private const val SESSION_ARTIST = "artist"
 private const val SESSION_TITLE = "title"
+private const val SESSION_BLANK_TITLE = " "
+private const val SESSION_EMPTY_TITLE = ""
 private const val USER_ID = 0
 private val DISMISS_INTENT = Intent().apply { action = "dismiss" }
 
@@ -214,6 +216,7 @@ class MediaDataManagerTest : SysuiTestCase() {
         whenever(mediaControllerFactory.create(eq(session.sessionToken))).thenReturn(controller)
         whenever(controller.transportControls).thenReturn(transportControls)
         whenever(controller.playbackInfo).thenReturn(playbackInfo)
+        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         whenever(playbackInfo.playbackType)
             .thenReturn(MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL)
 
@@ -318,18 +321,15 @@ class MediaDataManagerTest : SysuiTestCase() {
 
     @Test
     fun testLoadMetadata_withExplicitIndicator() {
-        val metadata =
+        whenever(controller.metadata)
-            MediaMetadata.Builder().run {
+            .thenReturn(
-                putString(MediaMetadata.METADATA_KEY_ARTIST, SESSION_ARTIST)
+                metadataBuilder
-                putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_TITLE)
+                    .putLong(
-                putLong(
                         MediaConstants.METADATA_KEY_IS_EXPLICIT,
                         MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT
                     )
-                build()
+                    .build()
-            }
+            )
-        whenever(mediaControllerFactory.create(anyObject())).thenReturn(controller)
-        whenever(controller.metadata).thenReturn(metadata)
 
         mediaDataManager.addListener(listener)
         mediaDataManager.onNotificationAdded(KEY, mediaNotification)
@@ -350,9 +350,6 @@ class MediaDataManagerTest : SysuiTestCase() {
 
     @Test
     fun testOnMetaDataLoaded_withoutExplicitIndicator() {
-        whenever(mediaControllerFactory.create(anyObject())).thenReturn(controller)
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
-
         mediaDataManager.addListener(listener)
         mediaDataManager.onNotificationAdded(KEY, mediaNotification)
 
@@ -385,7 +382,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @Test
     fun testOnMetaDataLoaded_conservesActiveFlag() {
         whenever(mediaControllerFactory.create(anyObject())).thenReturn(controller)
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         mediaDataManager.addListener(listener)
         mediaDataManager.onNotificationAdded(KEY, mediaNotification)
         assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
@@ -529,10 +525,79 @@ class MediaDataManagerTest : SysuiTestCase() {
         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.
+        whenever(controller.metadata)
+            .thenReturn(
+                metadataBuilder
+                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_EMPTY_TITLE)
+                    .build()
+            )
+        addNotificationAndLoad()
+        val data = mediaDataCaptor.value
+        val instanceId = data.instanceId
+        assertThat(data.resumption).isFalse()
+        mediaDataManager.onMediaDataLoaded(KEY, null, data.copy(resumeAction = Runnable {}))
+
+        // WHEN the notification is removed
+        reset(listener)
+        mediaDataManager.onNotificationRemoved(KEY)
+
+        // THEN active media is not converted to resume.
+        verify(listener, never())
+            .onMediaDataLoaded(
+                eq(PACKAGE_NAME),
+                eq(KEY),
+                capture(mediaDataCaptor),
+                eq(true),
+                eq(0),
+                eq(false)
+            )
+        verify(logger, never())
+            .logActiveConvertedToResume(anyInt(), eq(PACKAGE_NAME), eq(instanceId))
+        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())
+        verify(logger).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), eq(instanceId))
+    }
+
+    @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 {}))
+
+        // WHEN the notification is removed
+        reset(listener)
+        mediaDataManager.onNotificationRemoved(KEY)
+
+        // THEN active media is not converted to resume.
+        verify(listener, never())
+            .onMediaDataLoaded(
+                eq(PACKAGE_NAME),
+                eq(KEY),
+                capture(mediaDataCaptor),
+                eq(true),
+                eq(0),
+                eq(false)
+            )
+        verify(logger, never())
+            .logActiveConvertedToResume(anyInt(), eq(PACKAGE_NAME), eq(instanceId))
+        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())
+        verify(logger).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), eq(instanceId))
+    }
+
     @Test
     fun testOnNotificationRemoved_withResumption() {
         // GIVEN that the manager has a notification with a resume action
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         addNotificationAndLoad()
         val data = mediaDataCaptor.value
         assertThat(data.resumption).isFalse()
@@ -557,7 +622,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @Test
     fun testOnNotificationRemoved_twoWithResumption() {
         // GIVEN that the manager has two notifications with resume actions
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         mediaDataManager.onNotificationAdded(KEY, mediaNotification)
         mediaDataManager.onNotificationAdded(KEY_2, mediaNotification)
         assertThat(backgroundExecutor.runAllReady()).isEqualTo(2)
@@ -623,7 +687,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @Test
     fun testOnNotificationRemoved_withResumption_butNotLocal() {
         // GIVEN that the manager has a notification with a resume action, but is not local
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         whenever(playbackInfo.playbackType)
             .thenReturn(MediaController.PlaybackInfo.PLAYBACK_TYPE_REMOTE)
         addNotificationAndLoad()
@@ -660,7 +723,6 @@ class MediaDataManagerTest : SysuiTestCase() {
         }
 
         // And an active, resumable notification
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         addNotificationAndLoad()
         val data = mediaDataCaptor.value
         assertThat(data.resumption).isFalse()
@@ -844,6 +906,74 @@ class MediaDataManagerTest : SysuiTestCase() {
         assertThat(data.resumeProgress).isEqualTo(null)
     }
 
+    @Test
+    fun testAddResumptionControls_hasEmptyTitle() {
+        whenever(mediaFlags.isResumeProgressEnabled()).thenReturn(true)
+
+        // WHEN resumption controls are added that have empty title
+        val desc =
+            MediaDescription.Builder().run {
+                setTitle(SESSION_EMPTY_TITLE)
+                build()
+            }
+        mediaDataManager.addResumptionControls(
+            USER_ID,
+            desc,
+            Runnable {},
+            session.sessionToken,
+            APP_NAME,
+            pendingIntent,
+            PACKAGE_NAME
+        )
+
+        // Resumption controls are not added.
+        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
+        assertThat(foregroundExecutor.runAllReady()).isEqualTo(0)
+        verify(listener, never())
+            .onMediaDataLoaded(
+                eq(PACKAGE_NAME),
+                eq(null),
+                capture(mediaDataCaptor),
+                eq(true),
+                eq(0),
+                eq(false)
+            )
+    }
+
+    @Test
+    fun testAddResumptionControls_hasBlankTitle() {
+        whenever(mediaFlags.isResumeProgressEnabled()).thenReturn(true)
+
+        // WHEN resumption controls are added that have a blank title
+        val desc =
+            MediaDescription.Builder().run {
+                setTitle(SESSION_BLANK_TITLE)
+                build()
+            }
+        mediaDataManager.addResumptionControls(
+            USER_ID,
+            desc,
+            Runnable {},
+            session.sessionToken,
+            APP_NAME,
+            pendingIntent,
+            PACKAGE_NAME
+        )
+
+        // Resumption controls are not added.
+        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
+        assertThat(foregroundExecutor.runAllReady()).isEqualTo(0)
+        verify(listener, never())
+            .onMediaDataLoaded(
+                eq(PACKAGE_NAME),
+                eq(null),
+                capture(mediaDataCaptor),
+                eq(true),
+                eq(0),
+                eq(false)
+            )
+    }
+
     @Test
     fun testResumptionDisabled_dismissesResumeControls() {
         // WHEN there are resume controls and resumption is switched off
@@ -1213,7 +1343,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @Test
     fun testOnActiveMediaConverted_doesNotUpdateLastActiveTime() {
         // GIVEN that the manager has a notification with a resume action
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         addNotificationAndLoad()
         val data = mediaDataCaptor.value
         val instanceId = data.instanceId
@@ -1513,7 +1642,6 @@ class MediaDataManagerTest : SysuiTestCase() {
         val instanceId = mediaDataCaptor.value.instanceId
 
         // Location is updated to local cast
-        whenever(controller.metadata).thenReturn(metadataBuilder.build())
         whenever(playbackInfo.playbackType)
             .thenReturn(MediaController.PlaybackInfo.PLAYBACK_TYPE_REMOTE)
         addNotificationAndLoad()
@@ -1589,7 +1717,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @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()