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

Commit 97532973 authored by Beverly's avatar Beverly Committed by Beverly Tai
Browse files

Update mReadOnlyNotifList in ShadeListBuilder 

So that mReadOnlyNotifList always tracks mNotifList, even after
switching mNotifList and mNewNotifList

This CL also updates ShadeListBuilderTest to catch bugs like this.
Mockito keeps shallow references when verifying mocks, so we can't rely
on verifying the passed list after the test is run (since the list is
recycled within ShadeListBuilder). Instead, make a deep copy of the list
we receive in the listener and verify this list for equality.

Test: atest ShadeListBuilderTest
Fixes: 148421960
Change-Id: I85eb439f77686c67b365f04585da8fc54cb854ec
parent 9abb186c

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

+19 −12
Original line number Diff line number Diff line
@@ -94,7 +94,8 @@ public class ShadeListBuilder implements Dumpable { 
            new ArrayList<>();

    @Nullable private OnRenderListListener mOnRenderListListener;



    private final List<ListEntry> mReadOnlyNotifList = Collections.unmodifiableList(mNotifList);

    private List<ListEntry> mReadOnlyNotifList = Collections.unmodifiableList(mNotifList);

    private List<ListEntry> mReadOnlyNewNotifList = Collections.unmodifiableList(mNewNotifList);



    @Inject

    public ShadeListBuilder(
@@ -252,17 +253,6 @@ public class ShadeListBuilder implements Dumpable { 
        rebuildListIfBefore(STATE_SORTING);

    }



    /**

     * Points mNotifList to the list stored in mNewNotifList.

     * Reuses the (emptied) mNotifList as mNewNotifList.

     */

    private void applyNewNotifList() {

        mNotifList.clear();

        List<ListEntry> emptyList = mNotifList;

        mNotifList = mNewNotifList;

        mNewNotifList = emptyList;

    }



    /**

     * The core algorithm of the pipeline. See the top comment in {@link NotifPipeline} for

     * details on our contracts with other code.
@@ -333,6 +323,23 @@ public class ShadeListBuilder implements Dumpable { 
        mIterationCount++;

    }



    /**

     * Points mNotifList to the list stored in mNewNotifList.

     * Reuses the (emptied) mNotifList as mNewNotifList.

     *

     * Accordingly, updates the ReadOnlyNotifList pointers.

     */

    private void applyNewNotifList() {

        mNotifList.clear();

        List<ListEntry> emptyList = mNotifList;

        mNotifList = mNewNotifList;

        mNewNotifList = emptyList;



        List<ListEntry> readOnlyNotifList = mReadOnlyNotifList;

        mReadOnlyNotifList = mReadOnlyNewNotifList;

        mReadOnlyNewNotifList = readOnlyNotifList;

    }



    private void resetNotifs() {

        for (GroupEntry group : mGroups.values()) {

            group.setPreviousParent(group.getParent());

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

+90 −9
Original line number Diff line number Diff line
@@ -853,7 +853,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
    public void testGroupTransformEntries() {

        // GIVEN a registered OnBeforeTransformGroupsListener

        RecordingOnBeforeTransformGroupsListener listener =

                spy(new RecordingOnBeforeTransformGroupsListener());

                new RecordingOnBeforeTransformGroupsListener();

        mListBuilder.addOnBeforeTransformGroupsListener(listener);



        // GIVEN some new notifs
@@ -879,12 +879,12 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
        );



        // THEN all the new notifs, including the new GroupEntry, are passed to the listener

        verify(listener).onBeforeTransformGroups(

        assertEquals(

                Arrays.asList(

                        mEntrySet.get(0),

                        mBuiltList.get(1),

                        mEntrySet.get(4)

                )

                        mEntrySet.get(4)),

                listener.mEntriesReceived

        );

    }
@@ -925,13 +925,71 @@ public class ShadeListBuilderTest extends SysuiTestCase { 
        );



        // THEN all the new notifs, including the new GroupEntry, are passed to the listener

        verify(listener).onBeforeTransformGroups(

        assertEquals(

                Arrays.asList(

                        mEntrySet.get(0),

                        mEntrySet.get(1),

                        mBuiltList.get(2),

                        mEntrySet.get(7)

                )

                        mEntrySet.get(7),

                        mEntrySet.get(1)),

                listener.mEntriesReceived

        );

    }



    @Test

    public void testDispatchListOnBeforeSort() {

        // GIVEN a registered OnBeforeSortListener

        RecordingOnBeforeSortListener listener =

                new RecordingOnBeforeSortListener();

        mListBuilder.addOnBeforeSortListener(listener);

        mListBuilder.setComparators(Arrays.asList(new HypeComparator(PACKAGE_3)));



        // GIVEN some new notifs out of order

        addNotif(0, PACKAGE_1);

        addNotif(1, PACKAGE_2);

        addNotif(2, PACKAGE_3);



        // WHEN we run the pipeline

        dispatchBuild();



        // THEN all the new notifs are passed to the listener out of order

        assertEquals(

                Arrays.asList(

                        mEntrySet.get(0),

                        mEntrySet.get(1),

                        mEntrySet.get(2)),

                listener.mEntriesReceived

        );



        // THEN the final list is in order

        verifyBuiltList(

                notif(2),

                notif(0),

                notif(1)

        );

    }



    @Test

    public void testDispatchListOnBeforeRender() {

        // GIVEN a registered OnBeforeRenderList

        RecordingOnBeforeRenderistener listener =

                new RecordingOnBeforeRenderistener();

        mListBuilder.addOnBeforeRenderListListener(listener);



        // GIVEN some new notifs out of order

        addNotif(0, PACKAGE_1);

        addNotif(1, PACKAGE_2);

        addNotif(2, PACKAGE_3);



        // WHEN we run the pipeline

        dispatchBuild();



        // THEN all the new notifs are passed to the listener

        assertEquals(

                Arrays.asList(

                        mEntrySet.get(0),

                        mEntrySet.get(1),

                        mEntrySet.get(2)),

                listener.mEntriesReceived

        );

    }
@@ -1346,9 +1404,32 @@ public class ShadeListBuilderTest extends SysuiTestCase { 


    private static class RecordingOnBeforeTransformGroupsListener

            implements OnBeforeTransformGroupsListener {

        List<ListEntry> mEntriesReceived;



        @Override

        public void onBeforeTransformGroups(List<ListEntry> list) {

            mEntriesReceived = new ArrayList<>(list);

        }

    }



    private static class RecordingOnBeforeSortListener

            implements OnBeforeSortListener {

        List<ListEntry> mEntriesReceived;



        @Override

        public void onBeforeTransformGroups(List<ListEntry> list) { }

        public void onBeforeSort(List<ListEntry> list) {

            mEntriesReceived = new ArrayList<>(list);

        }

    }



    private static class RecordingOnBeforeRenderistener

            implements OnBeforeRenderListListener {

        List<ListEntry> mEntriesReceived;



        @Override

        public void onBeforeRenderList(List<ListEntry> list) {

            mEntriesReceived = new ArrayList<>(list);

        }

    }



    private static final String PACKAGE_1 = "com.test1";