Merge "Groups which had a priority conversation promoted out of them do not show that conversation's avatar." into tm-dev

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt

@@ -20,11 +20,13 @@ import com.android.systemui.statusbar.notification.collection.ListEntry
 import com.android.systemui.statusbar.notification.collection.NotifPipeline
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
 import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope
+import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner
 import com.android.systemui.statusbar.notification.collection.render.NodeController
 import com.android.systemui.statusbar.notification.dagger.PeopleHeader
+import com.android.systemui.statusbar.notification.icon.ConversationIconManager
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.PeopleNotificationType
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_NON_PERSON
@@ -39,12 +41,40 @@ import javax.inject.Inject
 @CoordinatorScope
 class ConversationCoordinator @Inject constructor(
     private val peopleNotificationIdentifier: PeopleNotificationIdentifier,
+    private val conversationIconManager: ConversationIconManager,
     @PeopleHeader peopleHeaderController: NodeController
 ) : Coordinator {
 
+    private val promotedEntriesToSummaryOfSameChannel =
+        mutableMapOf<NotificationEntry, NotificationEntry>()
+
+    private val onBeforeRenderListListener = OnBeforeRenderListListener { _ ->
+        val unimportantSummaries = promotedEntriesToSummaryOfSameChannel
+            .mapNotNull { (promoted, summary) ->
+                val originalGroup = summary.parent
+                when {
+                    originalGroup == null -> null
+                    originalGroup == promoted.parent -> null
+                    originalGroup.parent == null -> null
+                    originalGroup.summary != summary -> null
+                    originalGroup.children.any { it.channel == summary.channel } -> null
+                    else -> summary.key
+                }
+            }
+        conversationIconManager.setUnimportantConversations(unimportantSummaries)
+        promotedEntriesToSummaryOfSameChannel.clear()
+    }
+
     private val notificationPromoter = object : NotifPromoter(TAG) {
         override fun shouldPromoteToTopLevel(entry: NotificationEntry): Boolean {
-            return entry.channel?.isImportantConversation == true
+            val shouldPromote = entry.channel?.isImportantConversation == true
+            if (shouldPromote) {
+                val summary = entry.parent?.summary
+                if (summary != null && entry.channel == summary.channel) {
+                    promotedEntriesToSummaryOfSameChannel[entry] = summary
+                }
+            }
+            return shouldPromote
         }
     }
 
@@ -67,6 +97,7 @@ class ConversationCoordinator @Inject constructor(
 
     override fun attach(pipeline: NotifPipeline) {
         pipeline.addPromoter(notificationPromoter)
+        pipeline.addOnBeforeRenderListListener(onBeforeRenderListListener)
     }
 
     private fun isConversation(entry: ListEntry): Boolean =

packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java

@@ -70,6 +70,8 @@ import com.android.systemui.statusbar.notification.collection.render.GroupMember
 import com.android.systemui.statusbar.notification.collection.render.NotifGutsViewManager;
 import com.android.systemui.statusbar.notification.collection.render.NotifShadeEventSource;
 import com.android.systemui.statusbar.notification.collection.render.NotificationVisibilityProvider;
+import com.android.systemui.statusbar.notification.icon.ConversationIconManager;
+import com.android.systemui.statusbar.notification.icon.IconManager;
 import com.android.systemui.statusbar.notification.init.NotificationsController;
 import com.android.systemui.statusbar.notification.init.NotificationsControllerImpl;
 import com.android.systemui.statusbar.notification.init.NotificationsControllerStub;
@@ -368,6 +370,10 @@ public interface NotificationsModule {
     @Binds
     NotifInflater bindNotifInflater(NotifInflaterImpl notifInflaterImpl);
 
+    /** */
+    @Binds
+    ConversationIconManager bindConversationIconManager(IconManager iconManager);
+
     /** */
     @Binds
     BindEventManager bindBindEventManagerImpl(BindEventManagerImpl bindEventManagerImpl);

packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt

@@ -27,6 +27,7 @@ import android.view.View
 import android.widget.ImageView
 import com.android.internal.statusbar.StatusBarIcon
 import com.android.systemui.R
+import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.statusbar.StatusBarIconView
 import com.android.systemui.statusbar.notification.InflationException
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
@@ -44,11 +45,14 @@ import javax.inject.Inject
  * TODO: Much of this code was copied whole-sale in order to get it out of NotificationEntry.
  *  Long-term, it should probably live somewhere in the content inflation pipeline.
  */
+@SysUISingleton
 class IconManager @Inject constructor(
     private val notifCollection: CommonNotifCollection,
     private val launcherApps: LauncherApps,
     private val iconBuilder: IconBuilder
-) {
+) : ConversationIconManager {
+    private var unimportantConversationKeys: Set<String> = emptySet()
+
     fun attach() {
         notifCollection.addCollectionListener(entryListener)
     }
@@ -63,16 +67,8 @@ class IconManager @Inject constructor(
         }
 
         override fun onRankingApplied() {
-            // When the sensitivity changes OR when the isImportantConversation status changes,
+            // rankings affect whether a conversation is important, which can change the icons
-            // we need to update the icons
+            recalculateForImportantConversationChange()
-            for (entry in notifCollection.allNotifs) {
-                val isImportant = isImportantConversation(entry)
-                if (entry.icons.areIconsAvailable &&
-                        isImportant != entry.icons.isImportantConversation) {
-                    updateIconsSafe(entry)
-                }
-                entry.icons.isImportantConversation = isImportant
-            }
         }
     }
 
@@ -80,6 +76,18 @@ class IconManager @Inject constructor(
         entry -> updateIconsSafe(entry)
     }
 
+    private fun recalculateForImportantConversationChange() {
+        for (entry in notifCollection.allNotifs) {
+            val isImportant = isImportantConversation(entry)
+            if (entry.icons.areIconsAvailable &&
+                isImportant != entry.icons.isImportantConversation
+            ) {
+                updateIconsSafe(entry)
+            }
+            entry.icons.isImportantConversation = isImportant
+        }
+    }
+
     /**
      * Inflate icon views for each icon variant and assign appropriate icons to them. Stores the
      * result in [NotificationEntry.getIcons].
@@ -306,8 +314,28 @@ class IconManager @Inject constructor(
     }
 
     private fun isImportantConversation(entry: NotificationEntry): Boolean {
-        return entry.ranking.channel != null && entry.ranking.channel.isImportantConversation
+        return entry.ranking.channel != null &&
+                entry.ranking.channel.isImportantConversation &&
+                entry.key !in unimportantConversationKeys
+    }
+
+    override fun setUnimportantConversations(keys: Collection<String>) {
+        val newKeys = keys.toSet()
+        val changed = unimportantConversationKeys != newKeys
+        unimportantConversationKeys = newKeys
+        if (changed) {
+            recalculateForImportantConversationChange()
+        }
     }
 }
 
 private const val TAG = "IconManager"
+
+interface ConversationIconManager {
+    /**
+     * Sets the complete current set of notification keys which should (for the purposes of icon
+     * presentation) be considered unimportant.  This tells the icon manager to remove the avatar
+     * of a group from which the priority notification has been removed.
+     */
+    fun setUnimportantConversations(keys: Collection<String>)
+}

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/GroupEntryBuilder.java

@@ -88,4 +88,7 @@ public class GroupEntryBuilder {
         return this;
     }
 
+    public static List<NotificationEntry> getRawChildren(GroupEntry groupEntry) {
+        return groupEntry.getRawChildren();
+    }
 }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt

@@ -21,17 +21,22 @@ import android.testing.AndroidTestingRunner
 import android.testing.TestableLooper
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.statusbar.notification.collection.GroupEntry
+import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder
 import com.android.systemui.statusbar.notification.collection.NotifPipeline
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
 import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
 import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection
+import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter
 import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner
 import com.android.systemui.statusbar.notification.collection.render.NodeController
+import com.android.systemui.statusbar.notification.icon.ConversationIconManager
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_IMPORTANT_PERSON
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_PERSON
+import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.withArgCaptor
 import com.google.common.truth.Truth.assertThat
 import org.junit.Assert.assertFalse
@@ -52,8 +57,10 @@ class ConversationCoordinatorTest : SysuiTestCase() {
     private lateinit var promoter: NotifPromoter
     private lateinit var peopleSectioner: NotifSectioner
     private lateinit var peopleComparator: NotifComparator
+    private lateinit var beforeRenderListListener: OnBeforeRenderListListener
 
     @Mock private lateinit var pipeline: NotifPipeline
+    @Mock private lateinit var conversationIconManager: ConversationIconManager
     @Mock private lateinit var peopleNotificationIdentifier: PeopleNotificationIdentifier
     @Mock private lateinit var channel: NotificationChannel
     @Mock private lateinit var headerController: NodeController
@@ -66,7 +73,11 @@ class ConversationCoordinatorTest : SysuiTestCase() {
     @Before
     fun setUp() {
         MockitoAnnotations.initMocks(this)
-        coordinator = ConversationCoordinator(peopleNotificationIdentifier, headerController)
+        coordinator = ConversationCoordinator(
+            peopleNotificationIdentifier,
+            conversationIconManager,
+            headerController
+        )
         whenever(channel.isImportantConversation).thenReturn(true)
 
         coordinator.attach(pipeline)
@@ -75,6 +86,9 @@ class ConversationCoordinatorTest : SysuiTestCase() {
         promoter = withArgCaptor {
             verify(pipeline).addPromoter(capture())
         }
+        beforeRenderListListener = withArgCaptor {
+            verify(pipeline).addOnBeforeRenderListListener(capture())
+        }
 
         peopleSectioner = coordinator.sectioner
         peopleComparator = peopleSectioner.comparator!!
@@ -95,6 +109,25 @@ class ConversationCoordinatorTest : SysuiTestCase() {
         assertFalse(promoter.shouldPromoteToTopLevel(NotificationEntryBuilder().build()))
     }
 
+    @Test
+    fun testPromotedImportantConversationsMakesSummaryUnimportant() {
+        val altChildA = NotificationEntryBuilder().setTag("A").build()
+        val altChildB = NotificationEntryBuilder().setTag("B").build()
+        val summary = NotificationEntryBuilder().setId(2).setChannel(channel).build()
+        val groupEntry = GroupEntryBuilder()
+            .setParent(GroupEntry.ROOT_ENTRY)
+            .setSummary(summary)
+            .setChildren(listOf(entry, altChildA, altChildB))
+            .build()
+        assertTrue(promoter.shouldPromoteToTopLevel(entry))
+        assertFalse(promoter.shouldPromoteToTopLevel(altChildA))
+        assertFalse(promoter.shouldPromoteToTopLevel(altChildB))
+        NotificationEntryBuilder.setNewParent(entry, GroupEntry.ROOT_ENTRY)
+        GroupEntryBuilder.getRawChildren(groupEntry).remove(entry)
+        beforeRenderListListener.onBeforeRenderList(listOf(entry, groupEntry))
+        verify(conversationIconManager).setUnimportantConversations(eq(listOf(summary.key)))
+    }
+
     @Test
     fun testInPeopleSection() {
         whenever(peopleNotificationIdentifier.getPeopleNotificationType(entry))