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

Commit be04ad40 authored by Jay Aliomer's avatar Jay Aliomer
Browse files

Sort after all notifications are filtered 

Filtering alters the list of notifications' order.
So Sorting needed to happen after the filtering

Fixes: 196408434
Fixes: 196408153

Test: ShadeListBuilderTest
Change-Id: Id1f1da9faf6f1a50da03b12b2acec9ac724c08a6
parent 9d9d325e

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

+10 −11
Original line number Diff line number Diff line
@@ -28,8 +28,6 @@ import static com.android.systemui.statusbar.notification.collection.listbuilder 
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_SORTING;

import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_TRANSFORMING;



import static java.util.Objects.requireNonNull;



import android.annotation.MainThread;

import android.annotation.Nullable;

import android.util.ArrayMap;
@@ -344,14 +342,8 @@ public class ShadeListBuilder implements Dumpable { 
        mPipelineState.incrementTo(STATE_GROUP_STABILIZING);

        stabilizeGroupingNotifs(mNotifList);



        // Step 5: Sort

        // Assign each top-level entry a section, then sort the list by section and then within

        // section by our list of custom comparators

        dispatchOnBeforeSort(mReadOnlyNotifList);

        mPipelineState.incrementTo(STATE_SORTING);

        sortList();



        // Step 6: Filter out entries after pre-group filtering, grouping, promoting and sorting

        // Step 5: Filter out entries after pre-group filtering, grouping and promoting

        // Now filters can see grouping information to determine whether to filter or not.

        dispatchOnBeforeFinalizeFilter(mReadOnlyNotifList);

        mPipelineState.incrementTo(STATE_FINALIZE_FILTERING);
@@ -359,6 +351,13 @@ public class ShadeListBuilder implements Dumpable { 
        applyNewNotifList();

        pruneIncompleteGroups(mNotifList);



        // Step 6: Sort

        // Assign each top-level entry a section, then sort the list by section and then within

        // section by our list of custom comparators

        dispatchOnBeforeSort(mReadOnlyNotifList);

        mPipelineState.incrementTo(STATE_SORTING);

        sortList();



        // Step 7: Lock in our group structure and log anything that's changed since the last run

        mPipelineState.incrementTo(STATE_FINALIZING);

        logChanges();
@@ -837,8 +836,8 @@ public class ShadeListBuilder implements Dumpable { 
    private final Comparator<ListEntry> mTopLevelComparator = (o1, o2) -> {



        int cmp = Integer.compare(

                requireNonNull(o1.getSection()).getIndex(),

                requireNonNull(o2.getSection()).getIndex());

                o1.getSectionIndex(),

                o2.getSectionIndex());



        if (cmp == 0) {

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

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

+3 −3
Original line number Diff line number Diff line
@@ -82,8 +82,8 @@ public class PipelineState { 
    public static final int STATE_GROUPING = 4;

    public static final int STATE_TRANSFORMING = 5;

    public static final int STATE_GROUP_STABILIZING = 6;

    public static final int STATE_SORTING = 7;

    public static final int STATE_FINALIZE_FILTERING = 8;

    public static final int STATE_FINALIZE_FILTERING = 7;

    public static final int STATE_SORTING = 8;

    public static final int STATE_FINALIZING = 9;



    @IntDef(prefix = { "STATE_" }, value = {
@@ -94,8 +94,8 @@ public class PipelineState { 
            STATE_GROUPING,

            STATE_TRANSFORMING,

            STATE_GROUP_STABILIZING,

            STATE_SORTING,

            STATE_FINALIZE_FILTERING,

            STATE_SORTING,

            STATE_FINALIZING,

    })

    @Retention(RetentionPolicy.SOURCE)

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

+3 −0
Original line number Diff line number Diff line
@@ -53,6 +53,9 @@ class ShadeViewManager constructor( 
            notifList.firstOrNull()?.section?.headerController?.let {

                children.add(NodeSpecImpl(this, it))

            }

            notifList.firstOrNull()?.let {

                children.add(buildNotifNode(it, this))

            }

            notifList.asSequence().zipWithNext().forEach { (prev, entry) ->

                // Insert new header if the section has changed between two entries

                entry.section.takeIf { it != prev.section }?.headerController?.let {

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

+25 −5
Original line number Diff line number Diff line
@@ -802,13 +802,13 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
                .onBeforeTransformGroups(anyList());

        inOrder.verify(promoter, atLeastOnce())

                .shouldPromoteToTopLevel(any(NotificationEntry.class));

        inOrder.verify(mOnBeforeFinalizeFilterListener).onBeforeFinalizeFilter(anyList());

        inOrder.verify(preRenderFilter, atLeastOnce())

                .shouldFilterOut(any(NotificationEntry.class), anyLong());

        inOrder.verify(mOnBeforeSortListener).onBeforeSort(anyList());

        inOrder.verify(section, atLeastOnce()).isInSection(any(ListEntry.class));

        inOrder.verify(comparator, atLeastOnce())

                .compare(any(ListEntry.class), any(ListEntry.class));

        inOrder.verify(mOnBeforeFinalizeFilterListener).onBeforeFinalizeFilter(anyList());

        inOrder.verify(preRenderFilter, atLeastOnce())

                .shouldFilterOut(any(NotificationEntry.class), anyLong());

        inOrder.verify(mOnBeforeRenderListListener).onBeforeRenderList(anyList());

        inOrder.verify(mOnRenderListListener).onRenderList(anyList());

    }
@@ -1045,12 +1045,32 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
        // because group changes aren't allowed by the stability manager

        verifyBuiltList(

                notif(0),

                notif(2),

                group(

                        summary(3),

                        child(4),

                        child(5)

                ),

                notif(2)

                )

        );

    }



    @Test

    public void testBrokenGroupNotificationOrdering() {

        // GIVEN two group children with different sections & without a summary yet



        addGroupChild(0, PACKAGE_2, GROUP_1);

        addNotif(1, PACKAGE_1);

        addGroupChild(2, PACKAGE_2, GROUP_1);

        addGroupChild(3, PACKAGE_2, GROUP_1);



        dispatchBuild();



        // THEN all notifications are not grouped and posted in order by index

        verifyBuiltList(

                notif(0),

                notif(1),

                notif(2),

                notif(3)

        );

    }