From b837ed4a0c5920958e0ee8a6619548908dc7dcbe Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 1 Aug 2022 19:07:46 +0000 Subject: [PATCH] ShadeListBuilder clarity refactors There are no functionality changes in this CL, just code structure: * Standardize comparator code style * Move list attach annulment into ListAttachState.detach() * Add a TODO about the missing annulment Bug: 237216329 Test: atest ShadeListBuilderTest Change-Id: Id2d77e6bf9b89c4bae2026f2e5355542f1c76a20 --- .../collection/ListAttachState.kt | 12 +++++ .../collection/ShadeListBuilder.java | 51 +++++++++++-------- 2 files changed, 41 insertions(+), 22 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 9e5dab1152ece..f8449ae8807b6 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 @@ -90,6 +90,18 @@ data class ListAttachState private constructor( stableIndex = -1 } + /** + * Erases bookkeeping traces stored on an entry when it is removed from the notif list. + * This can happen if the entry is removed from a group that was broken up or if the entry was + * filtered out during any of the filtering steps. + */ + fun detach() { + parent = null + section = null + promoter = null + // stableIndex = -1 // TODO(b/241229236): Clear this once we fix the stability fragility + } + companion object { @JvmStatic fun create(): ListAttachState { 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 075a0dc7555e5..be427afd9656f 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 @@ -954,9 +954,7 @@ public class ShadeListBuilder implements Dumpable { * filtered out during any of the filtering steps. */ private void annulAddition(ListEntry entry) { - entry.setParent(null); - entry.getAttachState().setSection(null); - entry.getAttachState().setPromoter(null); + entry.getAttachState().detach(); } private void assignSections() { @@ -1194,9 +1192,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 +1208,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 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 +1243,16 @@ 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; + } + return entry.getPreviousAttachState().getStableIndex(); } private boolean applyFilters(NotificationEntry entry, long now, List filters) {