Only show people icons for Notifications that are MessagingStyle

Instead of only checking that the channel is used for important conversations. Note that this will also prevent the shortcut or large icon from being displayed in that case, but that seems to be the intent of the code (naming like isImportantConversation, showPeopleAvatar, etc).

Also several non-logic changes, especially in the test file (formatting, fixing inverted assertEquals, named arguments, etc).

Fixes: 295816841
Test: Unit test
Change-Id: I7e0bced9207576889f9c092ee41158ac8cf78246

2 files changed, 111 insertions, 82 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
index 9d953423ec28..98e167f84781 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
@@ -17,6 +17,7 @@
 package com.android.systemui.statusbar.notification.icon
 
 import android.app.Notification
+import android.app.Notification.MessagingStyle
 import android.app.Person
 import android.content.pm.LauncherApps
 import android.graphics.drawable.Icon
@@ -184,12 +185,10 @@ class IconManager @Inject constructor(
     }
 
     @Throws(InflationException::class)
-    private fun getIconDescriptors(
-        entry: NotificationEntry
-    ): Pair<StatusBarIcon, StatusBarIcon> {
-        val iconDescriptor = getIconDescriptor(entry, false /* redact */)
+    private fun getIconDescriptors(entry: NotificationEntry): Pair<StatusBarIcon, StatusBarIcon> {
+        val iconDescriptor = getIconDescriptor(entry, redact = false)
         val sensitiveDescriptor = if (entry.isSensitive) {
-            getIconDescriptor(entry, true /* redact */)
+            getIconDescriptor(entry, redact = true)
         } else {
             iconDescriptor
         }
@@ -197,10 +196,7 @@ class IconManager @Inject constructor(
     }
 
     @Throws(InflationException::class)
-    private fun getIconDescriptor(
-        entry: NotificationEntry,
-        redact: Boolean
-    ): StatusBarIcon {
+    private fun getIconDescriptor(entry: NotificationEntry, redact: Boolean): StatusBarIcon {
         val n = entry.sbn.notification
         val showPeopleAvatar = isImportantConversation(entry) && !redact
 
@@ -228,7 +224,8 @@ class IconManager @Inject constructor(
                 icon,
                 n.iconLevel,
                 n.number,
-                iconBuilder.getIconContentDescription(n))
+                iconBuilder.getIconContentDescription(n)
+            )
 
         // Cache if important conversation.
         if (isImportantConversation(entry)) {
@@ -267,8 +264,10 @@ class IconManager @Inject constructor(
         // Fall back to extract from message
         if (ic == null) {
             val extras: Bundle = entry.sbn.notification.extras
-            val messages = Notification.MessagingStyle.Message.getMessagesFromBundleArray(
-                    extras.getParcelableArray(Notification.EXTRA_MESSAGES))
+            val messages =
+                MessagingStyle.Message.getMessagesFromBundleArray(
+                    extras.getParcelableArray(Notification.EXTRA_MESSAGES)
+                )
             val user = extras.getParcelable<Person>(Notification.EXTRA_MESSAGING_PERSON)
             for (i in messages.indices.reversed()) {
                 val message = messages[i]
@@ -315,8 +314,11 @@ class IconManager @Inject constructor(
     }
 
     private fun isImportantConversation(entry: NotificationEntry): Boolean {
+        // Also verify that the Notification is MessagingStyle, since we're going to access
+        // MessagingStyle-specific data (EXTRA_MESSAGES, EXTRA_MESSAGING_PERSON).
         return entry.ranking.channel != null &&
                 entry.ranking.channel.isImportantConversation &&
+                entry.sbn.notification.isStyle(MessagingStyle::class.java) &&
                 entry.key !in unimportantConversationKeys
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/icon/IconManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/icon/IconManagerTest.kt
index b63e66f1ebe3..8b99811e3d5f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/icon/IconManagerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/icon/IconManagerTest.kt
@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-package com.android.systemui.statusbar.notification.icon;
+package com.android.systemui.statusbar.notification.icon
 
-import android.app.ActivityManager;
-import android.app.Notification;
+import android.app.ActivityManager
+import android.app.Notification
 import android.app.NotificationChannel
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.Person
@@ -25,9 +25,10 @@ import android.content.pm.LauncherApps
 import android.content.pm.ShortcutInfo
 import android.graphics.drawable.Drawable
 import android.graphics.drawable.Icon
+import android.os.Bundle
 import android.os.SystemClock
 import android.os.UserHandle
-import android.testing.AndroidTestingRunner;
+import android.testing.AndroidTestingRunner
 import androidx.test.InstrumentationRegistry
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
@@ -36,27 +37,27 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry
 import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
 import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
-import org.junit.Assert.assertEquals
+import com.google.common.truth.Truth.assertThat
 import org.junit.Before
 import org.junit.Test
-
-import org.junit.runner.RunWith;
+import org.junit.runner.RunWith
 import org.mockito.Mock
-import org.mockito.Mockito.`when`
 import org.mockito.Mockito.anyInt
+import org.mockito.Mockito.`when`
 import org.mockito.MockitoAnnotations
 
 @SmallTest
 @RunWith(AndroidTestingRunner::class)
-class IconManagerTest: SysuiTestCase() {
+class IconManagerTest : SysuiTestCase() {
     companion object {
-        private const val TEST_PACKAGE_NAME = "test";
-        private const val TEST_UID = 0;
-    }
+        private const val TEST_PACKAGE_NAME = "test"
 
+        private const val TEST_UID = 0
+    }
 
     private var id = 0
-    private val context = InstrumentationRegistry.getTargetContext();
+    private val context = InstrumentationRegistry.getTargetContext()
+
     @Mock private lateinit var shortcut: ShortcutInfo
     @Mock private lateinit var shortcutIc: Icon
     @Mock private lateinit var messageIc: Icon
@@ -89,106 +90,132 @@ class IconManagerTest: SysuiTestCase() {
 
     @Test
     fun testCreateIcons_importantConversation_shortcutIcon() {
-        val entry = notificationEntry(true, true, true)
+        val entry =
+            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = true)
         entry?.channel?.isImportantConversation = true
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, shortcutIc)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(shortcutIc)
     }
 
     @Test
     fun testCreateIcons_importantConversation_messageIcon() {
-        val entry = notificationEntry(false, true, true)
+        val entry =
+            notificationEntry(hasShortcut = false, hasMessageSenderIcon = true, hasLargeIcon = true)
         entry?.channel?.isImportantConversation = true
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, messageIc)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(messageIc)
     }
 
     @Test
     fun testCreateIcons_importantConversation_largeIcon() {
-        val entry = notificationEntry(false, false, true)
+        val entry =
+            notificationEntry(
+                hasShortcut = false,
+                hasMessageSenderIcon = false,
+                hasLargeIcon = true
+            )
         entry?.channel?.isImportantConversation = true
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, largeIc)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(largeIc)
     }
 
     @Test
     fun testCreateIcons_importantConversation_smallIcon() {
-        val entry = notificationEntry(false, false, false)
+        val entry =
+            notificationEntry(
+                hasShortcut = false,
+                hasMessageSenderIcon = false,
+                hasLargeIcon = false
+            )
         entry?.channel?.isImportantConversation = true
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, smallIc)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(smallIc)
+    }
+
+    @Test
+    fun testCreateIcons_importantConversationWithoutMessagingStyle() {
+        val entry =
+            notificationEntry(
+                hasShortcut = true,
+                hasMessageSenderIcon = true,
+                useMessagingStyle = false,
+                hasLargeIcon = true
+            )
+        entry?.channel?.isImportantConversation = true
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(smallIc)
     }
 
     @Test
     fun testCreateIcons_notImportantConversation() {
-        val entry = notificationEntry(true, true, true)
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, smallIc)
+        val entry =
+            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = true)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(smallIc)
     }
 
     @Test
     fun testCreateIcons_sensitiveImportantConversation() {
-        val entry = notificationEntry(true, false, false)
-        entry?.setSensitive(true, true);
+        val entry =
+            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = false)
+        entry?.setSensitive(true, true)
         entry?.channel?.isImportantConversation = true
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.statusBarIcon?.sourceIcon, shortcutIc)
-        assertEquals(entry?.icons?.shelfIcon?.sourceIcon, smallIc)
-        assertEquals(entry?.icons?.aodIcon?.sourceIcon, smallIc)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.statusBarIcon?.sourceIcon).isEqualTo(shortcutIc)
+        assertThat(entry?.icons?.shelfIcon?.sourceIcon).isEqualTo(smallIc)
+        assertThat(entry?.icons?.aodIcon?.sourceIcon).isEqualTo(smallIc)
     }
 
     @Test
     fun testUpdateIcons_sensitivityChange() {
-        val entry = notificationEntry(true, false, false)
+        val entry =
+            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = false)
         entry?.channel?.isImportantConversation = true
