Merge "Clean up STABILITY_INDEX_FIX and SEMI_STABLE_SORT flags which have... (40c815bc) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/flags/Flags.kt

+0 −6

Original line number	Diff line number	Diff line
		@@ -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)

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt

+0 −8

Original line number	Diff line number	Diff line
		@@ -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)
		}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt

+2 −4

Original line number	Diff line number	Diff line
		@@ -98,14 +98,12 @@ 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
		}
		}

		companion object {
		@JvmStatic

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java

+11 −94

Original line number	Diff line number	Diff line
		@@ -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();
		}
		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,7 +1059,6 @@ 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;
		@@ -1112,18 +1070,6 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
		child.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++);
		}
		}
		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)) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java

+0 −68

Original line number	Diff line number	Diff line
		@@ -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,
		@@ -1997,30 +1995,8 @@ public class ShadeListBuilderTest extends SysuiTestCase {
		return wtf;
		}

		@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);
		@@ -2228,50 +2202,8 @@ 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);