Merge "Fix notification grouping and section bug" into rvc-dev am: d04af4c0b0

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

Change-Id: Id299e713453626573a239bf58561ec2fc0a9c5ec

packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java

@@ -60,7 +60,11 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
 
     private final Handler mHandler;
 
-    /** Re-usable map of top-level notifications to their sorted children if any.*/
+    /**
+     * Re-usable map of top-level notifications to their sorted children if any.
+     * If the top-level notification doesn't have children, its key will still exist in this map
+     * with its value explicitly set to null.
+     */
     private final HashMap<NotificationEntry, List<NotificationEntry>> mTmpChildOrderMap =
             new HashMap<>();
 
@@ -212,10 +216,19 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
                 }
                 orderedChildren.add(ent);
             } else {
-                // Top-level notif
-                mTmpChildOrderMap.put(ent, null);
+                // Top-level notif (either a summary or single notification)
+
+                // A child may have already added its summary to mTmpChildOrderMap with a
+                // list of children. This can happen since there's no guarantee summaries are
+                // sorted before its children.
+                if (!mTmpChildOrderMap.containsKey(ent)) {
+                    // mTmpChildOrderMap's keyset is used to iterate through all entries, so it's
+                    // necessary to add each top-level notif as a key
+                    mTmpChildOrderMap.put(ent, null);
+                }
                 toShow.add(ent.getRow());
             }
+
         }
 
         ArrayList<ExpandableNotificationRow> viewsToRemove = new ArrayList<>();

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java

@@ -23,6 +23,9 @@ import com.android.systemui.statusbar.notification.collection.GroupEntry;
 import com.android.systemui.statusbar.notification.collection.ListEntry;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
+import com.android.systemui.statusbar.phone.NotificationGroupManager;
+
+import java.util.List;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -36,10 +39,14 @@ import javax.inject.Singleton;
 @Singleton
 public class HighPriorityProvider {
     private final PeopleNotificationIdentifier mPeopleNotificationIdentifier;
+    private final NotificationGroupManager mGroupManager;
 
     @Inject
-    public HighPriorityProvider(PeopleNotificationIdentifier peopleNotificationIdentifier) {
+    public HighPriorityProvider(
+            PeopleNotificationIdentifier peopleNotificationIdentifier,
+            NotificationGroupManager groupManager) {
         mPeopleNotificationIdentifier = peopleNotificationIdentifier;
+        mGroupManager = groupManager;
     }
 
     /**
@@ -74,13 +81,25 @@ public class HighPriorityProvider {
 
 
     private boolean hasHighPriorityChild(ListEntry entry) {
+        List<NotificationEntry> children = null;
+
         if (entry instanceof GroupEntry) {
-            for (NotificationEntry child : ((GroupEntry) entry).getChildren()) {
+            // New notification pipeline
+            children = ((GroupEntry) entry).getChildren();
+        } else if (entry.getRepresentativeEntry() != null
+                && mGroupManager.isGroupSummary(entry.getRepresentativeEntry().getSbn())) {
+            // Old notification pipeline
+            children = mGroupManager.getChildren(entry.getRepresentativeEntry().getSbn());
+        }
+
+        if (children != null) {
+            for (NotificationEntry child : children) {
                 if (isHighPriority(child)) {
                     return true;
                 }
             }
         }
+
         return false;
     }
 

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

@@ -38,6 +38,7 @@ import com.android.systemui.SysuiTestCase;
 import com.android.systemui.statusbar.RankingBuilder;
 import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
+import com.android.systemui.statusbar.phone.NotificationGroupManager;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -45,16 +46,22 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+
 @SmallTest
 @RunWith(AndroidTestingRunner.class)
 public class HighPriorityProviderTest extends SysuiTestCase {
     @Mock private PeopleNotificationIdentifier mPeopleNotificationIdentifier;
+    @Mock private NotificationGroupManager mGroupManager;
     private HighPriorityProvider mHighPriorityProvider;
 
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this);
-        mHighPriorityProvider = new HighPriorityProvider(mPeopleNotificationIdentifier);
+        mHighPriorityProvider = new HighPriorityProvider(
+                mPeopleNotificationIdentifier,
+                mGroupManager);
     }
 
     @Test
@@ -165,6 +172,22 @@ public class HighPriorityProviderTest extends SysuiTestCase {
         assertFalse(mHighPriorityProvider.isHighPriority(entry));
     }
 
+    @Test
+    public void testIsHighPriority_checkChildrenToCalculatePriority() {
+        // GIVEN: a summary with low priority has a highPriorityChild and a lowPriorityChild
+        final NotificationEntry summary = createNotifEntry(false);
+        final NotificationEntry lowPriorityChild = createNotifEntry(false);
+        final NotificationEntry highPriorityChild = createNotifEntry(true);
+        when(mGroupManager.isGroupSummary(summary.getSbn())).thenReturn(true);
+        when(mGroupManager.getChildren(summary.getSbn())).thenReturn(
+                new ArrayList<>(Arrays.asList(lowPriorityChild, highPriorityChild)));
+
+        // THEN the summary is high priority since it has a high priority child
+        assertTrue(mHighPriorityProvider.isHighPriority(summary));
+    }
+
+    // Tests below here are only relevant to the NEW notification pipeline which uses GroupEntry
+
     @Test
     public void testIsHighPriority_summaryUpdated() {
         // GIVEN a GroupEntry with a lowPrioritySummary and no children
@@ -186,7 +209,7 @@ public class HighPriorityProviderTest extends SysuiTestCase {
     }
 
     @Test
-    public void testIsHighPriority_checkChildrenToCalculatePriority() {
+    public void testIsHighPriority_checkChildrenToCalculatePriorityOf() {
         // GIVEN:
         // GroupEntry = parentEntry, summary = lowPrioritySummary
         //      NotificationEntry = lowPriorityChild

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

@@ -77,7 +77,8 @@ class NotificationRankingManagerTest : SysuiTestCase() {
                 mock(NotificationEntryManagerLogger::class.java),
                 sectionsManager,
                 personNotificationIdentifier,
-                HighPriorityProvider(personNotificationIdentifier)
+                HighPriorityProvider(personNotificationIdentifier,
+                    mock(NotificationGroupManager::class.java))
         )
     }