Change onRemoteEntry() to only fire when entries are removed

Conceptually, this should be a paired match with
onNotificationAdded(). Previously we were firing this method even
if the notification was no longer present (already removed) or
if it hadn't been added yet (was in pending).

Test: atest

Change-Id: I613f60aa8cf4e1aeb7bb13ff5883a221c9b623c6
This commit is contained in:
Ned Burns
2019-01-02 16:48:08 -05:00
parent 1a08863f3b
commit ef2ef6c909
13 changed files with 54 additions and 66 deletions

View File

@@ -61,14 +61,9 @@ public class ForegroundServiceNotificationListener {
@Override
public void onEntryRemoved(
NotificationData.Entry entry,
String key,
StatusBarNotification old,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
if (entry != null && !lifetimeExtended) {
removeNotification(entry.notification);
}
removeNotification(entry.notification);
}
});
}

View File

@@ -37,7 +37,6 @@ import android.media.session.PlaybackState;
import android.os.Handler;
import android.os.Trace;
import android.os.UserHandle;
import android.service.notification.StatusBarNotification;
import android.util.Log;
import android.view.View;
import android.widget.ImageView;
@@ -157,15 +156,10 @@ public class NotificationMediaManager implements Dumpable {
notificationEntryManager.addNotificationEntryListener(new NotificationEntryListener() {
@Override
public void onEntryRemoved(
@Nullable Entry entry,
String key,
StatusBarNotification old,
Entry entry,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
if (!lifetimeExtended) {
onNotificationRemoved(key);
}
onNotificationRemoved(entry.key);
}
});
}

View File

@@ -254,13 +254,10 @@ public class NotificationRemoteInputManager implements Dumpable {
@Override
public void onEntryRemoved(
@Nullable NotificationData.Entry entry,
String key,
StatusBarNotification old,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
if (removedByUser && entry != null) {
onPerformRemoveNotification(entry, key);
onPerformRemoveNotification(entry, entry.key);
}
}
});

View File

@@ -20,7 +20,6 @@ import static com.android.systemui.statusbar.NotificationRemoteInputManager.FORC
import static com.android.systemui.statusbar.notification.row.NotificationInflater.FLAG_CONTENT_VIEW_AMBIENT;
import static com.android.systemui.statusbar.notification.row.NotificationInflater.FLAG_CONTENT_VIEW_HEADS_UP;
import android.annotation.Nullable;
import android.app.Notification;
import android.service.notification.StatusBarNotification;
import android.util.Log;
@@ -83,13 +82,10 @@ public class NotificationAlertingManager {
@Override
public void onEntryRemoved(
@Nullable NotificationData.Entry entry,
String key,
StatusBarNotification old,
NotificationData.Entry entry,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
stopAlerting(key);
stopAlerting(entry.key);
}
});
}

View File

@@ -71,20 +71,14 @@ public interface NotificationEntryListener {
* because the developer retracted it).
* @param entry notification data entry that was removed. Null if no entry existed for the
* removed key at the time of removal.
* @param key key of notification that was removed
* @param old StatusBarNotification of the notification before it was removed
* @param visibility logging data related to the visibility of the notification at the time of
* removal, if it was removed by a user action. Null if it was not removed by
* a user action.
* @param lifetimeExtended true if something is artificially extending how long the notification
* @param removedByUser true if the notification was removed by a user action
*/
default void onEntryRemoved(
@Nullable NotificationData.Entry entry,
String key,
StatusBarNotification old,
NotificationData.Entry entry,
@Nullable NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
}
}

View File

@@ -153,6 +153,12 @@ public class NotificationEntryManager implements
return mNotificationRowBinder;
}
// TODO: Remove this once we can always use a mocked row binder in our tests
@VisibleForTesting
void setRowBinder(NotificationRowBinder notificationRowBinder) {
mNotificationRowBinder = notificationRowBinder;
}
public void setUpWithPresenter(NotificationPresenter presenter,
NotificationListContainer listContainer,
HeadsUpManager headsUpManager) {
@@ -309,7 +315,6 @@ public class NotificationEntryManager implements
abortExistingInflation(key);
StatusBarNotification old = null;
boolean lifetimeExtended = false;
if (entry != null) {
@@ -342,12 +347,12 @@ public class NotificationEntryManager implements
// Let's remove the children if this was a summary
handleGroupSummaryRemoved(key);
old = removeNotificationViews(key, ranking);
}
}
removeNotificationViews(key, ranking);
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryRemoved(entry, key, old, visibility, lifetimeExtended, removedByUser);
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryRemoved(entry, visibility, removedByUser);
}
}
}
}

View File

