Merge "Clean up index assignments and comparators" into tm-qpr-dev am: 0a291f4f64

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

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

3 files changed, 148 insertions, 27 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt
index 9e5dab1152ec..945bc72e3e81 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt
@@ -68,6 +68,9 @@ data class ListAttachState private constructor(
      */
     var stableIndex: Int = -1
 
+    /** Access the index of the [section] or -1 if the entry does not have one */
+    val sectionIndex: Int get() = section?.index ?: -1
+
     /** Copies the state of another instance. */
     fun clone(other: ListAttachState) {
         parent = other.parent
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 075a0dc7555e..5198d82a86c7 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
@@ -1039,22 +1039,25 @@ public class ShadeListBuilder implements Dumpable {
         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);
+            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++;
+                final GroupEntry parent = (GroupEntry) entry;
+                final NotificationEntry summary = parent.getSummary();
+                if (summary != null) {
+                    summary.getAttachState().setStableIndex(sectionMemberIndex++);
+                }
+                final List<NotificationEntry> children = parent.getChildren();
+                for (int j = 0; j < children.size(); j++) {
+                    final NotificationEntry child = children.get(j);
+                    child.getAttachState().setStableIndex(sectionMemberIndex++);
                 }
             }
-            sectionMemberIndex++;
         }
     }
 
@@ -1194,9 +1197,9 @@ public class ShadeListBuilder implements Dumpable {
                 o2.getSectionIndex());
         if (cmp != 0) return cmp;
 
-        int index1 = canReorder(o1) ? -1 : o1.getPreviousAttachState().getStableIndex();
-        int index2 = canReorder(o2) ? -1 : o2.getPreviousAttachState().getStableIndex();
-        cmp = Integer.compare(index1, index2);
+        cmp = Integer.compare(
+                getStableOrderIndex(o1),
+                getStableOrderIndex(o2));
         if (cmp != 0) return cmp;
 
         NotifComparator sectionComparator = getSectionComparator(o1, o2);
@@ -1210,31 +1213,32 @@ public class ShadeListBuilder implements Dumpable {
             if (cmp != 0) return cmp;
         }
 
-        final NotificationEntry rep1 = o1.getRepresentativeEntry();
-        final NotificationEntry rep2 = o2.getRepresentativeEntry();
-            cmp = rep1.getRanking().getRank() - rep2.getRanking().getRank();
+        cmp = Integer.compare(
+                o1.getRepresentativeEntry().getRanking().getRank(),
+                o2.getRepresentativeEntry().getRanking().getRank());
         if (cmp != 0) return cmp;
 
-        cmp = Long.compare(
-                rep2.getSbn().getNotification().when,
-                rep1.getSbn().getNotification().when);
+        cmp = -1 * Long.compare(
+                o1.getRepresentativeEntry().getSbn().getNotification().when,
+                o2.getRepresentativeEntry().getSbn().getNotification().when);
         return cmp;
     };
 
 
     private final Comparator<NotificationEntry> mGroupChildrenComparator = (o1, o2) -> {
-        int index1 = canReorder(o1) ? -1 : o1.getPreviousAttachState().getStableIndex();
-        int index2 = canReorder(o2) ? -1 : o2.getPreviousAttachState().getStableIndex();
-        int cmp = Integer.compare(index1, index2);
+        int cmp = Integer.compare(
+                getStableOrderIndex(o1),
+                getStableOrderIndex(o2));
         if (cmp != 0) return cmp;
 
-        cmp = o1.getRepresentativeEntry().getRanking().getRank()
-                - o2.getRepresentativeEntry().getRanking().getRank();
+        cmp = Integer.compare(
+                o1.getRepresentativeEntry().getRanking().getRank(),
+                o2.getRepresentativeEntry().getRanking().getRank());
         if (cmp != 0) return cmp;
 
-        cmp = Long.compare(
-                o2.getRepresentativeEntry().getSbn().getNotification().when,
-                o1.getRepresentativeEntry().getSbn().getNotification().when);
+        cmp = -1 * Long.compare(
+                o1.getRepresentativeEntry().getSbn().getNotification().when,
+                o2.getRepresentativeEntry().getSbn().getNotification().when);
         return cmp;
     };
 
