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

Commit 558fb9cc authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Drop ranking update events when another one is already queued. 

Fixes: 207737509
Test: atest NotificationListenerTest
Change-Id: I5c082de30b7b40fc72c14dd3087df0853eb62531
parent 6f0493a3

packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java

+60 −12
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.app.NotificationChannel; 
import android.app.NotificationManager;

import android.content.ComponentName;

import android.content.Context;

import android.os.Handler;

import android.os.RemoteException;

import android.os.UserHandle;

import android.service.notification.StatusBarNotification;
@@ -35,9 +34,13 @@ import com.android.systemui.dagger.qualifiers.Main; 
import com.android.systemui.statusbar.dagger.StatusBarModule;

import com.android.systemui.statusbar.phone.NotificationListenerWithPlugins;

import com.android.systemui.statusbar.phone.StatusBar;

import com.android.systemui.util.time.SystemClock;



import java.util.ArrayList;

import java.util.Deque;

import java.util.List;

import java.util.concurrent.ConcurrentLinkedDeque;

import java.util.concurrent.Executor;



/**

 * This class handles listening to notification updates and passing them along to
@@ -47,23 +50,31 @@ import java.util.List; 
public class NotificationListener extends NotificationListenerWithPlugins {

    private static final String TAG = "NotificationListener";

    private static final boolean DEBUG = StatusBar.DEBUG;

    private static final long MAX_RANKING_DELAY_MILLIS = 500L;



    private final Context mContext;

    private final NotificationManager mNotificationManager;

    private final Handler mMainHandler;

    private final SystemClock mSystemClock;

    private final Executor mMainExecutor;

    private final List<NotificationHandler> mNotificationHandlers = new ArrayList<>();

    private final ArrayList<NotificationSettingsListener> mSettingsListeners = new ArrayList<>();



    private final Deque<RankingMap> mRankingMapQueue = new ConcurrentLinkedDeque<>();

    private final Runnable mDispatchRankingUpdateRunnable = this::dispatchRankingUpdate;

    private long mSkippingRankingUpdatesSince = -1;



    /**

     * Injected constructor. See {@link StatusBarModule}.

     */

    public NotificationListener(

            Context context,

            NotificationManager notificationManager,

            @Main Handler mainHandler) {

            SystemClock systemClock,

            @Main Executor mainExecutor) {

        mContext = context;

        mNotificationManager = notificationManager;

        mMainHandler = mainHandler;

        mSystemClock = systemClock;

        mMainExecutor = mainExecutor;

    }



    /** Registers a listener that's notified when notifications are added/removed/etc. */
