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 945bc72e3e812..9e5dab1152ece 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 @@ 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 5198d82a86c7f..075a0dc7555e5 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 @@ public class ShadeListBuilder implements Dumpable { 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 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 @@ public class ShadeListBuilder implements Dumpable { 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 @@ public class ShadeListBuilder implements Dumpable { 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 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 @@ public class ShadeListBuilder implements Dumpable { */ 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 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 c283cec5c15c0..dfa38abc1ff89 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.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,103 +1844,6 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); } - @Test - public void stableOrderingDisregardedWithSectionChange() { - // GIVEN the first sectioner's packages can be changed from run-to-run - List 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 @@ -2138,7 +2040,6 @@ 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); } @@ -2147,7 +2048,6 @@ 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); } @@ -2155,7 +2055,6 @@ 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); }