Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 19e4fd3a authored by Steve Elliott's avatar Steve Elliott
Browse files

visual stability for bundles and children 

Bug: 394483200
Test: make
Test: atest SystemUITests
Flag: com.android.systemui.notification_bundle_ui
Change-Id: Ie1d4365e2e0bc28d62969fc5cc30add33b57df62
parent ff1874ce

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());