@@ -1244,8 +1248,21 @@ public class ShadeListBuilder implements Dumpable {
      */
     private boolean mForceReorderable = false;
 
-    private boolean canReorder(ListEntry entry) {
-        return mForceReorderable || getStabilityManager().isEntryReorderingAllowed(entry);
+    private int getStableOrderIndex(ListEntry entry) {
+        if (mForceReorderable) {
+            // this is used to determine if the list is correctly sorted
+            return -1;
+        }
+        if (getStabilityManager().isEntryReorderingAllowed(entry)) {
+            // let the stability manager constrain or allow reordering
+            return -1;
+        }
+        if (entry.getAttachState().getSectionIndex()
+                != entry.getPreviousAttachState().getSectionIndex()) {
+            // stable index is only valid within the same section; otherwise we allow reordering
+            return -1;
+        }
+        return entry.getPreviousAttachState().getStableIndex();
     }
 
     private boolean applyFilters(NotificationEntry entry, long now, List<NotifFilter> filters) {
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 dfa38abc1ff8..c283cec5c15c 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
@@ -33,6 +33,7 @@ import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -1845,6 +1846,103 @@ public class ShadeListBuilderTest extends SysuiTestCase {
     }
 
     @Test
+    public void stableOrderingDisregardedWithSectionChange() {
+        // GIVEN the first sectioner's packages can be changed from run-to-run
+        List<String> mutableSectionerPackages = new ArrayList<>();
+        mutableSectionerPackages.add(PACKAGE_1);
+        mListBuilder.setSectioners(asList(
+                new PackageSectioner(mutableSectionerPackages, null),
+                new PackageSectioner(List.of(PACKAGE_1, PACKAGE_2, PACKAGE_3), null)));
+        mStabilityManager.setAllowEntryReordering(false);
+
+        // WHEN the list is originally built with reordering disabled (and section changes allowed)
+        addNotif(0, PACKAGE_1).setRank(1);
+        addNotif(1, PACKAGE_1).setRank(5);
+        addNotif(2, PACKAGE_2).setRank(2);
+        addNotif(3, PACKAGE_2).setRank(3);
+        addNotif(4, PACKAGE_3).setRank(4);
+        dispatchBuild();
+
+        // VERIFY the order and that entry reordering has not been suppressed
+        verifyBuiltList(
+                notif(0),
+                notif(1),
+                notif(2),
+                notif(3),
+                notif(4)
+        );
+        verify(mStabilityManager, never()).onEntryReorderSuppressed();
+
+        // WHEN the first section now claims PACKAGE_3 notifications
+        mutableSectionerPackages.add(PACKAGE_3);
+        dispatchBuild();
+
+        // VERIFY the re-sectioned notification is inserted at the top of the first section, because
+        // it's effectively "new" and "new" things are inserted at the top of their section.
+        verifyBuiltList(
+                notif(4),
+                notif(0),
+                notif(1),
+                notif(2),
+                notif(3)
+        );
+        verify(mStabilityManager).onEntryReorderSuppressed();
+        clearInvocations(mStabilityManager);
+
+        // WHEN reordering is now allowed again
+        mStabilityManager.setAllowEntryReordering(true);
+        dispatchBuild();
+
+        // VERIFY that list order changes to put the re-sectioned notification in the middle where
+        // it is ranked.
+        verifyBuiltList(
+                notif(0),
+                notif(4),
+                notif(1),
+                notif(2),
+                notif(3)
+        );
+        verify(mStabilityManager, never()).onEntryReorderSuppressed();
+    }
+
+    @Test
+    public void groupRevertingToSummaryRetainsStablePosition() {
+        // 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)
+        );
+    }
+
+    @Test
     public void testStableChildOrdering() {
         // WHEN the list is originally built with reordering disabled
         mStabilityManager.setAllowEntryReordering(false);
@@ -2040,6 +2138,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
 
     private void assertOrder(String visible, String active, String expected) {
         StringBuilder differenceSb = new StringBuilder();
+        NotifSection section = new NotifSection(mock(NotifSectioner.class), 0);
         for (char c : active.toCharArray()) {
             if (visible.indexOf(c) < 0) differenceSb.append(c);
         }
@@ -2048,6 +2147,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
         for (int i = 0; i < visible.length(); i++) {
             addNotif(i, String.valueOf(visible.charAt(i)))
                     .setRank(active.indexOf(visible.charAt(i)))
+                    .setSection(section)
                     .setStableIndex(i);
 
         }
@@ -2055,6 +2155,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
         for (int i = 0; i < difference.length(); i++) {
             addNotif(i + visible.length(), String.valueOf(difference.charAt(i)))
                     .setRank(active.indexOf(difference.charAt(i)))
+                    .setSection(section)
                     .setStableIndex(-1);
         }