[Notifications] Notify NotifCollectionListeners when a pending entry has
been removed. The NotifCollectionListener (listener for the new pipeline) specifies that #onEntryRemoved should be called when a notification has been cancelled, regardless of whether or not it has been inflated. This CL adds a call to that method to ensure that specification is met. Test: atest + New unit tests Fixes: 201097913 Change-Id: I337e5a942b1f46b51102a698dad90fbfa82d66ca
This commit is contained in:
@@ -305,9 +305,6 @@ public class NotificationEntryManager implements
|
||||
NotificationEntry entry = mPendingNotifications.get(key);
|
||||
entry.abortTask();
|
||||
mPendingNotifications.remove(key);
|
||||
for (NotifCollectionListener listener : mNotifCollectionListeners) {
|
||||
listener.onEntryCleanUp(entry);
|
||||
}
|
||||
mLogger.logInflationAborted(key, "pending", reason);
|
||||
}
|
||||
NotificationEntry addedEntry = getActiveNotificationUnfiltered(key);
|
||||
@@ -477,6 +474,18 @@ public class NotificationEntryManager implements
|
||||
if (!lifetimeExtended) {
|
||||
// At this point, we are guaranteed the notification will be removed
|
||||
abortExistingInflation(key, "removeNotification");
|
||||
// Fix for b/201097913: NotifCollectionListener#onEntryRemoved specifies that
|
||||
// #onEntryRemoved should be called when a notification is cancelled,
|
||||
// regardless of whether the notification was pending or active.
|
||||
// Note that mNotificationEntryListeners are NOT notified of #onEntryRemoved
|
||||
// because for that interface, #onEntryRemoved should only be called for
|
||||
// active entries, NOT pending ones.
|
||||
for (NotifCollectionListener listener : mNotifCollectionListeners) {
|
||||
listener.onEntryRemoved(pendingEntry, REASON_UNKNOWN);
|
||||
}
|
||||
for (NotifCollectionListener listener : mNotifCollectionListeners) {
|
||||
listener.onEntryCleanUp(pendingEntry);
|
||||
}
|
||||
mAllNotifications.remove(pendingEntry);
|
||||
mLeakDetector.trackGarbage(pendingEntry);
|
||||
}
|
||||
|
||||
@@ -32,6 +32,7 @@ import static org.junit.Assert.assertFalse;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyBoolean;
|
||||
import static org.mockito.ArgumentMatchers.anyInt;
|
||||
import static org.mockito.ArgumentMatchers.argThat;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.atLeastOnce;
|
||||
import static org.mockito.Mockito.doAnswer;
|
||||
@@ -81,6 +82,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationRankin
|
||||
import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder;
|
||||
import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy;
|
||||
import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
|
||||
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
|
||||
import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
|
||||
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
|
||||
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
|
||||
@@ -94,6 +96,7 @@ import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.ArgumentMatcher;
|
||||
import org.mockito.InOrder;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
@@ -120,6 +123,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
|
||||
@Mock private KeyguardEnvironment mEnvironment;
|
||||
@Mock private ExpandableNotificationRow mRow;
|
||||
@Mock private NotificationEntryListener mEntryListener;
|
||||
@Mock private NotifCollectionListener mNotifCollectionListener;
|
||||
@Mock private NotificationRemoveInterceptor mRemoveInterceptor;
|
||||
@Mock private HeadsUpManager mHeadsUpManager;
|
||||
@Mock private RankingMap mRankingMap;
|
||||
@@ -215,6 +219,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
|
||||
mEnvironment));
|
||||
mEntryManager.setUpWithPresenter(mPresenter);
|
||||
mEntryManager.addNotificationEntryListener(mEntryListener);
|
||||
mEntryManager.addCollectionListener(mNotifCollectionListener);
|
||||
mEntryManager.addNotificationRemoveInterceptor(mRemoveInterceptor);
|
||||
|
||||
setUserSentiment(mSbn.getKey(), Ranking.USER_SENTIMENT_NEUTRAL);
|
||||
@@ -318,13 +323,20 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
|
||||
eq(mEntry), any(), eq(false) /* removedByUser */, eq(UNDEFINED_DISMISS_REASON));
|
||||
}
|
||||
|
||||
/** Regression test for b/201097913. */
|
||||
@Test
|
||||
public void testRemoveNotification_whilePending() {
|
||||
public void testRemoveNotification_whilePending_onlyCollectionListenerNotified() {
|
||||
// Add and then remove a pending entry (entry that hasn't been inflated).
|
||||
mEntryManager.addNotification(mSbn, mRankingMap);
|
||||
mEntryManager.removeNotification(mSbn.getKey(), mRankingMap, UNDEFINED_DISMISS_REASON);
|
||||
|
||||
// Verify that only the listener for the NEW pipeline is notified.
|
||||
// Old pipeline:
|
||||
verify(mEntryListener, never()).onEntryRemoved(
|
||||
eq(mEntry), any(), eq(false /* removedByUser */), eq(UNDEFINED_DISMISS_REASON));
|
||||
argThat(matchEntryOnSbn()), any(), anyBoolean(), anyInt());
|
||||
// New pipeline:
|
||||
verify(mNotifCollectionListener).onEntryRemoved(
|
||||
argThat(matchEntryOnSbn()), anyInt());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -639,6 +651,11 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
|
||||
PendingIntent.FLAG_IMMUTABLE)).build();
|
||||
}
|
||||
|
||||
// TODO(b/201321631): Update more tests to use this function instead of eq(mEntry).
|
||||
private ArgumentMatcher<NotificationEntry> matchEntryOnSbn() {
|
||||
return e -> e.getSbn().equals(mSbn);
|
||||
}
|
||||
|
||||
private static class FakeNotificationLifetimeExtender implements NotificationLifetimeExtender {
|
||||
private NotificationSafeToRemoveCallback mCallback;
|
||||
private boolean mExtendLifetimes = true;
|
||||
|
||||
Reference in New Issue
Block a user