Do not unsnooze canceled notifications.

Test: runtest systemui-notification
Change-Id: I3edef5822aa74359bf3a34f54f59d4021696365d
This commit is contained in:
Julia Reynolds
2017-03-07 16:30:21 -05:00
parent 030107f0bd
commit a8b766fb73
4 changed files with 85 additions and 48 deletions

View File

@@ -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;
}

View File

@@ -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);
}
}
}
}

View File

@@ -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,16 +157,46 @@ 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);

View File

@@ -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(