diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index a8c03243c1174..5bee9a762f6f4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/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> 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 viewsToRemove = new ArrayList<>(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java index df3609bbc6b6e..1c076c49249b4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java +++ b/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 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; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java index 78e9b336f79a7..2b12c22cae390 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java +++ b/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 diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt index 1c47131e2e86c..82a7774b4d828 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt +++ b/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)) ) }