Merge "visual stability for bundles and children" into main (b05725a7) · Commits · e / os / android_frameworks_base

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.kt

+18 −18

Original line number	Diff line number	Diff line
		@@ -169,7 +169,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isTrue()

		// THEN section changes are allowed
		@@ -186,7 +186,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -203,7 +203,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -220,7 +220,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isTrue()

		// THEN section changes are allowed
		@@ -237,7 +237,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -254,7 +254,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -271,7 +271,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(false)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -287,7 +287,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(true)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -304,7 +304,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setPulsing(true)

		// THEN group changes are NOT allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are NOT allowed
		@@ -328,7 +328,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		)

		// THEN group changes aren't allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// THEN section changes are allowed for this notification but not other notifications
		@@ -504,7 +504,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setSleepy(false)
		setPanelExpanded(true)

		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()

		// WHEN the panel isn't expanded anymore
		@@ -699,7 +699,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		setSleepy(false)
		setPanelExpanded(true)
		setPulsing(true)
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isFalse()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isFalse()
		assertThat(notifStabilityManager.isSectionChangeAllowed(entry)).isFalse()

		@@ -707,7 +707,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		headsUpRepository.setHeadsUpKeys(entry.key)

		// THEN group + section changes are allowed
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isSectionChangeAllowed(entry)).isTrue()

		// BUT pruning the group for which this is the summary would still NOT be allowed.
		@@ -724,7 +724,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa

		// THEN
		assertThat(notifStabilityManager.isEveryChangeAllowed()).isTrue()
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isTrue()
		assertThat(notifStabilityManager.isSectionChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isEntryReorderingAllowed(entry)).isTrue()
		@@ -743,7 +743,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa

		// THEN
		assertThat(notifStabilityManager.isEveryChangeAllowed()).isTrue()
		assertThat(notifStabilityManager.isGroupChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isGroupPruneAllowed(groupEntry)).isTrue()
		assertThat(notifStabilityManager.isSectionChangeAllowed(entry)).isTrue()
		assertThat(notifStabilityManager.isEntryReorderingAllowed(entry)).isTrue()
		@@ -856,7 +856,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		.thenReturn(true)

		// THEN
		assertThat(notifStabilityManager.isGroupChangeAllowed(headsUpGroupSummary)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(headsUpGroupSummary)).isFalse()
		assertThat(notifStabilityManager.isEntryReorderingAllowed(headsUpGroupSummary))
		.isFalse()
		assertThat(notifStabilityManager.isSectionChangeAllowed(headsUpGroupSummary)).isFalse()
		@@ -892,7 +892,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		whenever(childEntry.parent).thenReturn(nonHeadsUpGroupEntry)

		// THEN
		assertThat(notifStabilityManager.isGroupChangeAllowed(childEntry)).isTrue()
		assertThat(notifStabilityManager.isParentChangeAllowed(childEntry)).isTrue()
		assertThat(notifStabilityManager.isSectionChangeAllowed(childEntry)).isTrue()
		assertThat(notifStabilityManager.isEntryReorderingAllowed(nonHeadsUpGroupEntry))
		.isTrue()
		@@ -932,7 +932,7 @@ class VisualStabilityCoordinatorTest(flags: FlagsParameterization) : SysuiTestCa
		.thenReturn(true)

		// THEN
		assertThat(notifStabilityManager.isGroupChangeAllowed(childEntry)).isFalse()
		assertThat(notifStabilityManager.isParentChangeAllowed(childEntry)).isFalse()
		assertThat(notifStabilityManager.isSectionChangeAllowed(childEntry)).isFalse()
		assertThat(notifStabilityManager.isEntryReorderingAllowed(childEntry)).isFalse()
		}

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

+10 −1

