Summaries receive a stability section index

This fixes an issue where the summary with no children wouldn't have its sort position stabilized, resulting in unintended move animations while the shade is open when child notifications were cancelled.

Bug: 241229236
Test: atest ShadeListBuilderTest
Change-Id: I742d68ce05e080f3b7a928247e46d8406f21eebb

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