Merge "Clean up index assignments and comparators" into tm-qpr-dev

This commit is contained in:
Jeff DeCew
2022-07-28 10:35:17 +00:00
committed by Android (Google) Code Review
3 changed files with 148 additions and 27 deletions

View File

@@ -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

View File

@@ -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) {

View File

@@ -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;
@@ -1844,6 +1845,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
@@ -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);
}