diff options
5 files changed, 13 insertions, 180 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index 58bb742a2615..c45c8e761abc 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -77,12 +77,6 @@ object Flags { // TODO(b/254512731): Tracking Bug @JvmField val NOTIFICATION_DISMISSAL_FADE = releasedFlag(113, "notification_dismissal_fade") - // TODO(b/259558771): Tracking Bug - val STABILITY_INDEX_FIX = releasedFlag(114, "stability_index_fix") - - // TODO(b/259559750): Tracking Bug - val SEMI_STABLE_SORT = releasedFlag(115, "semi_stable_sort") - @JvmField val USE_ROUNDNESS_SOURCETYPES = releasedFlag(116, "use_roundness_sourcetype") // TODO(b/259217907) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt index 3072c810d31b..05a9a427870e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt @@ -35,14 +35,6 @@ class NotifPipelineFlags @Inject constructor( fun fsiOnDNDUpdate(): Boolean = featureFlags.isEnabled(Flags.FSI_ON_DND_UPDATE) - val isStabilityIndexFixEnabled: Boolean by lazy { - featureFlags.isEnabled(Flags.STABILITY_INDEX_FIX) - } - - val isSemiStableSortEnabled: Boolean by lazy { - featureFlags.isEnabled(Flags.SEMI_STABLE_SORT) - } - val shouldFilterUnseenNotifsOnKeyguard: Boolean by lazy { featureFlags.isEnabled(Flags.FILTER_UNSEEN_NOTIFS_ON_KEYGUARD) } 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 84ab0d1190f0..b5fce4163bb0 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 @@ -98,13 +98,11 @@ data class ListAttachState private constructor( * 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(includingStableIndex: Boolean) { + fun detach() { parent = null section = null promoter = null - if (includingStableIndex) { - stableIndex = -1 - } + stableIndex = -1 } companion object { 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 65a21a4e2190..4065b98ab0c8 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 @@ -965,8 +965,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { * filtered out during any of the filtering steps. */ private void annulAddition(ListEntry entry) { - // NOTE(b/241229236): Don't clear stableIndex until we fix stability fragility - entry.getAttachState().detach(/* includingStableIndex= */ mFlags.isSemiStableSortEnabled()); + entry.getAttachState().detach(); } private void assignSections() { @@ -986,50 +985,10 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { private void sortListAndGroups() { Trace.beginSection("ShadeListBuilder.sortListAndGroups"); - if (mFlags.isSemiStableSortEnabled()) { - sortWithSemiStableSort(); - } else { - sortWithLegacyStability(); - } + sortWithSemiStableSort(); Trace.endSection(); } - private void sortWithLegacyStability() { - // Sort all groups and the top level list - for (ListEntry entry : mNotifList) { - if (entry instanceof GroupEntry) { - GroupEntry parent = (GroupEntry) entry; - parent.sortChildren(mGroupChildrenComparator); - } - } - mNotifList.sort(mTopLevelComparator); - assignIndexes(mNotifList); - - // Check for suppressed order changes - if (!getStabilityManager().isEveryChangeAllowed()) { - mForceReorderable = true; - boolean isSorted = isShadeSortedLegacy(); - mForceReorderable = false; - if (!isSorted) { - getStabilityManager().onEntryReorderSuppressed(); - } - } - } - - private boolean isShadeSortedLegacy() { - if (!isSorted(mNotifList, mTopLevelComparator)) { - return false; - } - for (ListEntry entry : mNotifList) { - if (entry instanceof GroupEntry) { - if (!isSorted(((GroupEntry) entry).getChildren(), mGroupChildrenComparator)) { - return false; - } - } - } - return true; - } - private void sortWithSemiStableSort() { // Sort each group's children boolean allSorted = true; @@ -1100,29 +1059,16 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { sectionMemberIndex = 0; currentSection = section; } - if (mFlags.isStabilityIndexFixEnabled()) { - 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++); - } - for (NotificationEntry child : parent.getChildren()) { - child.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++); } - } else { - // This old implementation uses the same index number for the group as the first - // child, and fails to assign an index to the summary. Remove once tested. - entry.getAttachState().setStableIndex(sectionMemberIndex); - if (entry instanceof GroupEntry) { - final GroupEntry parent = (GroupEntry) entry; - for (NotificationEntry child : parent.getChildren()) { - child.getAttachState().setStableIndex(sectionMemberIndex++); - } + for (NotificationEntry child : parent.getChildren()) { + child.getAttachState().setStableIndex(sectionMemberIndex++); } - sectionMemberIndex++; } } } @@ -1272,11 +1218,6 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { o2.getSectionIndex()); if (cmp != 0) return cmp; - cmp = mFlags.isSemiStableSortEnabled() ? 0 : Integer.compare( - getStableOrderIndex(o1), - getStableOrderIndex(o2)); - if (cmp != 0) return cmp; - NotifComparator sectionComparator = getSectionComparator(o1, o2); if (sectionComparator != null) { cmp = sectionComparator.compare(o1, o2); @@ -1301,12 +1242,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { private final Comparator<NotificationEntry> mGroupChildrenComparator = (o1, o2) -> { - int cmp = mFlags.isSemiStableSortEnabled() ? 0 : Integer.compare( - getStableOrderIndex(o1), - getStableOrderIndex(o2)); - if (cmp != 0) return cmp; - - cmp = Integer.compare( + int cmp = Integer.compare( o1.getRepresentativeEntry().getRanking().getRank(), o2.getRepresentativeEntry().getRanking().getRank()); if (cmp != 0) return cmp; @@ -1317,25 +1253,6 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { return cmp; }; - /** - * A flag that is set to true when we want to run the comparators as if all reordering is - * allowed. This is used to check if the list is "out of order" after the sort is complete. - */ - 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; - } - // NOTE(b/241229236): Can't use cleared section index until we fix stability fragility - return entry.getPreviousAttachState().getStableIndex(); - } - @Nullable private Integer getStableOrderRank(ListEntry entry) { if (getStabilityManager().isEntryReorderingAllowed(entry)) { 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 09f8a10f88c7..a8690388c3e3 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 @@ -39,7 +39,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -137,7 +136,6 @@ public class ShadeListBuilderTest extends SysuiTestCase { public void setUp() { MockitoAnnotations.initMocks(this); allowTestableLooperAsMainThread(); - when(mNotifPipelineFlags.isStabilityIndexFixEnabled()).thenReturn(true); mListBuilder = new ShadeListBuilder( mDumpManager, @@ -1998,29 +1996,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { } @Test - public void testActiveOrdering_withLegacyStability() { - when(mNotifPipelineFlags.isSemiStableSortEnabled()).thenReturn(false); - assertOrder("ABCDEFG", "ABCDEFG", "ABCDEFG", true); // no change - assertOrder("ABCDEFG", "ACDEFXBG", "ACDEFXBG", true); // X - assertOrder("ABCDEFG", "ACDEFBG", "ACDEFBG", true); // no change - assertOrder("ABCDEFG", "ACDEFBXZG", "ACDEFBXZG", true); // Z and X - assertOrder("ABCDEFG", "AXCDEZFBG", "AXCDEZFBG", true); // Z and X + gap - } - - @Test - public void testStableOrdering_withLegacyStability() { - when(mNotifPipelineFlags.isSemiStableSortEnabled()).thenReturn(false); - mStabilityManager.setAllowEntryReordering(false); - assertOrder("ABCDEFG", "ABCDEFG", "ABCDEFG", true); // no change - assertOrder("ABCDEFG", "ACDEFXBG", "XABCDEFG", false); // X - assertOrder("ABCDEFG", "ACDEFBG", "ABCDEFG", false); // no change - assertOrder("ABCDEFG", "ACDEFBXZG", "XZABCDEFG", false); // Z and X - assertOrder("ABCDEFG", "AXCDEZFBG", "XZABCDEFG", false); // Z and X + gap - } - - @Test public void testStableOrdering() { - when(mNotifPipelineFlags.isSemiStableSortEnabled()).thenReturn(true); mStabilityManager.setAllowEntryReordering(false); // No input or output assertOrder("", "", "", true); @@ -2076,7 +2052,6 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Test public void testActiveOrdering() { - when(mNotifPipelineFlags.isSemiStableSortEnabled()).thenReturn(true); assertOrder("ABCDEFG", "ACDEFXBG", "ACDEFXBG", true); // X assertOrder("ABCDEFG", "ACDEFBG", "ACDEFBG", true); // no change assertOrder("ABCDEFG", "ACDEFBXZG", "ACDEFBXZG", true); // Z and X @@ -2133,7 +2108,6 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Test public void stableOrderingDisregardedWithSectionChange() { - when(mNotifPipelineFlags.isSemiStableSortEnabled()).thenReturn(true); // GIVEN the first sectioner's packages can be changed from run-to-run List<String> mutableSectionerPackages = new ArrayList<>(); mutableSectionerPackages.add(PACKAGE_1); @@ -2229,49 +2203,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { } @Test - public void groupRevertingToSummaryDoesNotRetainStablePositionWithLegacyIndexLogic() { - when(mNotifPipelineFlags.isStabilityIndexFixEnabled()).thenReturn(false); - - // 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 (incorrectly) moves to the top of the section where it is ranked, - // despite visual stability being active - verifyBuiltList( - notif(2), - notif(0), - notif(1) - ); - } - - @Test public void groupRevertingToSummaryRetainsStablePosition() { - when(mNotifPipelineFlags.isStabilityIndexFixEnabled()).thenReturn(true); - // GIVEN a notification group is on screen mStabilityManager.setAllowEntryReordering(false); |