From 4ba808972afb28a128403bfa8e93efcdcb95e23e Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Thu, 5 May 2022 18:56:43 +0000 Subject: [PATCH] Detect when group children are mis-ordered to ensure stability invalidation. Fixes: 204817567 Bug: 237216329 Test: atest ShadeListBuilderTest Change-Id: I3c9fcd91dd268e03b176235b1c4c79517f9aaa89 (cherry picked from commit 91c4b8dd60caaa8bf831990a57ebbe0d91083759) --- .../collection/ShadeListBuilder.java | 20 ++++- .../collection/ShadeListBuilderTest.java | 84 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) 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 df2fe4e8511f8..2a8a67281c2c9 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 @@ -990,7 +990,7 @@ public class ShadeListBuilder implements Dumpable { // Check for suppressed order changes if (!getStabilityManager().isEveryChangeAllowed()) { mForceReorderable = true; - boolean isSorted = isSorted(mNotifList, mTopLevelComparator); + boolean isSorted = isShadeSorted(); mForceReorderable = false; if (!isSorted) { getStabilityManager().onEntryReorderSuppressed(); @@ -999,9 +999,23 @@ public class ShadeListBuilder implements Dumpable { Trace.endSection(); } + private boolean isShadeSorted() { + if (!isSorted(mNotifList, mTopLevelComparator)) { + return false; + } + for (ListEntry entry : mNotifList) { + if (entry instanceof GroupEntry) { + if (!isSorted(((GroupEntry) entry).getChildren(), mGroupChildrenComparator)) { + return false; + } + } + } + return true; + } + /** Determine whether the items in the list are sorted according to the comparator */ @VisibleForTesting - public static boolean isSorted(List items, Comparator comparator) { + public static boolean isSorted(List items, Comparator comparator) { if (items.size() <= 1) { return true; } @@ -1209,7 +1223,7 @@ public class ShadeListBuilder implements Dumpable { }; - private final Comparator mGroupChildrenComparator = (o1, o2) -> { + 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); 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 4e7e79f2cb26c..95460583e857b 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 @@ -53,6 +53,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; import com.android.systemui.statusbar.NotificationInteractionTracker; +import com.android.systemui.statusbar.RankingBuilder; import com.android.systemui.statusbar.notification.NotifPipelineFlags; import com.android.systemui.statusbar.notification.collection.ShadeListBuilder.OnRenderListListener; import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection; @@ -1797,6 +1798,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Test public void testStableMultipleSectionOrdering() { + // WHEN the list is originally built with reordering disabled mListBuilder.setSectioners(asList( new PackageSectioner(PACKAGE_1), new PackageSectioner(PACKAGE_2))); mStabilityManager.setAllowEntryReordering(false); @@ -1807,12 +1809,94 @@ public class ShadeListBuilderTest extends SysuiTestCase { addNotif(3, PACKAGE_1).setRank(3); dispatchBuild(); + // VERIFY the order and that entry reordering has not been suppressed verifyBuiltList( notif(0), notif(1), notif(3), notif(2) ); + verify(mStabilityManager, never()).onEntryReorderSuppressed(); + + // WHEN the ranks change + setNewRank(notif(0).entry, 4); + dispatchBuild(); + + // VERIFY the order does not change that entry reordering has been suppressed + verifyBuiltList( + notif(0), + notif(1), + notif(3), + notif(2) + ); + verify(mStabilityManager).onEntryReorderSuppressed(); + + // WHEN reordering is now allowed again + mStabilityManager.setAllowEntryReordering(true); + dispatchBuild(); + + // VERIFY that list order changes + verifyBuiltList( + notif(1), + notif(3), + notif(0), + notif(2) + ); + } + + @Test + public void testStableChildOrdering() { + // WHEN the list is originally built with reordering disabled + mStabilityManager.setAllowEntryReordering(false); + addGroupSummary(0, PACKAGE_1, GROUP_1).setRank(0); + addGroupChild(1, PACKAGE_1, GROUP_1).setRank(1); + addGroupChild(2, PACKAGE_1, GROUP_1).setRank(2); + addGroupChild(3, PACKAGE_1, GROUP_1).setRank(3); + dispatchBuild(); + + // VERIFY the order and that entry reordering has not been suppressed + verifyBuiltList( + group( + summary(0), + child(1), + child(2), + child(3) + ) + ); + verify(mStabilityManager, never()).onEntryReorderSuppressed(); + + // WHEN the ranks change + setNewRank(notif(2).entry, 5); + dispatchBuild(); + + // VERIFY the order does not change that entry reordering has been suppressed + verifyBuiltList( + group( + summary(0), + child(1), + child(2), + child(3) + ) + ); + verify(mStabilityManager).onEntryReorderSuppressed(); + + // WHEN reordering is now allowed again + mStabilityManager.setAllowEntryReordering(true); + dispatchBuild(); + + // VERIFY that list order changes + verifyBuiltList( + group( + summary(0), + child(1), + child(3), + child(2) + ) + ); + } + + private static void setNewRank(NotificationEntry entry, int rank) { + entry.setRanking(new RankingBuilder(entry.getRanking()).setRank(rank).build()); } @Test