Merge "Do not unsnooze canceled notifications." into oc-dev am: 8183328af7
am: e53ae6aa23
Change-Id: Ie75b1d98124935c7575665dd85f1efa81e86a8f3
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 8b84205..ed47c3e 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 @@
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 @@
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 @@
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 42b4f57..a178a52 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 51ec05c..bc25860 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 @@
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 @@
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 @@
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 93677e3..5dd42dd 100644
--- a/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java
+++ b/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java
@@ -385,6 +385,23 @@
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(