Summaries receive a stability section index am: b0c2be9dca

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

Change-Id: I45b9fd553d0b9bf75c5b614b6f50e7d01c1068d5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

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)
+    }
 }

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,29 +1038,43 @@ 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;
             }
+            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) {
-                GroupEntry parent = (GroupEntry) entry;
+                    final GroupEntry parent = (GroupEntry) entry;
-                for (int j = 0; j < parent.getChildren().size(); j++) {
+                    for (NotificationEntry child : parent.getChildren()) {
-                    entry = parent.getChildren().get(j);
+                        child.getAttachState().setStableIndex(sectionMemberIndex++);
-                    entry.getAttachState().setStableIndex(sectionMemberIndex);
-                    sectionMemberIndex++;
                     }
                 }
                 sectionMemberIndex++;
             }
         }
+    }
 
     private void freeEmptyGroups() {
         Trace.beginSection("ShadeListBuilder.freeEmptyGroups");

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());
     }