Revert "Clean up index assignments and comparators"
This reverts commit 6c579ef0eb396bae246e15925289c1e0fa9d8036.
Reason for revert: b/240857315
Change-Id: I6302bc24dd61bd59f900962fb3392f941e30c241
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 945bc72..9e5dab1 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,9 +68,6 @@
*/
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 5198d82..075a0dc 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,25 +1039,22 @@
NotifSection currentSection = requireNonNull(notifList.get(0).getSection());
int sectionMemberIndex = 0;
for (int i = 0; i < notifList.size(); i++) {
- final ListEntry entry = notifList.get(i);
+ 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) {
- 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++);
+ GroupEntry parent = (GroupEntry) entry;
+ for (int j = 0; j < parent.getChildren().size(); j++) {
+ entry = parent.getChildren().get(j);
+ entry.getAttachState().setStableIndex(sectionMemberIndex);
+ sectionMemberIndex++;
}
}
+ sectionMemberIndex++;
}
}
@@ -1197,9 +1194,9 @@
o2.getSectionIndex());
if (cmp != 0) return cmp;
- cmp = Integer.compare(
- getStableOrderIndex(o1),
- getStableOrderIndex(o2));
+ int index1 = canReorder(o1) ? -1 : o1.getPreviousAttachState().getStableIndex();
+ int index2 = canReorder(o2) ? -1 : o2.getPreviousAttachState().getStableIndex();
+ cmp = Integer.compare(index1, index2);
if (cmp != 0) return cmp;
NotifComparator sectionComparator = getSectionComparator(o1, o2);
@@ -1213,32 +1210,31 @@
if (cmp != 0) return cmp;
}
- cmp = Integer.compare(
- o1.getRepresentativeEntry().getRanking().getRank(),
- o2.getRepresentativeEntry().getRanking().getRank());
+ final NotificationEntry rep1 = o1.getRepresentativeEntry();
+ final NotificationEntry rep2 = o2.getRepresentativeEntry();
+ cmp = rep1.getRanking().getRank() - rep2.getRanking().getRank();
if (cmp != 0) return cmp;
- cmp = -1 * Long.compare(
- o1.getRepresentativeEntry().getSbn().getNotification().when,
- o2.getRepresentativeEntry().getSbn().getNotification().when);
+ cmp = Long.compare(
+ rep2.getSbn().getNotification().when,
+ rep1.getSbn().getNotification().when);
return cmp;
};
private final Comparator<NotificationEntry> mGroupChildrenComparator = (o1, o2) -> {
- int cmp = Integer.compare(
- getStableOrderIndex(o1),
- getStableOrderIndex(o2));
+ int index1 = canReorder(o1) ? -1 : o1.getPreviousAttachState().getStableIndex();
+ int index2 = canReorder(o2) ? -1 : o2.getPreviousAttachState().getStableIndex();
+ int cmp = Integer.compare(index1, index2);
if (cmp != 0) return cmp;
- cmp = Integer.compare(
- o1.getRepresentativeEntry().getRanking().getRank(),
- o2.getRepresentativeEntry().getRanking().getRank());
+ cmp = o1.getRepresentativeEntry().getRanking().getRank()
+ - o2.getRepresentativeEntry().getRanking().getRank();
if (cmp != 0) return cmp;
- cmp = -1 * Long.compare(
- o1.getRepresentativeEntry().getSbn().getNotification().when,
- o2.getRepresentativeEntry().getSbn().getNotification().when);
+ cmp = Long.compare(
+ o2.getRepresentativeEntry().getSbn().getNotification().when,
+ o1.getRepresentativeEntry().getSbn().getNotification().when);
return cmp;
};
@@ -1248,21 +1244,8 @@
*/
private boolean mForceReorderable = false;
- 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 canReorder(ListEntry entry) {
+ return mForceReorderable || getStabilityManager().isEntryReorderingAllowed(entry);
}
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 c283cec..dfa38ab 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,7 +33,6 @@
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;
@@ -1846,103 +1845,6 @@
}
@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);
@@ -2138,7 +2040,6 @@
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);
}
@@ -2147,7 +2048,6 @@
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);
}
@@ -2155,7 +2055,6 @@
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);
}
