Merge "Drop ranking update events when another one is already queued."
This commit is contained in:
@@ -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,12 +162,49 @@ 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(() -> {
|
||||
for (NotificationHandler handler : mNotificationHandlers) {
|
||||
handler.onNotificationRankingUpdate(r);
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/** */
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user