diff options
4 files changed, 85 insertions, 48 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 8b8420571db4..ed47c3e55b74 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4217,7 +4217,9 @@ public class NotificationManagerService extends SystemService { if (wasPosted) { // status bar if (r.getNotification().getSmallIcon() != null) { - r.isCanceled = true; + if (reason != REASON_SNOOZED) { + r.isCanceled = true; + } mListeners.notifyRemovedLocked(r.sbn, reason); mHandler.post(new Runnable() { @Override @@ -4340,9 +4342,11 @@ public class NotificationManagerService extends SystemService { updateLightsLocked(); } else { // No notification was found, assume that it is snoozed and cancel it. - final boolean wasSnoozed = mSnoozeHelper.cancel(userId, pkg, tag, id); - if (wasSnoozed) { - savePolicyFile(); + if (reason != REASON_SNOOZED) { + final boolean wasSnoozed = mSnoozeHelper.cancel(userId, pkg, tag, id); + if (wasSnoozed) { + savePolicyFile(); + } } } } @@ -4472,7 +4476,7 @@ public class NotificationManagerService extends SystemService { void snoozeNotificationInt(String key, long duration, String snoozeCriterionId, ManagedServiceInfo listener) { String listenerName = listener == null ? null : listener.component.toShortString(); - if (duration <= 0 && snoozeCriterionId == null) { + if (duration <= 0 && snoozeCriterionId == null || key == null) { return; } diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 42b4f573dfcc..a178a525cede 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -167,16 +167,10 @@ public class SnoozeHelper { for (Map.Entry<String, NotificationRecord> record : records) { final StatusBarNotification sbn = record.getValue().sbn; if (Objects.equals(sbn.getTag(), tag) && sbn.getId() == id) { - key = record.getKey(); + record.getValue().isCanceled = true; + return true; } } - if (key != null) { - recordsForPkg.remove(key); - cancelAlarm(userId, pkg, key); - mPackages.remove(key); - mUsers.remove(key); - return true; - } } } return false; @@ -190,7 +184,7 @@ public class SnoozeHelper { final int N = userIds.length; for (int i = 0; i < N; i++) { final ArrayMap<String, ArrayMap<String, NotificationRecord>> snoozedPkgs = - mSnoozedNotifications.remove(userIds[i]); + mSnoozedNotifications.get(userIds[i]); if (snoozedPkgs != null) { final int M = snoozedPkgs.size(); for (int j = 0; j < M; j++) { @@ -198,10 +192,7 @@ public class SnoozeHelper { if (records != null) { int P = records.size(); for (int k = 0; k < P; k++) { - final String key = records.keyAt(k); - cancelAlarm(userId, snoozedPkgs.keyAt(j), key); - mPackages.remove(key); - mUsers.remove(key); + records.valueAt(k).isCanceled = true; } } } @@ -215,13 +206,10 @@ public class SnoozeHelper { if (mSnoozedNotifications.containsKey(userId)) { if (mSnoozedNotifications.get(userId).containsKey(pkg)) { ArrayMap<String, NotificationRecord> records = - mSnoozedNotifications.get(userId).remove(pkg); + mSnoozedNotifications.get(userId).get(pkg); int N = records.size(); for (int i = 0; i < N; i++) { - final String key = records.keyAt(i); - cancelAlarm(userId, pkg, key); - mPackages.remove(key); - mUsers.remove(key); + records.valueAt(i).isCanceled = true; } return true; } @@ -229,16 +217,6 @@ public class SnoozeHelper { return false; } - private void cancelAlarm(int userId, String pkg, String key) { - long identity = Binder.clearCallingIdentity(); - try { - final PendingIntent pi = createPendingIntent(pkg, key, userId); - mAm.cancel(pi); - } finally { - Binder.restoreCallingIdentity(identity); - } - } - /** * Updates the notification record so the most up to date information is shown on re-post. */ @@ -252,6 +230,10 @@ public class SnoozeHelper { if (pkgRecords == null) { return; } + NotificationRecord existing = pkgRecords.get(record.getKey()); + if (existing != null && existing.isCanceled) { + return; + } pkgRecords.put(record.getKey(), record); } @@ -274,8 +256,10 @@ public class SnoozeHelper { return; } final NotificationRecord record = pkgRecords.remove(key); + mPackages.remove(key); + mUsers.remove(key); - if (record != null) { + if (record != null && !record.isCanceled) { MetricsLogger.action(record.getLogMaker() .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); @@ -309,10 +293,12 @@ public class SnoozeHelper { mPackages.remove(groupSummaryKey); mUsers.remove(groupSummaryKey); - MetricsLogger.action(record.getLogMaker() - .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) - .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); - mCallback.repost(userId, record); + if (record != null && !record.isCanceled) { + MetricsLogger.action(record.getLogMaker() + .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) + .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); + mCallback.repost(userId, record); + } } } } diff --git a/services/tests/notification/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/notification/src/com/android/server/notification/SnoozeHelperTest.java index 51ec05c51b07..bc25860e8e92 100644 --- a/services/tests/notification/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/notification/src/com/android/server/notification/SnoozeHelperTest.java @@ -107,9 +107,9 @@ public class SnoozeHelperTest { UserHandle.USER_SYSTEM, r2.sbn.getPackageName(), r2.getKey())); mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, r.sbn.getPackageName(), "one", 1); - // 3 = one for each snooze, above + one for cancel itself. - verify(mAm, times(3)).cancel(any(PendingIntent.class)); - assertFalse(mSnoozeHelper.isSnoozed( + // 2 = one for each snooze, above, zero for the cancel. + verify(mAm, times(2)).cancel(any(PendingIntent.class)); + assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r2.sbn.getPackageName(), r2.getKey())); @@ -131,11 +131,11 @@ public class SnoozeHelperTest { UserHandle.USER_ALL, r3.sbn.getPackageName(), r3.getKey())); mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, false); - // 5 = once for each snooze above (3) + once for each notification canceled (2). - verify(mAm, times(5)).cancel(any(PendingIntent.class)); - assertFalse(mSnoozeHelper.isSnoozed( + // 3 = once for each snooze above (3), only. + verify(mAm, times(3)).cancel(any(PendingIntent.class)); + assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); - assertFalse(mSnoozeHelper.isSnoozed( + assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r2.sbn.getPackageName(), r2.getKey())); assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_ALL, r3.sbn.getPackageName(), r3.getKey())); @@ -157,17 +157,47 @@ public class SnoozeHelperTest { UserHandle.USER_SYSTEM, r3.sbn.getPackageName(), r3.getKey())); mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, "pkg2"); - // 4 = once for each snooze above (3) + once for each notification canceled (1). - verify(mAm, times(4)).cancel(any(PendingIntent.class)); + // 3 = once for each snooze above (3), only. + verify(mAm, times(3)).cancel(any(PendingIntent.class)); assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r2.sbn.getPackageName(), r2.getKey())); - assertFalse(mSnoozeHelper.isSnoozed( + assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r3.sbn.getPackageName(), r3.getKey())); } @Test + public void testCancelDoesNotUnsnooze() throws Exception { + NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); + mSnoozeHelper.snooze(r, 1000); + assertTrue(mSnoozeHelper.isSnoozed( + UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); + + mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, r.sbn.getPackageName(), "one", 1); + + assertTrue(mSnoozeHelper.isSnoozed( + UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); + } + + @Test + public void testCancelDoesNotRepost() throws Exception { + NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); + NotificationRecord r2 = getNotificationRecord("pkg", 2, "two", UserHandle.SYSTEM); + mSnoozeHelper.snooze(r, 1000); + mSnoozeHelper.snooze(r2 , 1000); + assertTrue(mSnoozeHelper.isSnoozed( + UserHandle.USER_SYSTEM, r.sbn.getPackageName(), r.getKey())); + assertTrue(mSnoozeHelper.isSnoozed( + UserHandle.USER_SYSTEM, r2.sbn.getPackageName(), r2.getKey())); + + mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, r.sbn.getPackageName(), "one", 1); + + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM); + verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r); + } + + @Test public void testRepost() throws Exception { NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); mSnoozeHelper.snooze(r, 1000); diff --git a/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java b/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java index 93677e35dc4f..5dd42dd21c90 100644 --- a/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java +++ b/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java @@ -385,6 +385,23 @@ public class NotificationTestList extends TestActivity mNM.notify("timeout_min", 7013, n); } }, + new Test("Too many cancels") { + public void run() + { + mNM.cancelAll(); + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + Notification n = new Notification.Builder(NotificationTestList.this, "default") + .setSmallIcon(R.drawable.icon2) + .setContentTitle("Cancel then post") + .setContentText("instead of just updating the existing notification") + .build(); + mNM.notify("cancel_madness", 7014, n); + } + }, new Test("Off") { public void run() { PowerManager pm = (PowerManager) NotificationTestList.this.getSystemService( |