Original line number	Diff line number	Diff line
		@@ -35,17 +35,26 @@ class BundleEntry(spec: BundleSpec) : PipelineEntry(spec.key) {

		var row: ExpandableNotificationRow? = null

		private val _children: MutableList<ListEntry> = ArrayList()
		private val _children = ArrayList<ListEntry>()

		/**
		* Modifiable list of children for this bundle. You should prefer [children] to this property.
		*/
		@InternalNotificationsApi val rawChildren: MutableList<ListEntry> = _children

		val children: List<ListEntry> = Collections.unmodifiableList(_children)

		@InternalNotificationsApi
		fun addChild(child: ListEntry) {
		_children.add(child)
		}

		@InternalNotificationsApi
		fun removeChild(child: ListEntry) {
		_children.remove(child)
		}

		@InternalNotificationsApi
		fun clearChildren() {
		_children.clear()
		}

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

0 → 100644

+21 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2025 The Android Open Source Project
		*
		* Licensed under the Apache License, Version 2.0 (the "License");
		* you may not use this file except in compliance with the License.
		* You may obtain a copy of the License at
		*
		* http://www.apache.org/licenses/LICENSE-2.0
		*
		* Unless required by applicable law or agreed to in writing, software
		* distributed under the License is distributed on an "AS IS" BASIS,
		* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		package com.android.systemui.statusbar.notification.collection

		@RequiresOptIn(message = "This API is meant only for internal use by the notifications pipeline.")
		@Retention(AnnotationRetention.BINARY)
		annotation class InternalNotificationsApi

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

+68 −28

Original line number	Diff line number	Diff line
		@@ -42,6 +42,7 @@ import android.util.ArraySet;
		import android.util.Log;

		import androidx.annotation.NonNull;
		import androidx.annotation.OptIn;
		import androidx.annotation.VisibleForTesting;

		import com.android.systemui.Dumpable;
		@@ -795,34 +796,56 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
		Trace.endSection();
		}

		@OptIn(markerClass = InternalNotificationsApi.class) // for BundleEntry#getRawChildren()
		private void stabilizeGroupingNotifs(List<PipelineEntry> topLevelList) {
		if (getStabilityManager().isEveryChangeAllowed()) {
		return;
		}
		Trace.beginSection("ShadeListBuilder.stabilizeGroupingNotifs");

		for (int i = 0; i < topLevelList.size(); i++) {
		final ListEntry tle = topLevelList.get(i).asListEntry();
		if (tle == null) {
		// Promoters ignore bundles so we don't have to demote any here.
		continue;
		final PipelineEntry tle = topLevelList.get(i);
		if (tle instanceof BundleEntry bundleEntry) {
		// maybe put bundle children back into their old parents (including moving back to
		// top-level)
		final List<ListEntry> bundleChildren = bundleEntry.getRawChildren();
		final int bundleSize = bundleChildren.size();
		for (int j = 0; j < bundleSize; j++) {
		final ListEntry child = bundleChildren.get(j);
		if (maybeSuppressParentChange(child, topLevelList)) {
		bundleChildren.remove(j);
		j--;
		}
		if (child instanceof GroupEntry groupEntry) {
		// maybe put group children back into their old parents (including moving
		// back to top-level)
		final List<NotificationEntry> groupChildren = groupEntry.getRawChildren();
		final int groupSize = groupChildren.size();
		for (int k = 0; k < groupSize; k++) {
		if (maybeSuppressParentChange(groupChildren.get(k), topLevelList)) {
		// child was put back into its previous parent, so we remove it from
		// this group
		groupChildren.remove(k);
		k--;
		}
		if (tle instanceof GroupEntry) {
		// maybe put children back into their old group (including moving back to top-level)
		GroupEntry groupEntry = (GroupEntry) tle;
		}
		}
		}
		} else if (tle instanceof GroupEntry groupEntry) {
		// maybe put children back into their old parents (including moving back to
		// top-level)
		List<NotificationEntry> children = groupEntry.getRawChildren();
		for (int j = 0; j < groupEntry.getChildren().size(); j++) {
		if (maybeSuppressGroupChange(children.get(j), topLevelList)) {
		// child was put back into its previous group, so we remove it from this
		if (maybeSuppressParentChange(children.get(j), topLevelList)) {
		// child was put back into its previous parent, so we remove it from this
		// group
		children.remove(j);
		j--;
		}
		}
		} else if (tle instanceof NotificationEntry) {
		// maybe put top-level-entries back into their previous groups
		if (maybeSuppressGroupChange(tle.getRepresentativeEntry(), topLevelList)) {
		// entry was put back into its previous group, so we remove it from the list of
		} else if (tle instanceof NotificationEntry notifEntry) {
		// maybe put top-level-entries back into their previous parents
		if (maybeSuppressParentChange(notifEntry, topLevelList)) {
		// entry was put back into its previous parent, so we remove it from the list of
		// top-level-entries
		topLevelList.remove(i);
		i--;
		@@ -833,9 +856,9 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
		}

		/**
		* Returns true if the group change was suppressed, else false
		* Returns true if the parent change was suppressed, else false
		*/
		private boolean maybeSuppressGroupChange(NotificationEntry entry, List<PipelineEntry> out) {
		private boolean maybeSuppressParentChange(ListEntry entry, List<PipelineEntry> out) {
		final PipelineEntry prevParent = entry.getPreviousAttachState().getParent();
		if (prevParent == null) {
		// New entries are always allowed.
		@@ -854,20 +877,37 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
		}
		// TODO: Rather than perform "half" of the move here and require the caller remove the child
		// from the assignedParent, ideally we would have an atomic "move" operation.
		if (!getStabilityManager().isGroupChangeAllowed(entry.getRepresentativeEntry())) {
		if (entry instanceof NotificationEntry notifEntry) {
		if (!getStabilityManager().isParentChangeAllowed(notifEntry)) {
		entry.getAttachState().getSuppressedChanges().setParent(assignedParent);
		entry.setParent(prevParent);
		if (prevParent == ROOT_ENTRY) {
		out.add(entry);
		} else if (prevParent instanceof GroupEntry) {
		((GroupEntry) prevParent).addChild(entry);
		if (!mGroups.containsKey(prevParent.getKey())) {
		mGroups.put(prevParent.getKey(), (GroupEntry) prevParent);
		} else if (prevParent instanceof GroupEntry groupEntry) {
		groupEntry.addChild(notifEntry);
		if (!mGroups.containsKey(groupEntry.getKey())) {
		mGroups.put(groupEntry.getKey(), groupEntry);
		}
		} else if (prevParent instanceof BundleEntry bundleEntry) {
		bundleEntry.addChild(entry);
		}
		// TODO(b/394483200): Revisit group stability for BundleEntry
		return true;
		}
		} else if (entry instanceof GroupEntry groupEntry) {
		if (!getStabilityManager().isParentChangeAllowed(groupEntry)) {
		entry.getAttachState().getSuppressedChanges().setParent(assignedParent);
		entry.setParent(prevParent);
		if (prevParent == ROOT_ENTRY) {
		out.add(entry);
		} else if (prevParent instanceof BundleEntry bundleEntry) {
		bundleEntry.addChild(entry);
		} else {
		throw new IllegalStateException("GroupEntry " + groupEntry.getKey()
		+ " was previously attached to illegal parent: " + prevParent.getKey());
		}
		return true;
		}
		}
		return false;
		}

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

+15 −9

Original line number	Diff line number	Diff line
		@@ -102,7 +102,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		private boolean mPipelineRunAllowed;
		private boolean mReorderingAllowed;
		private boolean mIsSuppressingPipelineRun = false;
		private boolean mIsSuppressingGroupChange = false;
		private boolean mIsSuppressingParentChange = false;
		private final Set<String> mEntriesWithSuppressedSectionChange = new HashSet<>();
		private boolean mIsSuppressingEntryReorder = false;

		@@ -311,7 +311,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		@Override
		public void onBeginRun() {
		mIsSuppressingPipelineRun = false;
		mIsSuppressingGroupChange = false;
		mIsSuppressingParentChange = false;
		mEntriesWithSuppressedSectionChange.clear();
		mIsSuppressingEntryReorder = false;
		}
		@@ -323,7 +323,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		}

		@Override
		public boolean isGroupChangeAllowed(@NonNull NotificationEntry entry) {
		public boolean isParentChangeAllowed(@NonNull NotificationEntry entry) {
		final boolean isGroupChangeAllowedForEntry;
		if (StabilizeHeadsUpGroup.isEnabled()) {
		isGroupChangeAllowedForEntry =
		@@ -334,10 +334,17 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		isGroupChangeAllowedForEntry = mReorderingAllowed
		\|\| canMoveForHeadsUp(entry);
		}
		mIsSuppressingGroupChange \|= !isGroupChangeAllowedForEntry;
		mIsSuppressingParentChange \|= !isGroupChangeAllowedForEntry;
		return isGroupChangeAllowedForEntry;
		}

		@Override
		public boolean isParentChangeAllowed(@NonNull GroupEntry entry) {
		final boolean isBundleChangeAllowedForGroup = isEveryChangeAllowed();
		mIsSuppressingParentChange \|= !isBundleChangeAllowedForGroup;
		return isBundleChangeAllowedForGroup;
		}

		@Override
		public boolean isGroupPruneAllowed(@NonNull GroupEntry entry) {
		boolean isGroupPruneAllowedForEntry;
		@@ -348,7 +355,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		isGroupPruneAllowedForEntry = mReorderingAllowed;
		}

		mIsSuppressingGroupChange \|= !isGroupPruneAllowedForEntry;
		mIsSuppressingParentChange \|= !isGroupPruneAllowedForEntry;
		return isGroupPruneAllowedForEntry;
		}

		@@ -384,7 +391,6 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		return true;
		}

		// TODO(b/395698521): Handle BundleEntry
		return canReorderNotificationEntry(notificationEntry)
		\|\| canMoveForHeadsUp(notificationEntry);
		} else {
		@@ -462,11 +468,11 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		private void maybeInvalidateList() {
		if (mPipelineRunAllowed && mIsSuppressingPipelineRun) {
		mNotifStabilityManager.invalidateList("pipeline run suppression ended");
		} else if (mReorderingAllowed && (mIsSuppressingGroupChange
		} else if (mReorderingAllowed && (mIsSuppressingParentChange
		\|\| isSuppressingSectionChange()
		\|\| mIsSuppressingEntryReorder)) {
		final String reason = "reorder suppression ended for"
		+ " group=" + mIsSuppressingGroupChange
		+ " group=" + mIsSuppressingParentChange
		+ " section=" + isSuppressingSectionChange()
		+ " sort=" + mIsSuppressingEntryReorder;
		mNotifStabilityManager.invalidateList(reason);
		@@ -581,7 +587,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
		pw.println(" pulsing: " + mPulsing);
		pw.println(" communalShowing: " + mCommunalShowing);
		pw.println("isSuppressingPipelineRun: " + mIsSuppressingPipelineRun);
		pw.println("isSuppressingGroupChange: " + mIsSuppressingGroupChange);
		pw.println("isSuppressingGroupChange: " + mIsSuppressingParentChange);
		pw.println("isSuppressingEntryReorder: " + mIsSuppressingEntryReorder);
		if (StabilizeHeadsUpGroup.isEnabled()) {
		pw.println("headsUpGroupKeys: " + mHeadsUpGroupKeys.size());