@@ -89,7 +100,7 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
            return;

        }

        final RankingMap currentRanking = getCurrentRanking();

        mMainHandler.post(() -> {

        mMainExecutor.execute(() -> {

            // There's currently a race condition between the calls to getActiveNotifications() and

            // getCurrentRanking(). It's possible for the ranking that we store here to not contain

            // entries for every notification in getActiveNotifications(). To prevent downstream
@@ -119,7 +130,7 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
            final RankingMap rankingMap) {

        if (DEBUG) Log.d(TAG, "onNotificationPosted: " + sbn);

        if (sbn != null && !onPluginNotificationPosted(sbn, rankingMap)) {

            mMainHandler.post(() -> {

            mMainExecutor.execute(() -> {

                processForRemoteInput(sbn.getNotification(), mContext);



                for (NotificationHandler handler : mNotificationHandlers) {
@@ -134,7 +145,7 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
            int reason) {

        if (DEBUG) Log.d(TAG, "onNotificationRemoved: " + sbn + " reason: " + reason);

        if (sbn != null && !onPluginNotificationRemoved(sbn, rankingMap)) {

            mMainHandler.post(() -> {

            mMainExecutor.execute(() -> {

                for (NotificationHandler handler : mNotificationHandlers) {

                    handler.onNotificationRemoved(sbn, rankingMap, reason);

                }
@@ -151,13 +162,50 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
    public void onNotificationRankingUpdate(final RankingMap rankingMap) {

        if (DEBUG) Log.d(TAG, "onRankingUpdate");

        if (rankingMap != null) {

            // Add the ranking to the queue, then run dispatchRankingUpdate() on the main thread

            RankingMap r = onPluginRankingUpdate(rankingMap);

            mMainHandler.post(() -> {

            mRankingMapQueue.addLast(r);

            // Maintaining our own queue and always posting the runnable allows us to guarantee the

            //  relative ordering of all events which are dispatched, which is important so that the

            //  RankingMap always has exactly the same elements that are current, per add/remove

            //  events.

            mMainExecutor.execute(mDispatchRankingUpdateRunnable);

        }

    }



    /**

     * This method is (and must be) the sole consumer of the RankingMap queue.  After pulling an

     * object off the queue, it checks if the queue is empty, and only dispatches the ranking update

     * if the queue is still empty.

     */

    private void dispatchRankingUpdate() {

        if (DEBUG) Log.d(TAG, "dispatchRankingUpdate");

        RankingMap r = mRankingMapQueue.pollFirst();

        if (r == null) {

            Log.wtf(TAG, "mRankingMapQueue was empty!");

        }

        if (!mRankingMapQueue.isEmpty()) {

            final long now = mSystemClock.elapsedRealtime();

            if (mSkippingRankingUpdatesSince == -1) {

                mSkippingRankingUpdatesSince = now;

            }

            final long timeSkippingRankingUpdates = now - mSkippingRankingUpdatesSince;

            if (timeSkippingRankingUpdates < MAX_RANKING_DELAY_MILLIS) {

                if (DEBUG) {

                    Log.d(TAG, "Skipping dispatch of onNotificationRankingUpdate() -- "

                            + mRankingMapQueue.size() + " more updates already in the queue.");

                }

                return;

            }

            if (DEBUG) {

                Log.d(TAG, "Proceeding with dispatch of onNotificationRankingUpdate() -- "

                        + mRankingMapQueue.size() + " more updates already in the queue.");

            }

        }

        mSkippingRankingUpdatesSince = -1;

        for (NotificationHandler handler : mNotificationHandlers) {

            handler.onNotificationRankingUpdate(r);

        }

            });

        }

    }



    @Override
@@ -165,7 +213,7 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
            String pkgName, UserHandle user, NotificationChannel channel, int modificationType) {

        if (DEBUG) Log.d(TAG, "onNotificationChannelModified");

        if (!onPluginNotificationChannelModified(pkgName, user, channel, modificationType)) {

            mMainHandler.post(() -> {

            mMainExecutor.execute(() -> {

                for (NotificationHandler handler : mNotificationHandlers) {

                    handler.onNotificationChannelModified(pkgName, user, channel, modificationType);

                }

packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java

+3 −2
Original line number Diff line number Diff line
@@ -168,9 +168,10 @@ public interface StatusBarDependenciesModule { 
    static NotificationListener provideNotificationListener(

            Context context,

            NotificationManager notificationManager,

            @Main Handler mainHandler) {

            SystemClock systemClock,

            @Main Executor mainExecutor) {

        return new NotificationListener(

                context, notificationManager, mainHandler);

                context, notificationManager, systemClock, mainExecutor);

    }



    /** */

packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationListenerWithPlugins.java

+6 −7
Original line number Diff line number Diff line
@@ -66,11 +66,7 @@ public class NotificationListenerWithPlugins extends NotificationListenerService 


    @Override

    public RankingMap getCurrentRanking() {

        RankingMap currentRanking = super.getCurrentRanking();

        for (NotificationListenerController plugin : mPlugins) {

            currentRanking = plugin.getCurrentRanking(currentRanking);

        }

        return currentRanking;

        return onPluginRankingUpdate(super.getCurrentRanking());

    }



    public void onPluginConnected() {
@@ -120,8 +116,11 @@ public class NotificationListenerWithPlugins extends NotificationListenerService 
        return false;

    }



    public RankingMap onPluginRankingUpdate(RankingMap rankingMap) {

        return getCurrentRanking();

    protected RankingMap onPluginRankingUpdate(RankingMap rankingMap) {

        for (NotificationListenerController plugin : mPlugins) {

            rankingMap = plugin.getCurrentRanking(rankingMap);

        }

        return rankingMap;

    }



    @Override

packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationListenerTest.java

+59 −10
Original line number Diff line number Diff line
@@ -16,27 +16,30 @@ 


package com.android.systemui.statusbar;



import static org.mockito.ArgumentMatchers.any;

import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verifyNoMoreInteractions;

import static org.mockito.Mockito.when;



import android.app.Notification;

import android.app.NotificationManager;

import android.os.Handler;

import android.os.UserHandle;

import android.service.notification.NotificationListenerService.Ranking;

import android.service.notification.NotificationListenerService.RankingMap;

import android.service.notification.StatusBarNotification;

import android.testing.AndroidTestingRunner;

import android.testing.TestableLooper;



import androidx.test.filters.SmallTest;



import com.android.systemui.SysuiTestCase;

import com.android.systemui.statusbar.NotificationListener.NotificationHandler;

import com.android.systemui.util.concurrency.FakeExecutor;

import com.android.systemui.util.time.FakeSystemClock;



import org.junit.Before;

import org.junit.Test;
@@ -46,7 +49,6 @@ import org.mockito.MockitoAnnotations; 


@SmallTest

@RunWith(AndroidTestingRunner.class)

@TestableLooper.RunWithLooper

public class NotificationListenerTest extends SysuiTestCase {

    private static final String TEST_PACKAGE_NAME = "test";

    private static final int TEST_UID = 0;
@@ -54,6 +56,8 @@ public class NotificationListenerTest extends SysuiTestCase { 
    @Mock private NotificationHandler mNotificationHandler;

    @Mock private NotificationManager mNotificationManager;



    private final FakeSystemClock mFakeSystemClock = new FakeSystemClock();

    private final FakeExecutor mFakeExecutor = new FakeExecutor(mFakeSystemClock);

    private NotificationListener mListener;

    private StatusBarNotification mSbn;

    private RankingMap mRanking = new RankingMap(new Ranking[0]);
@@ -65,7 +69,8 @@ public class NotificationListenerTest extends SysuiTestCase { 
        mListener = new NotificationListener(

                mContext,

                mNotificationManager,

                new Handler(TestableLooper.get(this).getLooper()));

                mFakeSystemClock,

                mFakeExecutor);

        mSbn = new StatusBarNotification(TEST_PACKAGE_NAME, TEST_PACKAGE_NAME, 0, null, TEST_UID, 0,

                new Notification(), UserHandle.CURRENT, null, 0);
@@ -75,23 +80,67 @@ public class NotificationListenerTest extends SysuiTestCase { 
    @Test

    public void testNotificationAddCallsAddNotification() {

        mListener.onNotificationPosted(mSbn, mRanking);

        TestableLooper.get(this).processAllMessages();

        mFakeExecutor.runAllReady();

        verify(mNotificationHandler).onNotificationPosted(mSbn, mRanking);

    }



    @Test

    public void testNotificationRemovalCallsRemoveNotification() {

        mListener.onNotificationRemoved(mSbn, mRanking);

        TestableLooper.get(this).processAllMessages();

        mFakeExecutor.runAllReady();

        verify(mNotificationHandler).onNotificationRemoved(eq(mSbn), eq(mRanking), anyInt());

    }



    @Test

    public void testRankingUpdateCallsNotificationRankingUpdate() {

        mListener.onNotificationRankingUpdate(mRanking);

        TestableLooper.get(this).processAllMessages();

        // RankingMap may be modified by plugins.

        verify(mNotificationHandler).onNotificationRankingUpdate(any());

        assertThat(mFakeExecutor.runAllReady()).isEqualTo(1);

        verify(mNotificationHandler).onNotificationRankingUpdate(eq(mRanking));

    }



    @Test

    public void testRankingUpdateMultipleTimesCallsNotificationRankingUpdateOnce() {

        // GIVEN multiple notification ranking updates

        RankingMap ranking1 = mock(RankingMap.class);

        RankingMap ranking2 = mock(RankingMap.class);

        RankingMap ranking3 = mock(RankingMap.class);

        mListener.onNotificationRankingUpdate(ranking1);

        mListener.onNotificationRankingUpdate(ranking2);

        mListener.onNotificationRankingUpdate(ranking3);



        // WHEN executor runs with multiple updates in the queue

        assertThat(mFakeExecutor.numPending()).isEqualTo(3);

        assertThat(mFakeExecutor.runAllReady()).isEqualTo(3);



        // VERIFY that only the last ranking actually gets handled

        verify(mNotificationHandler, never()).onNotificationRankingUpdate(eq(ranking1));

        verify(mNotificationHandler, never()).onNotificationRankingUpdate(eq(ranking2));

        verify(mNotificationHandler).onNotificationRankingUpdate(eq(ranking3));

        verifyNoMoreInteractions(mNotificationHandler);

    }



    @Test

    public void testRankingUpdateWillCallAgainIfQueueIsSlow() {

        // GIVEN multiple notification ranking updates

        RankingMap ranking1 = mock(RankingMap.class);

        RankingMap ranking2 = mock(RankingMap.class);

        RankingMap ranking3 = mock(RankingMap.class);

        mListener.onNotificationRankingUpdate(ranking1);

        mListener.onNotificationRankingUpdate(ranking2);

        mListener.onNotificationRankingUpdate(ranking3);



        // WHEN executor runs with a 1-second gap between handling events 1 and 2

        assertThat(mFakeExecutor.numPending()).isEqualTo(3);

        assertThat(mFakeExecutor.runNextReady()).isTrue();

        // delay a second, which empties the executor

        mFakeSystemClock.advanceTime(1000);

        assertThat(mFakeExecutor.numPending()).isEqualTo(0);



        // VERIFY that both event 2 and event 3 are called

        verify(mNotificationHandler, never()).onNotificationRankingUpdate(eq(ranking1));

        verify(mNotificationHandler).onNotificationRankingUpdate(eq(ranking2));

        verify(mNotificationHandler).onNotificationRankingUpdate(eq(ranking3));

        verifyNoMoreInteractions(mNotificationHandler);

    }



    @Test