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

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java
index b820dc0..a630e49 100644 (file)
--- 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 4ed9ae4..7d224fb 100644 (file)
--- 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 5cd1210..f1373d1 100644 (file)
--- 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 5543744..c8005dd 100644 (file)
--- 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