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

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

Summaries receive a stability section index 

This fixes an issue where the summary with no children wouldn't have its sort position stabilized, resulting in unintended move animations while the shade is open when child notifications were cancelled.

Bug: 241229236
Test: atest ShadeListBuilderTest
Change-Id: I742d68ce05e080f3b7a928247e46d8406f21eebb
parent dfb618d8

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

+3 −1
Original line number Diff line number Diff line
@@ -73,7 +73,9 @@ public class Flags { 


    public static final UnreleasedFlag NOTIFICATION_DISMISSAL_FADE = new UnreleasedFlag(113, true);



    // next id: 114

    public static final UnreleasedFlag STABILITY_INDEX_FIX = new UnreleasedFlag(114, true);



    // next id: 115



    /***************************************/

    // 200 - keyguard/lockscreen

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

+4 −0
Original line number Diff line number Diff line
@@ -53,4 +53,8 @@ class NotifPipelineFlags @Inject constructor( 


    fun fullScreenIntentRequiresKeyguard(): Boolean =

        featureFlags.isEnabled(Flags.FSI_REQUIRES_KEYGUARD)



    val isStabilityIndexFixEnabled: Boolean by lazy {

        featureFlags.isEnabled(Flags.STABILITY_INDEX_FIX)

    }

}

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

+26 −10
Original line number Diff line number Diff line
@@ -96,6 +96,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { 
    // used exclusivly by ShadeListBuilder#notifySectionEntriesUpdated

    // TODO replace temp with collection pool for readability

    private final ArrayList<ListEntry> mTempSectionMembers = new ArrayList<>();

    private final NotifPipelineFlags mFlags;

    private final boolean mAlwaysLogList;



    private List<ListEntry> mNotifList = new ArrayList<>();
@@ -141,6 +142,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { 
    ) {

        mSystemClock = systemClock;

        mLogger = logger;

        mFlags = flags;

        mAlwaysLogList = flags.isDevLoggingEnabled();

        mInteractionTracker = interactionTracker;

        mChoreographer = pipelineChoreographer;
@@ -1036,29 +1038,43 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable { 
    /**

     * Assign the index of each notification relative to the total order

     */

    private static void assignIndexes(List<ListEntry> notifList) {

    private void assignIndexes(List<ListEntry> notifList) {

        if (notifList.size() == 0) return;

        NotifSection currentSection = requireNonNull(notifList.get(0).getSection());

        int sectionMemberIndex = 0;

        for (int i = 0; i < notifList.size(); i++) {

            ListEntry entry = notifList.get(i);

            final ListEntry entry = notifList.get(i);

            NotifSection section = requireNonNull(entry.getSection());

            if (section.getIndex() != currentSection.getIndex()) {

                sectionMemberIndex = 0;

                currentSection = section;

            }

            entry.getAttachState().setStableIndex(sectionMemberIndex);

            if (mFlags.isStabilityIndexFixEnabled()) {

                entry.getAttachState().setStableIndex(sectionMemberIndex++);

                if (entry instanceof GroupEntry) {

                GroupEntry parent = (GroupEntry) entry;

                for (int j = 0; j < parent.getChildren().size(); j++) {

                    entry = parent.getChildren().get(j);

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

                    }

                }

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

                    sectionMemberIndex++;

                if (entry instanceof GroupEntry) {

                    final GroupEntry parent = (GroupEntry) entry;

                    for (NotificationEntry child : parent.getChildren()) {

                        child.getAttachState().setStableIndex(sectionMemberIndex++);

                    }

                }

                sectionMemberIndex++;

            }

        }

    }



    private void freeEmptyGroups() {

        Trace.beginSection("ShadeListBuilder.freeEmptyGroups");

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

+80 −0
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ 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;
@@ -2112,6 +2113,85 @@ 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);



        // 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 stays in the same location on rebuild

        verifyBuiltList(

                notif(0),

                notif(1),

                notif(2)

        );

    }



    private static void setNewRank(NotificationEntry entry, int rank) {

        entry.setRanking(new RankingBuilder(entry.getRanking()).setRank(rank).build());

    }