-        entry?.setSensitive(true, true);
-        entry?.let {
-            iconManager.createIcons(it)
-        }
-        assertEquals(entry?.icons?.aodIcon?.sourceIcon, smallIc)
-        entry?.setSensitive(false, false);
-        entry?.let {
-            iconManager.updateIcons(it)
-        }
-        assertEquals(entry?.icons?.shelfIcon?.sourceIcon, shortcutIc)
+        entry?.setSensitive(true, true)
+        entry?.let { iconManager.createIcons(it) }
+        assertThat(entry?.icons?.aodIcon?.sourceIcon).isEqualTo(smallIc)
+        entry?.setSensitive(false, false)
+        entry?.let { iconManager.updateIcons(it) }
+        assertThat(entry?.icons?.shelfIcon?.sourceIcon).isEqualTo(shortcutIc)
     }
 
     private fun notificationEntry(
-            hasShortcut: Boolean,
-            hasMessage: Boolean,
-            hasLargeIcon: Boolean
+        hasShortcut: Boolean,
+        hasMessageSenderIcon: Boolean,
+        useMessagingStyle: Boolean = true,
+        hasLargeIcon: Boolean
     ): NotificationEntry? {
-        val n = Notification.Builder(mContext, "id")
+        val n =
+            Notification.Builder(mContext, "id")
                 .setSmallIcon(smallIc)
                 .setContentTitle("Title")
                 .setContentText("Text")
 
-        if (hasMessage) {
-            n.style = Notification.MessagingStyle("")
-                    .addMessage(Notification.MessagingStyle.Message(
-                            "",
-                            SystemClock.currentThreadTimeMillis(),
-                            Person.Builder().setIcon(messageIc).build()
-                    ))
+        val messagingStyle =
+            Notification.MessagingStyle("")
+                .addMessage(
+                    Notification.MessagingStyle.Message(
+                        "",
+                        SystemClock.currentThreadTimeMillis(),
+                        Person.Builder()
+                            .setIcon(if (hasMessageSenderIcon) messageIc else null)
+                            .build()
+                    )
+                )
+        if (useMessagingStyle) {
+            n.style = messagingStyle
+        } else {
+            val bundle = Bundle()
+            messagingStyle.addExtras(bundle, false, 0) // Set extras but not EXTRA_TEMPLATE
+            n.addExtras(bundle)
         }
 
         if (hasLargeIcon) {
             n.setLargeIcon(largeIc)
         }
 
-        val builder = NotificationEntryBuilder()
+        val builder =
+            NotificationEntryBuilder()
                 .setPkg(TEST_PACKAGE_NAME)
                 .setOpPkg(TEST_PACKAGE_NAME)
                 .setUid(TEST_UID)
@@ -203,7 +230,7 @@ class IconManagerTest: SysuiTestCase() {
 
         val entry = builder.build()
         entry.row = row
-        entry.setSensitive(false, true);
+        entry.setSensitive(false, true)
         return entry
     }
 }