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

Commit f5adb86e authored by Jeff DeCew's avatar Jeff DeCew
Browse files

New pipeline: Separate stability rules for group pruning from group changing 

Group changes for alerting children are allowed under the stability manager rules, but we were using the same method to determine if the group as a whole could be pruned, but that shouldn't actually be allowed for alerting summaries - it results in the weird unstable animation (and is a regression from S).

Though I made this change more cleanly, the effective delta of this change is just a boolean logic change from:
  isGroupChangeAllowed = isReorderingAllowed || isEntryAlerting
to:
  isGroupChangeAllowed = isReorderingAllowed || (isEntryAlerting && !isPruning)

Fixes: 230734197
Test: atest VisualStabilityCoordinatorTest ShadeListBuilderTest
Change-Id: Ib1a927de2b4cc9b614be29deaf9754c2dc79390f
parent 44e14e72

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

+1 −1
Original line number Diff line number Diff line
@@ -749,7 +749,7 @@ public class ShadeListBuilder implements Dumpable { 
                        continue;

                    }

                    if (group.wasAttachedInPreviousPass()

                            && !getStabilityManager().isGroupChangeAllowed(group.getSummary())) {

                            && !getStabilityManager().isGroupPruneAllowed(group)) {

                        checkState(!children.isEmpty(), "empty group should have been pruned");

                        // This group was previously attached and group changes aren't

                        //  allowed; keep it around until group changes are allowed again.

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

+8 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import com.android.systemui.dagger.SysUISingleton; 
import com.android.systemui.dump.DumpManager;

import com.android.systemui.keyguard.WakefulnessLifecycle;

import com.android.systemui.plugins.statusbar.StatusBarStateController;

import com.android.systemui.statusbar.notification.collection.GroupEntry;

import com.android.systemui.statusbar.notification.collection.ListEntry;

import com.android.systemui.statusbar.notification.collection.NotifPipeline;

import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -138,6 +139,13 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable, 
                    return isGroupChangeAllowedForEntry;

                }



                @Override

                public boolean isGroupPruneAllowed(@NonNull GroupEntry entry) {

                    final boolean isGroupPruneAllowedForEntry = mReorderingAllowed;

                    mIsSuppressingGroupChange |= !isGroupPruneAllowedForEntry;

                    return isGroupPruneAllowedForEntry;

                }



                @Override

                public boolean isSectionChangeAllowed(@NonNull NotificationEntry entry) {

                    final boolean isSectionChangeAllowedForEntry =

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.kt

+10 −0
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */

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



import com.android.systemui.statusbar.notification.collection.GroupEntry

import com.android.systemui.statusbar.notification.collection.ListEntry

import com.android.systemui.statusbar.notification.collection.NotificationEntry
@@ -51,6 +52,14 @@ abstract class NotifStabilityManager protected constructor(name: String) : 
     */

    abstract fun isGroupChangeAllowed(entry: NotificationEntry): Boolean



    /**

     * Returns whether this notification group can be pruned for not having enough children.

     * Per iteration of the notification pipeline, locally stores this information until the next

     * run of the pipeline. When this method returns false, it's expected that a group prune for

     * this entry is being suppressed.

     */

    abstract fun isGroupPruneAllowed(entry: GroupEntry): Boolean



    /**

     * Returns whether this notification entry can currently change sections.

     * Per iteration of the notification pipeline, locally stores this information until the next
@@ -89,6 +98,7 @@ object DefaultNotifStabilityManager : NotifStabilityManager("DefaultNotifStabili 
    override fun isPipelineRunAllowed(): Boolean = true

    override fun onBeginRun() {}

    override fun isGroupChangeAllowed(entry: NotificationEntry): Boolean = true

    override fun isGroupPruneAllowed(entry: GroupEntry): Boolean = true

    override fun isSectionChangeAllowed(entry: NotificationEntry): Boolean = true

    override fun isEntryReorderingAllowed(entry: ListEntry): Boolean = true

    override fun isEveryChangeAllowed(): Boolean = true

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

+14 −1
Original line number Diff line number Diff line
@@ -1439,14 +1439,16 @@ public class ShadeListBuilderTest extends SysuiTestCase { 


        // WHEN a new child is added and the old one gets filtered while group changes are disabled.

        mStabilityManager.setAllowGroupChanges(false);

        mStabilityManager.setAllowGroupPruning(false);

        mFinalizeFilter.mIndicesToFilter.add(1);

        addGroupChild(2, PACKAGE_1, GROUP_1);



        dispatchBuild();



        // THEN the new child should be shown without a group

        // (Note that this is the same as the expected result if there were no stability rules.)

        verifyBuiltList(

                notif(2)  // previously promoted child

                notif(2)  // new child

        );

    }
@@ -2269,6 +2271,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
    private static class TestableStabilityManager extends NotifStabilityManager {

        boolean mAllowPipelineRun = true;

        boolean mAllowGroupChanges = true;

        boolean mAllowGroupPruning = true;

        boolean mAllowSectionChanges = true;

        boolean mAllowEntryReodering = true;
@@ -2281,6 +2284,11 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
            return this;

        }



        TestableStabilityManager setAllowGroupPruning(boolean allowGroupPruning) {

            mAllowGroupPruning = allowGroupPruning;

            return this;

        }



        TestableStabilityManager setAllowSectionChanges(boolean allowSectionChanges) {

            mAllowSectionChanges = allowSectionChanges;

            return this;
@@ -2310,6 +2318,11 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
            return mAllowGroupChanges;

        }



        @Override

        public boolean isGroupPruneAllowed(@NonNull GroupEntry entry) {

            return mAllowGroupPruning;

        }



        @Override

        public boolean isSectionChangeAllowed(@NonNull NotificationEntry entry) {

            return mAllowSectionChanges;

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

+17 −0
Original line number Diff line number Diff line
@@ -35,6 +35,8 @@ import com.android.systemui.SysuiTestCase; 
import com.android.systemui.dump.DumpManager;

import com.android.systemui.keyguard.WakefulnessLifecycle;

import com.android.systemui.plugins.statusbar.StatusBarStateController;

import com.android.systemui.statusbar.notification.collection.GroupEntry;

import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder;

import com.android.systemui.statusbar.notification.collection.NotifPipeline;

import com.android.systemui.statusbar.notification.collection.NotificationEntry;

import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder;
@@ -83,6 +85,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
    private NotifPanelEvents.Listener mNotifPanelEventsCallback;

    private NotifStabilityManager mNotifStabilityManager;

    private NotificationEntry mEntry;

    private GroupEntry mGroupEntry;



    @Before

    public void setUp() {
@@ -117,6 +120,10 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
                .setPkg("testPkg1")

                .build();



        mGroupEntry = new GroupEntryBuilder()

                .setSummary(mEntry)

                .build();



        when(mHeadsUpManager.isAlerting(mEntry.getKey())).thenReturn(false);



        // Whenever we invalidate, the pipeline runs again, so we invalidate the state
@@ -135,6 +142,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes are allowed

        assertTrue(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertTrue(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are allowed

        assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -149,6 +157,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes are allowed

        assertTrue(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertTrue(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are allowed

        assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -163,6 +172,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes are NOT allowed

        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are NOT allowed

        assertFalse(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -176,6 +186,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes are NOT allowed

        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are NOT allowed

        assertFalse(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -190,6 +201,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes are NOT allowed

        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are NOT allowed

        assertFalse(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -208,6 +220,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


        // THEN group changes aren't allowed

        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // THEN section changes are allowed for this notification but not other notifications

        assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry));
@@ -321,6 +334,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
        setPanelExpanded(true);



        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));



        // WHEN the panel isn't expanded anymore

        setPanelExpanded(false);
@@ -422,6 +436,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
        setPanelExpanded(true);

        setPulsing(true);

        assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));

        assertFalse(mNotifStabilityManager.isSectionChangeAllowed(mEntry));



        // GIVEN mEntry is a HUN
@@ -431,6 +446,8 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
        assertTrue(mNotifStabilityManager.isGroupChangeAllowed(mEntry));

        assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry));



        // BUT pruning the group for which this is the summary would still NOT be allowed.

        assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry));

    }



    private void setActivityLaunching(boolean activityLaunching) {