From 7bc9be7158c8732ffccd54c5238cab1d24c2b4f9 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Wed, 6 Mar 2019 18:42:05 -0800 Subject: Fixed an issue where notifications weren't dismissed properly Whenever the blocking helper was showing, the notification wasn't actually dismissed in the notification manager and therefore the notification dot and other visuals could be wrong as a result. The reason was that we were lifetime extenders could even trigger when the notification was dismissed. Test: atest SystemUITest Bug: 125799998 Change-Id: Iac7e690f609414de83eefcfeb38623d7668691c3 --- .../statusbar/NotificationRemoteInputManager.java | 9 --------- .../notification/NotificationEntryManager.java | 23 +++++++++++++--------- .../notification/collection/NotificationEntry.java | 8 -------- .../notification/NotificationEntryManagerTest.java | 10 +++++----- 4 files changed, 19 insertions(+), 31 deletions(-) (limited to 'packages') diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java index b820dc09657a..a630e49d850a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java @@ -461,9 +461,6 @@ public class NotificationRemoteInputManager implements Dumpable { } public boolean shouldKeepForRemoteInputHistory(NotificationEntry entry) { - if (entry.isDismissed()) { - return false; - } if (!FORCE_REMOTE_INPUT_HISTORY) { return false; } @@ -471,9 +468,6 @@ public class NotificationRemoteInputManager implements Dumpable { } public boolean shouldKeepForSmartReplyHistory(NotificationEntry entry) { - if (entry.isDismissed()) { - return false; - } if (!FORCE_REMOTE_INPUT_HISTORY) { return false; } @@ -661,9 +655,6 @@ public class NotificationRemoteInputManager implements Dumpable { protected class RemoteInputActiveExtender extends RemoteInputExtender { @Override public boolean shouldExtendLifetime(@NonNull NotificationEntry entry) { - if (entry.isDismissed()) { - return false; - } return mRemoteInputController.isRemoteInputActive(entry); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index 4ed9ae4d5142..7d224fb13e2d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -159,16 +159,19 @@ public class NotificationEntryManager implements } public void performRemoveNotification(StatusBarNotification n) { - final int rank = mNotificationData.getRank(n.getKey()); - final int count = mNotificationData.getActiveNotifications().size(); - NotificationVisibility.NotificationLocation location = - NotificationLogger.getNotificationLocation(getNotificationData().get(n.getKey())); - final NotificationVisibility nv = NotificationVisibility.obtain(n.getKey(), rank, count, - true, location); + final NotificationVisibility nv = obtainVisibility(n.getKey()); removeNotificationInternal( n.getKey(), null, nv, false /* forceRemove */, true /* removedByUser */); } + private NotificationVisibility obtainVisibility(String key) { + final int rank = mNotificationData.getRank(key); + final int count = mNotificationData.getActiveNotifications().size(); + NotificationVisibility.NotificationLocation location = + NotificationLogger.getNotificationLocation(getNotificationData().get(key)); + return NotificationVisibility.obtain(key, rank, count, true, location); + } + private void abortExistingInflation(String key) { if (mPendingNotifications.containsKey(key)) { NotificationEntry entry = mPendingNotifications.get(key); @@ -226,8 +229,8 @@ public class NotificationEntryManager implements @Override public void removeNotification(String key, NotificationListenerService.RankingMap ranking) { - removeNotificationInternal( - key, ranking, null, false /* forceRemove */, false /* removedByUser */); + removeNotificationInternal(key, ranking, obtainVisibility(key), false /* forceRemove */, + false /* removedByUser */); } private void removeNotificationInternal( @@ -245,7 +248,8 @@ public class NotificationEntryManager implements if (entry != null) { // If a manager needs to keep the notification around for whatever reason, we // keep the notification - if (!forceRemove) { + boolean entryDismissed = entry.isRowDismissed(); + if (!forceRemove && !entryDismissed) { for (NotificationLifetimeExtender extender : mNotificationLifetimeExtenders) { if (extender.shouldExtendLifetime(entry)) { mLatestRankingMap = ranking; @@ -272,6 +276,7 @@ public class NotificationEntryManager implements mNotificationData.remove(key, ranking); updateNotifications(); Dependency.get(LeakDetector.class).trackGarbage(entry); + removedByUser |= entryDismissed; for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onEntryRemoved(entry, visibility, removedByUser); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java index 5cd1210f7c9f..f1373d142402 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java @@ -543,14 +543,6 @@ public final class NotificationEntry { return row == null || row.isRemoved(); } - /** - * @return {@code true} if the row is null or dismissed - */ - public boolean isDismissed() { - //TODO: recycling - return row == null || row.isDismissed(); - } - public boolean isRowPinned() { return row != null && row.isPinned(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java index 554374432135..c8005ddacdb1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java @@ -347,7 +347,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { verify(mPresenter).updateNotificationViews(); verify(mEntryListener).onEntryRemoved( - mEntry, null, false /* removedByUser */); + eq(mEntry), any(), eq(false) /* removedByUser */); verify(mRow).setRemoved(); assertNull(mEntryManager.getNotificationData().get(mSbn.getKey())); @@ -360,7 +360,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { mEntryManager.removeNotification("not_a_real_key", mRankingMap); verify(mEntryListener, never()).onEntryRemoved( - mEntry, null, false /* removedByUser */); + eq(mEntry), any(), eq(false) /* removedByUser */); } @Test @@ -373,7 +373,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { mEntryManager.removeNotification(mSbn.getKey(), mRankingMap); verify(mEntryListener, never()).onEntryRemoved( - mEntry, null, false /* removedByUser */); + eq(mEntry), any(), eq(false /* removedByUser */)); } @Test @@ -455,7 +455,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { verify(extender).setShouldManageLifetime(mEntry, true); // THEN the notification is retained assertNotNull(mEntryManager.getNotificationData().get(mSbn.getKey())); - verify(mEntryListener, never()).onEntryRemoved(mEntry, null, false); + verify(mEntryListener, never()).onEntryRemoved(eq(mEntry), any(), eq(false)); } @Test @@ -474,7 +474,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { // THEN the notification is removed assertNull(mEntryManager.getNotificationData().get(mSbn.getKey())); - verify(mEntryListener).onEntryRemoved(mEntry, null, false); + verify(mEntryListener).onEntryRemoved(eq(mEntry), any(), eq(false)); } @Test -- cgit v1.2.3-59-g8ed1b