@@ -15,7 +15,6 @@
*/
package com.android.systemui.statusbar.notification.logging;
import android.annotation.Nullable;
import android.content.Context;
import android.os.Handler;
import android.os.RemoteException;
@@ -168,14 +167,11 @@ public class NotificationLogger implements StateListener {
entryManager.addNotificationEntryListener(new NotificationEntryListener() {
@Override
public void onEntryRemoved(
@Nullable NotificationData.Entry entry,
String key,
StatusBarNotification old,
NotificationData.Entry entry,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
if (removedByUser && visibility != null && entry != null) {
logNotificationClear(key, entry.notification, visibility);
if (removedByUser && visibility != null) {
logNotificationClear(entry.key, entry.notification, visibility);
}
}

View File

@@ -220,15 +220,12 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis
@Override
public void onEntryRemoved(
@Nullable Entry entry,
String key,
StatusBarNotification old,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
// Removes any alerts pending on this entry. Note that this will not stop any inflation
// tasks started by a transfer, so this should only be used as clean-up for when
// inflation is stopped and the pending alert no longer needs to happen.
mPendingAlerts.remove(key);
mPendingAlerts.remove(entry.key);
}
};

View File

@@ -196,14 +196,10 @@ public class StatusBarNotificationPresenter implements NotificationPresenter,
@Override
public void onEntryRemoved(
@Nullable Entry entry,
String key,
StatusBarNotification old,
NotificationVisibility visibility,
boolean lifetimeExtended,
boolean removedByUser) {
if (!lifetimeExtended) {
StatusBarNotificationPresenter.this.onNotificationRemoved(key, old);
}
StatusBarNotificationPresenter.this.onNotificationRemoved(
entry.key, entry.notification);
if (removedByUser) {
maybeEndAmbientPulse();
}

View File

@@ -392,7 +392,7 @@ public class ForegroundServiceControllerTest extends SysuiTestCase {
private void entryRemoved(StatusBarNotification notification) {
mEntryListener.onEntryRemoved(new NotificationData.Entry(notification),
null, null, null, false, false);
null, false);
}
private void entryAdded(StatusBarNotification notification, int importance) {

View File

@@ -121,6 +121,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
@Mock private MetricsLogger mMetricsLogger;
@Mock private SmartReplyController mSmartReplyController;
@Mock private RowInflaterTask mAsyncInflationTask;
@Mock private NotificationRowBinder mMockedRowBinder;
private NotificationData.Entry mEntry;
private StatusBarNotification mSbn;
@@ -310,8 +311,8 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
verify(mListContainer).cleanUpViewStateForEntry(mEntry);
verify(mPresenter).updateNotificationViews();
verify(mEntryListener).onEntryRemoved(mEntry, mSbn.getKey(), mSbn,
null, false /* lifetimeExtended */, false /* removedByUser */);
verify(mEntryListener).onEntryRemoved(
mEntry, null, false /* removedByUser */);
verify(mRow).setRemoved();
assertNull(mEntryManager.getNotificationData().get(mSbn.getKey()));
@@ -335,8 +336,31 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
assertNotNull(mEntryManager.getNotificationData().get(mSbn.getKey()));
verify(extender).setShouldManageLifetime(mEntry, true /* shouldManage */);
verify(mEntryListener).onEntryRemoved(mEntry, mSbn.getKey(), null,
null, true /* lifetimeExtended */, false /* removedByUser */);
verify(mEntryListener, never()).onEntryRemoved(
mEntry, null, false /* removedByUser */);
}
@Test
public void testRemoveNotification_onEntryRemoveNotFiredIfEntryDoesntExist() {
com.android.systemui.util.Assert.isNotMainThread();
mEntryManager.removeNotification("not_a_real_key", mRankingMap);
verify(mEntryListener, never()).onEntryRemoved(
mEntry, null, false /* removedByUser */);
}
@Test
public void testRemoveNotification_whilePending() throws InterruptedException {
com.android.systemui.util.Assert.isNotMainThread();
mEntryManager.setRowBinder(mMockedRowBinder);
mEntryManager.addNotification(mSbn, mRankingMap);
mEntryManager.removeNotification(mSbn.getKey(), mRankingMap);
verify(mEntryListener, never()).onEntryRemoved(
mEntry, null, false /* removedByUser */);
}
@Test

View File

@@ -159,11 +159,6 @@ public class NotificationLoggerTest extends SysuiTestCase {
verify(mBarService, times(1)).onNotificationVisibilityChanged(any(), any());
}
@Test
public void testHandleNullEntryOnEntryRemoved() {
mNotificationEntryListener.onEntryRemoved(null, "foobar", null, null, false, false);
}
private class TestableNotificationLogger extends NotificationLogger {
TestableNotificationLogger(NotificationListener notificationListener,

View File

@@ -237,8 +237,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase {
mGroupManager.onEntryAdded(summaryEntry);
mGroupManager.onEntryAdded(childEntry);
mNotificationEntryListener.onEntryRemoved(childEntry, childEntry.key, null, null,
false, false);
mNotificationEntryListener.onEntryRemoved(childEntry, null, false);
assertFalse(mGroupAlertTransferHelper.isAlertTransferPending(childEntry));
}