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

Commit 4d103c11 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Clean up STABILITY_INDEX_FIX and SEMI_STABLE_SORT flags which have already been released. 

Test: atest ShadeListBuilderTest
Fixes: 259558771
Fixes: 259559750
Bug: 241229236
Change-Id: If11142b11abdd201eb5d065ce9832d4a9e91869a
parent a09626e7

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

+0 −6
Original line number Diff line number Diff line
@@ -79,12 +79,6 @@ object Flags { 
    val NOTIFICATION_DISMISSAL_FADE =

        unreleasedFlag(113, "notification_dismissal_fade", teamfood = true)



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