diff options
4 files changed, 113 insertions, 11 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.java b/packages/SystemUI/src/com/android/systemui/flags/Flags.java index 6e2f22fa29f7..5c44e529340e 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.java +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.java @@ -73,7 +73,9 @@ public class Flags { public static final UnreleasedFlag NOTIFICATION_DISMISSAL_FADE = new UnreleasedFlag(113, true); - // next id: 114 + public static final UnreleasedFlag STABILITY_INDEX_FIX = new UnreleasedFlag(114, true); + + // next id: 115 /***************************************/ // 200 - keyguard/lockscreen diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt index 7fbdd35796c1..2f0a1aa9b0d5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt @@ -53,4 +53,8 @@ class NotifPipelineFlags @Inject constructor( fun fullScreenIntentRequiresKeyguard(): Boolean = featureFlags.isEnabled(Flags.FSI_REQUIRES_KEYGUARD) + + val isStabilityIndexFixEnabled: Boolean by lazy { + featureFlags.isEnabled(Flags.STABILITY_INDEX_FIX) + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index e129ee45817a..225236f95179 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -96,6 +96,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { // used exclusivly by ShadeListBuilder#notifySectionEntriesUpdated // TODO replace temp with collection pool for readability private final ArrayList<ListEntry> mTempSectionMembers = new ArrayList<>(); + private final NotifPipelineFlags mFlags; private final boolean mAlwaysLogList; private List<ListEntry> mNotifList = new ArrayList<>(); @@ -141,6 +142,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { ) { mSystemClock = systemClock; mLogger = logger; + mFlags = flags; mAlwaysLogList = flags.isDevLoggingEnabled(); mInteractionTracker = interactionTracker; mChoreographer = pipelineChoreographer; @@ -1036,27 +1038,41 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { /** * Assign the index of each notification relative to the total order */ - private static void assignIndexes(List<ListEntry> notifList) { + private void assignIndexes(List<ListEntry> notifList) { if (notifList.size() == 0) return; NotifSection currentSection = requireNonNull(notifList.get(0).getSection()); int sectionMemberIndex = 0; for (int i = 0; i < notifList.size(); i++) { - ListEntry entry = notifList.get(i); + final ListEntry entry = notifList.get(i); NotifSection section = requireNonNull(entry.getSection()); if (section.getIndex() != currentSection.getIndex()) { sectionMemberIndex = 0; currentSection = section; } - entry.getAttachState().setStableIndex(sectionMemberIndex); - if (entry instanceof GroupEntry) { - GroupEntry parent = (GroupEntry) entry; - for (int j = 0; j < parent.getChildren().size(); j++) { - entry = parent.getChildren().get(j); - entry.getAttachState().setStableIndex(sectionMemberIndex); - sectionMemberIndex++; + if (mFlags.isStabilityIndexFixEnabled()) { + entry.getAttachState().setStableIndex(sectionMemberIndex++); + if (entry instanceof GroupEntry) { + final GroupEntry parent = (GroupEntry) entry; + final NotificationEntry summary = parent.getSummary(); + if (summary != null) { + summary.getAttachState().setStableIndex(sectionMemberIndex++); + } + for (NotificationEntry child : parent.getChildren()) { + child.getAttachState().setStableIndex(sectionMemberIndex++); + } + } + } else { + // This old implementation uses the same index number for the group as the first + // child, and fails to assign an index to the summary. Remove once tested. + entry.getAttachState().setStableIndex(sectionMemberIndex); + if (entry instanceof GroupEntry) { + final GroupEntry parent = (GroupEntry) entry; + for (NotificationEntry child : parent.getChildren()) { + child.getAttachState().setStableIndex(sectionMemberIndex++); + } } + sectionMemberIndex++; } - sectionMemberIndex++; } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index 82e32b2fdc64..896ce2b3994b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -38,6 +38,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -2112,6 +2113,85 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); } + @Test + public void groupRevertingToSummaryDoesNotRetainStablePositionWithLegacyIndexLogic() { + when(mNotifPipelineFlags.isStabilityIndexFixEnabled()).thenReturn(false); + + // GIVEN a notification group is on screen + mStabilityManager.setAllowEntryReordering(false); + + // WHEN the list is originally built with reordering disabled (and section changes allowed) + addNotif(0, PACKAGE_1).setRank(2); + addNotif(1, PACKAGE_1).setRank(3); + addGroupSummary(2, PACKAGE_1, "group").setRank(4); + addGroupChild(3, PACKAGE_1, "group").setRank(5); + addGroupChild(4, PACKAGE_1, "group").setRank(6); + dispatchBuild(); + + verifyBuiltList( + notif(0), + notif(1), + group( + summary(2), + child(3), + child(4) + ) + ); + + // WHEN the notification summary rank increases and children removed + setNewRank(notif(2).entry, 1); + mEntrySet.remove(4); + mEntrySet.remove(3); + dispatchBuild(); + + // VERIFY the summary (incorrectly) moves to the top of the section where it is ranked, + // despite visual stability being active + verifyBuiltList( + notif(2), + notif(0), + notif(1) + ); + } + + @Test + public void groupRevertingToSummaryRetainsStablePosition() { + when(mNotifPipelineFlags.isStabilityIndexFixEnabled()).thenReturn(true); + + // GIVEN a notification group is on screen + mStabilityManager.setAllowEntryReordering(false); + + // WHEN the list is originally built with reordering disabled (and section changes allowed) + addNotif(0, PACKAGE_1).setRank(2); + addNotif(1, PACKAGE_1).setRank(3); + addGroupSummary(2, PACKAGE_1, "group").setRank(4); + addGroupChild(3, PACKAGE_1, "group").setRank(5); + addGroupChild(4, PACKAGE_1, "group").setRank(6); + dispatchBuild(); + + verifyBuiltList( + notif(0), + notif(1), + group( + summary(2), + child(3), + child(4) + ) + ); + + // WHEN the notification summary rank increases and children removed + setNewRank(notif(2).entry, 1); + mEntrySet.remove(4); + mEntrySet.remove(3); + dispatchBuild(); + + // VERIFY the summary stays in the same location on rebuild + verifyBuiltList( + notif(0), + notif(1), + notif(2) + ); + } + private static void setNewRank(NotificationEntry entry, int rank) { entry.setRanking(new RankingBuilder(entry.getRanking()).setRank(rank).build()); } |