From 6503162db155501237e9b5e141fcc9a0b0940387 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Thu, 27 Feb 2020 09:11:25 -0500 Subject: [PATCH 1/2] Silence notifications that are manually unsnoozed Test: atest Bug: 149486431 Change-Id: I7d11859210f6000120b2c51dc33da3d0928fb22e --- .../NotificationManagerService.java | 42 ++++++++++++------- .../notification/NotificationRecord.java | 12 ++++++ .../server/notification/SnoozeHelper.java | 14 +++---- .../notification/BuzzBeepBlinkTest.java | 12 ++++++ .../server/notification/SnoozeHelperTest.java | 41 +++++++++++------- 5 files changed, 83 insertions(+), 38 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index ceb1cd41f5676..f4d65a82a104f 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2068,19 +2068,16 @@ public class NotificationManagerService extends SystemService { @Override public void onStart() { - SnoozeHelper snoozeHelper = new SnoozeHelper(getContext(), new SnoozeHelper.Callback() { - @Override - public void repost(int userId, NotificationRecord r) { - try { - if (DBG) { - Slog.d(TAG, "Reposting " + r.getKey()); - } - enqueueNotificationInternal(r.getSbn().getPackageName(), r.getSbn().getOpPkg(), - r.getSbn().getUid(), r.getSbn().getInitialPid(), r.getSbn().getTag(), - r.getSbn().getId(), r.getSbn().getNotification(), userId); - } catch (Exception e) { - Slog.e(TAG, "Cannot un-snooze notification", e); + SnoozeHelper snoozeHelper = new SnoozeHelper(getContext(), (userId, r, muteOnReturn) -> { + try { + if (DBG) { + Slog.d(TAG, "Reposting " + r.getKey()); } + enqueueNotificationInternal(r.getSbn().getPackageName(), r.getSbn().getOpPkg(), + r.getSbn().getUid(), r.getSbn().getInitialPid(), r.getSbn().getTag(), + r.getSbn().getId(), r.getSbn().getNotification(), userId, true); + } catch (Exception e) { + Slog.e(TAG, "Cannot un-snooze notification", e); } }, mUserProfiles); @@ -3983,7 +3980,7 @@ public class NotificationManagerService extends SystemService { synchronized (mNotificationLock) { final ManagedServiceInfo info = mAssistants.checkServiceTokenLocked(token); - unsnoozeNotificationInt(key, info); + unsnoozeNotificationInt(key, info, false); } } finally { Binder.restoreCallingIdentity(identity); @@ -4006,7 +4003,7 @@ public class NotificationManagerService extends SystemService { if (!info.isSystem) { throw new SecurityException("Not allowed to unsnooze before deadline"); } - unsnoozeNotificationInt(key, info); + unsnoozeNotificationInt(key, info, true); } } finally { Binder.restoreCallingIdentity(identity); @@ -5525,6 +5522,13 @@ public class NotificationManagerService extends SystemService { void enqueueNotificationInternal(final String pkg, final String opPkg, final int callingUid, final int callingPid, final String tag, final int id, final Notification notification, int incomingUserId) { + enqueueNotificationInternal(pkg, opPkg, callingUid, callingPid, tag, id, notification, + incomingUserId, false); + } + + void enqueueNotificationInternal(final String pkg, final String opPkg, final int callingUid, + final int callingPid, final String tag, final int id, final Notification notification, + int incomingUserId, boolean postSilently) { if (DBG) { Slog.v(TAG, "enqueueNotificationInternal: pkg=" + pkg + " id=" + id + " notification=" + notification); @@ -5605,6 +5609,7 @@ public class NotificationManagerService extends SystemService { user, null, System.currentTimeMillis()); final NotificationRecord r = new NotificationRecord(getContext(), n, channel); r.setIsAppImportanceLocked(mPreferencesHelper.getIsAppImportanceLocked(pkg, callingUid)); + r.setPostSilently(postSilently); if ((notification.flags & Notification.FLAG_FOREGROUND_SERVICE) != 0) { final boolean fgServiceShown = channel.isFgServiceShown(); @@ -7040,6 +7045,11 @@ public class NotificationManagerService extends SystemService { return true; } + // Suppressed because a user manually unsnoozed something (or similar) + if (record.shouldPostSilently()) { + return true; + } + // muted by listener final String disableEffects = disableNotificationEffects(record); if (disableEffects != null) { @@ -8054,13 +8064,13 @@ public class NotificationManagerService extends SystemService { mHandler.post(new SnoozeNotificationRunnable(key, duration, snoozeCriterionId)); } - void unsnoozeNotificationInt(String key, ManagedServiceInfo listener) { + void unsnoozeNotificationInt(String key, ManagedServiceInfo listener, boolean muteOnReturn) { String listenerName = listener == null ? null : listener.component.toShortString(); if (DBG) { Slog.d(TAG, String.format("unsnooze event(%s, %s)", key, listenerName)); } mSnoozeHelper.cleanupPersistedContext(key); - mSnoozeHelper.repost(key); + mSnoozeHelper.repost(key, muteOnReturn); handleSavePolicyFile(); } diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index 9d243e4d75a97..70415af98ea4a 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -187,6 +187,7 @@ public final class NotificationRecord { private boolean mSuggestionsGeneratedByAssistant; private boolean mEditChoicesBeforeSending; private boolean mHasSeenSmartReplies; + private boolean mPostSilently; /** * Whether this notification (and its channels) should be considered user locked. Used in * conjunction with user sentiment calculation. @@ -856,6 +857,17 @@ public final class NotificationRecord { return mHidden; } + /** + * Override of all alerting information on the channel and notification. Used when notifications + * are reposted in response to direct user action and thus don't need to alert. + */ + public void setPostSilently(boolean postSilently) { + mPostSilently = postSilently; + } + + public boolean shouldPostSilently() { + return mPostSilently; + } public void setSuppressedVisualEffects(int effects) { mSuppressedVisualEffects = effects; diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index d60c29119c182..26dbd22ce0287 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -332,14 +332,14 @@ public class SnoozeHelper { records.put(record.getKey(), record); } - protected void repost(String key) { + protected void repost(String key, boolean muteOnReturn) { Integer userId = mUsers.get(key); if (userId != null) { - repost(key, userId); + repost(key, userId, muteOnReturn); } } - protected void repost(String key, int userId) { + protected void repost(String key, int userId, boolean muteOnReturn) { final String pkg = mPackages.remove(key); ArrayMap records = mSnoozedNotifications.get(getPkgKey(userId, pkg)); @@ -356,7 +356,7 @@ public class SnoozeHelper { MetricsLogger.action(record.getLogMaker() .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); - mCallback.repost(userId, record); + mCallback.repost(userId, record, muteOnReturn); } } @@ -388,7 +388,7 @@ public class SnoozeHelper { MetricsLogger.action(record.getLogMaker() .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); - mCallback.repost(userId, record); + mCallback.repost(userId, record, false); } } } @@ -601,7 +601,7 @@ public class SnoozeHelper { } protected interface Callback { - void repost(int userId, NotificationRecord r); + void repost(int userId, NotificationRecord r, boolean muteOnReturn); } private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() { @@ -612,7 +612,7 @@ public class SnoozeHelper { } if (REPOST_ACTION.equals(intent.getAction())) { repost(intent.getStringExtra(EXTRA_KEY), intent.getIntExtra(EXTRA_USER_ID, - UserHandle.USER_SYSTEM)); + UserHandle.USER_SYSTEM), false); } } }; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index f029bc80480a6..afd10ddb8ec26 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -845,6 +845,18 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { assertNotEquals(-1, r.getLastAudiblyAlertedMs()); } + @Test + public void testPostSilently() throws Exception { + NotificationRecord r = getBuzzyNotification(); + r.setPostSilently(true); + + mService.buzzBeepBlinkLocked(r); + + verifyNeverBeep(); + assertFalse(r.isInterruptive()); + assertEquals(-1, r.getLastAudiblyAlertedMs()); + } + @Test public void testGroupAlertSummarySilenceChild() throws Exception { NotificationRecord child = getBeepyNotificationRecord("a", GROUP_ALERT_SUMMARY); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 816e8e57acc6f..863bbbb0d8dbf 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -22,6 +22,7 @@ import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; @@ -363,8 +364,8 @@ public class SnoozeHelperTest extends UiServiceTestCase { mSnoozeHelper.cancel(UserHandle.USER_SYSTEM, r.getSbn().getPackageName(), "one", 1); - mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM); - verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r); + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM, false); + verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r, false); } @Test @@ -374,8 +375,8 @@ public class SnoozeHelperTest extends UiServiceTestCase { NotificationRecord r2 = getNotificationRecord("pkg", 2, "one", UserHandle.ALL); mSnoozeHelper.snooze(r2, 1000); reset(mAm); - mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM); - verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r); + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM, false); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); ArgumentCaptor captor = ArgumentCaptor.forClass(PendingIntent.class); verify(mAm).cancel(captor.capture()); assertEquals(r.getKey(), captor.getValue().getIntent().getStringExtra(EXTRA_KEY)); @@ -388,8 +389,8 @@ public class SnoozeHelperTest extends UiServiceTestCase { NotificationRecord r2 = getNotificationRecord("pkg", 2, "one", UserHandle.ALL); mSnoozeHelper.snooze(r2, 1000); reset(mAm); - mSnoozeHelper.repost(r.getKey()); - verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r); + mSnoozeHelper.repost(r.getKey(), false); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); verify(mAm).cancel(any(PendingIntent.class)); } @@ -400,10 +401,10 @@ public class SnoozeHelperTest extends UiServiceTestCase { r.getNotification().category = "NEW CATEGORY"; mSnoozeHelper.update(UserHandle.USER_SYSTEM, r); - verify(mCallback, never()).repost(anyInt(), any(NotificationRecord.class)); + verify(mCallback, never()).repost(anyInt(), any(NotificationRecord.class), anyBoolean()); - mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM); - verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r); + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM, false); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); } @Test @@ -420,12 +421,22 @@ public class SnoozeHelperTest extends UiServiceTestCase { mSnoozeHelper.update(UserHandle.USER_SYSTEM, r); // verify callback is called when repost (snooze is expired) - verify(mCallback, never()).repost(anyInt(), any(NotificationRecord.class)); - mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM); - verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r); + verify(mCallback, never()).repost(anyInt(), any(NotificationRecord.class), anyBoolean()); + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM, false); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); assertFalse(r.isCanceled); } + @Test + public void testReport_passesFlag() throws Exception { + // snooze a notification + NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); + mSnoozeHelper.snooze(r , 1000); + + mSnoozeHelper.repost(r.getKey(), UserHandle.USER_SYSTEM, true); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, true); + } + @Test public void testGetSnoozedBy() throws Exception { NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); @@ -523,7 +534,7 @@ public class SnoozeHelperTest extends UiServiceTestCase { mSnoozeHelper.snooze(r2, 1000); mSnoozeHelper.repostGroupSummary("pkg", UserHandle.USER_SYSTEM, "group1"); - verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r); + verify(mCallback, never()).repost(eq(UserHandle.USER_SYSTEM), eq(r), anyBoolean()); } @Test @@ -542,8 +553,8 @@ public class SnoozeHelperTest extends UiServiceTestCase { mSnoozeHelper.repostGroupSummary("pkg", UserHandle.USER_SYSTEM, r.getGroupKey()); - verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r); - verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r2); + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); + verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r2, false); assertEquals(1, mSnoozeHelper.getSnoozed().size()); assertEquals(1, mSnoozeHelper.getSnoozed(UserHandle.USER_SYSTEM, "pkg").size()); From 6cab5ff811cfecc8570ca98fb2276f48f07af406 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Thu, 27 Feb 2020 14:56:58 -0500 Subject: [PATCH 2/2] Fix some more snoozing bugs - Make all time based snoozing use the same time frame (current millis, not elapsed walltime) - Add synchronization to snoozing - Update when entries are and are not removed from data structures to avoid NPEs Test: atest, and manual, repeated snooze and unsnooze Fixes: 150377159 Change-Id: Ic91ff7ac0bd1b3635b35267bdbc64d3ee614c8ab --- .../NotificationManagerService.java | 1 - .../server/notification/SnoozeHelper.java | 534 ++++++++++-------- .../server/notification/SnoozeHelperTest.java | 2 +- 3 files changed, 298 insertions(+), 239 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index f4d65a82a104f..77d2fc2a0c3c6 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -8069,7 +8069,6 @@ public class NotificationManagerService extends SystemService { if (DBG) { Slog.d(TAG, String.format("unsnooze event(%s, %s)", key, listenerName)); } - mSnoozeHelper.cleanupPersistedContext(key); mSnoozeHelper.repost(key, muteOnReturn); handleSavePolicyFile(); } diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 26dbd22ce0287..9a9e733cb3900 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -106,6 +106,8 @@ public class SnoozeHelper { private ArrayMap mUsers = new ArrayMap<>(); private Callback mCallback; + private final Object mLock = new Object(); + public SnoozeHelper(Context context, Callback callback, ManagedServices.UserProfiles userProfiles) { mContext = context; @@ -122,41 +124,52 @@ public class SnoozeHelper { } void cleanupPersistedContext(String key){ - int userId = mUsers.get(key); - String pkg = mPackages.get(key); - synchronized (mPersistedSnoozedNotificationsWithContext) { - removeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext); + synchronized (mLock) { + int userId = mUsers.get(key); + String pkg = mPackages.get(key); + removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotificationsWithContext); } } - //This function has a side effect of removing the time from the list of persisted notifications. - //IT IS NOT IDEMPOTENT! @NonNull protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) { - Long time; - synchronized (mPersistedSnoozedNotifications) { - time = removeRecord(pkg, key, userId, mPersistedSnoozedNotifications); + Long time = null; + synchronized (mLock) { + ArrayMap snoozed = + mPersistedSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (snoozed != null) { + time = snoozed.get(key); + } } if (time == null) { - return 0L; + time = 0L; } return time; } protected String getSnoozeContextForUnpostedNotification(int userId, String pkg, String key) { - synchronized (mPersistedSnoozedNotificationsWithContext) { - return removeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext); + synchronized (mLock) { + ArrayMap snoozed = + mPersistedSnoozedNotificationsWithContext.get(getPkgKey(userId, pkg)); + if (snoozed != null) { + return snoozed.get(key); + } } + return null; } protected boolean isSnoozed(int userId, String pkg, String key) { - return mSnoozedNotifications.containsKey(getPkgKey(userId, pkg)) - && mSnoozedNotifications.get(getPkgKey(userId, pkg)).containsKey(key); + synchronized (mLock) { + return mSnoozedNotifications.containsKey(getPkgKey(userId, pkg)) + && mSnoozedNotifications.get(getPkgKey(userId, pkg)).containsKey(key); + } } protected Collection getSnoozed(int userId, String pkg) { - if (mSnoozedNotifications.containsKey(getPkgKey(userId, pkg))) { - return mSnoozedNotifications.get(getPkgKey(userId, pkg)).values(); + synchronized (mLock) { + if (mSnoozedNotifications.containsKey(getPkgKey(userId, pkg))) { + return mSnoozedNotifications.get(getPkgKey(userId, pkg)).values(); + } } return Collections.EMPTY_LIST; } @@ -165,14 +178,16 @@ public class SnoozeHelper { ArrayList getNotifications(String pkg, String groupKey, Integer userId) { ArrayList records = new ArrayList<>(); - ArrayMap allRecords = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (allRecords != null) { - for (int i = 0; i < allRecords.size(); i++) { - NotificationRecord r = allRecords.valueAt(i); - String currentGroupKey = r.getSbn().getGroup(); - if (Objects.equals(currentGroupKey, groupKey)) { - records.add(r); + synchronized (mLock) { + ArrayMap allRecords = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (allRecords != null) { + for (int i = 0; i < allRecords.size(); i++) { + NotificationRecord r = allRecords.valueAt(i); + String currentGroupKey = r.getSbn().getGroup(); + if (Objects.equals(currentGroupKey, groupKey)) { + records.add(r); + } } } } @@ -180,30 +195,34 @@ public class SnoozeHelper { } protected NotificationRecord getNotification(String key) { - if (!mUsers.containsKey(key) || !mPackages.containsKey(key)) { - Slog.w(TAG, "Snoozed data sets no longer agree for " + key); - return null; + synchronized (mLock) { + if (!mUsers.containsKey(key) || !mPackages.containsKey(key)) { + Slog.w(TAG, "Snoozed data sets no longer agree for " + key); + return null; + } + int userId = mUsers.get(key); + String pkg = mPackages.get(key); + ArrayMap snoozed = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (snoozed == null) { + return null; + } + return snoozed.get(key); } - int userId = mUsers.get(key); - String pkg = mPackages.get(key); - ArrayMap snoozed = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (snoozed == null) { - return null; - } - return snoozed.get(key); } protected @NonNull List getSnoozed() { - // caller filters records based on the current user profiles and listener access, so just - // return everything - List snoozed= new ArrayList<>(); - for (String userPkgKey : mSnoozedNotifications.keySet()) { - ArrayMap snoozedRecords = - mSnoozedNotifications.get(userPkgKey); - snoozed.addAll(snoozedRecords.values()); + synchronized (mLock) { + // caller filters records based on the current user profiles and listener access, so just + // return everything + List snoozed = new ArrayList<>(); + for (String userPkgKey : mSnoozedNotifications.keySet()) { + ArrayMap snoozedRecords = + mSnoozedNotifications.get(userPkgKey); + snoozed.addAll(snoozedRecords.values()); + } + return snoozed; } - return snoozed; } /** @@ -216,9 +235,9 @@ public class SnoozeHelper { snooze(record); scheduleRepost(pkg, key, userId, duration); - Long activateAt = SystemClock.elapsedRealtime() + duration; - synchronized (mPersistedSnoozedNotifications) { - storeRecord(pkg, key, userId, mPersistedSnoozedNotifications, activateAt); + Long activateAt = System.currentTimeMillis() + duration; + synchronized (mLock) { + storeRecordLocked(pkg, key, userId, mPersistedSnoozedNotifications, activateAt); } } @@ -228,8 +247,8 @@ public class SnoozeHelper { protected void snooze(NotificationRecord record, String contextId) { int userId = record.getUser().getIdentifier(); if (contextId != null) { - synchronized (mPersistedSnoozedNotificationsWithContext) { - storeRecord(record.getSbn().getPackageName(), record.getKey(), + synchronized (mLock) { + storeRecordLocked(record.getSbn().getPackageName(), record.getKey(), userId, mPersistedSnoozedNotificationsWithContext, contextId); } } @@ -241,25 +260,26 @@ public class SnoozeHelper { if (DEBUG) { Slog.d(TAG, "Snoozing " + record.getKey()); } - storeRecord(record.getSbn().getPackageName(), record.getKey(), - userId, mSnoozedNotifications, record); + synchronized (mLock) { + storeRecordLocked(record.getSbn().getPackageName(), record.getKey(), + userId, mSnoozedNotifications, record); + } } - private void storeRecord(String pkg, String key, Integer userId, + private void storeRecordLocked(String pkg, String key, Integer userId, ArrayMap> targets, T object) { + mPackages.put(key, pkg); + mUsers.put(key, userId); ArrayMap keyToValue = targets.get(getPkgKey(userId, pkg)); if (keyToValue == null) { keyToValue = new ArrayMap<>(); } keyToValue.put(key, object); targets.put(getPkgKey(userId, pkg), keyToValue); - - mPackages.put(key, pkg); - mUsers.put(key, userId); } - private T removeRecord(String pkg, String key, Integer userId, + private T removeRecordLocked(String pkg, String key, Integer userId, ArrayMap> targets) { T object = null; ArrayMap keyToValue = targets.get(getPkgKey(userId, pkg)); @@ -274,15 +294,17 @@ public class SnoozeHelper { } protected boolean cancel(int userId, String pkg, String tag, int id) { - ArrayMap recordsForPkg = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (recordsForPkg != null) { - final Set> records = recordsForPkg.entrySet(); - for (Map.Entry record : records) { - final StatusBarNotification sbn = record.getValue().getSbn(); - if (Objects.equals(sbn.getTag(), tag) && sbn.getId() == id) { - record.getValue().isCanceled = true; - return true; + synchronized (mLock) { + ArrayMap recordsForPkg = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (recordsForPkg != null) { + final Set> records = recordsForPkg.entrySet(); + for (Map.Entry record : records) { + final StatusBarNotification sbn = record.getValue().getSbn(); + if (Objects.equals(sbn.getTag(), tag) && sbn.getId() == id) { + record.getValue().isCanceled = true; + return true; + } } } } @@ -290,68 +312,82 @@ public class SnoozeHelper { } protected void cancel(int userId, boolean includeCurrentProfiles) { - if (mSnoozedNotifications.size() == 0) { - return; - } - IntArray userIds = new IntArray(); - userIds.add(userId); - if (includeCurrentProfiles) { - userIds = mUserProfiles.getCurrentProfileIds(); - } - for (ArrayMap snoozedRecords : mSnoozedNotifications.values()) { - for (NotificationRecord r : snoozedRecords.values()) { - if (userIds.binarySearch(r.getUserId()) >= 0) { - r.isCanceled = true; + synchronized (mLock) { + if (mSnoozedNotifications.size() == 0) { + return; + } + IntArray userIds = new IntArray(); + userIds.add(userId); + if (includeCurrentProfiles) { + userIds = mUserProfiles.getCurrentProfileIds(); + } + for (ArrayMap snoozedRecords : mSnoozedNotifications.values()) { + for (NotificationRecord r : snoozedRecords.values()) { + if (userIds.binarySearch(r.getUserId()) >= 0) { + r.isCanceled = true; + } } } } } protected boolean cancel(int userId, String pkg) { - ArrayMap records = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (records == null) { - return false; + synchronized (mLock) { + ArrayMap records = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (records == null) { + return false; + } + int N = records.size(); + for (int i = 0; i < N; i++) { + records.valueAt(i).isCanceled = true; + } + return true; } - int N = records.size(); - for (int i = 0; i < N; i++) { - records.valueAt(i).isCanceled = true; - } - return true; } /** * Updates the notification record so the most up to date information is shown on re-post. */ protected void update(int userId, NotificationRecord record) { - ArrayMap records = - mSnoozedNotifications.get(getPkgKey(userId, record.getSbn().getPackageName())); - if (records == null) { - return; + synchronized (mLock) { + ArrayMap records = + mSnoozedNotifications.get(getPkgKey(userId, record.getSbn().getPackageName())); + if (records == null) { + return; + } + records.put(record.getKey(), record); } - records.put(record.getKey(), record); } protected void repost(String key, boolean muteOnReturn) { - Integer userId = mUsers.get(key); - if (userId != null) { - repost(key, userId, muteOnReturn); + synchronized (mLock) { + Integer userId = mUsers.get(key); + if (userId != null) { + repost(key, userId, muteOnReturn); + } } } protected void repost(String key, int userId, boolean muteOnReturn) { - final String pkg = mPackages.remove(key); - ArrayMap records = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (records == null) { - return; + NotificationRecord record; + synchronized (mLock) { + final String pkg = mPackages.remove(key); + mUsers.remove(key); + removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotifications); + removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotificationsWithContext); + ArrayMap records = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (records == null) { + return; + } + record = records.remove(key); + } - final NotificationRecord record = records.remove(key); - mPackages.remove(key); - mUsers.remove(key); if (record != null && !record.isCanceled) { - final PendingIntent pi = createPendingIntent(pkg, record.getKey(), userId); + final PendingIntent pi = createPendingIntent( + record.getSbn().getPackageName(), record.getKey(), userId); mAm.cancel(pi); MetricsLogger.action(record.getLogMaker() .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) @@ -361,54 +397,64 @@ public class SnoozeHelper { } protected void repostGroupSummary(String pkg, int userId, String groupKey) { - ArrayMap recordsByKey - = mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (recordsByKey == null) { - return; - } - - String groupSummaryKey = null; - int N = recordsByKey.size(); - for (int i = 0; i < N; i++) { - final NotificationRecord potentialGroupSummary = recordsByKey.valueAt(i); - if (potentialGroupSummary.getSbn().isGroup() - && potentialGroupSummary.getNotification().isGroupSummary() - && groupKey.equals(potentialGroupSummary.getGroupKey())) { - groupSummaryKey = potentialGroupSummary.getKey(); - break; + synchronized (mLock) { + ArrayMap recordsByKey + = mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (recordsByKey == null) { + return; } - } - if (groupSummaryKey != null) { - NotificationRecord record = recordsByKey.remove(groupSummaryKey); - mPackages.remove(groupSummaryKey); - mUsers.remove(groupSummaryKey); + String groupSummaryKey = null; + int N = recordsByKey.size(); + for (int i = 0; i < N; i++) { + final NotificationRecord potentialGroupSummary = recordsByKey.valueAt(i); + if (potentialGroupSummary.getSbn().isGroup() + && potentialGroupSummary.getNotification().isGroupSummary() + && groupKey.equals(potentialGroupSummary.getGroupKey())) { + groupSummaryKey = potentialGroupSummary.getKey(); + break; + } + } - if (record != null && !record.isCanceled) { - MetricsLogger.action(record.getLogMaker() - .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) - .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); - mCallback.repost(userId, record, false); + if (groupSummaryKey != null) { + NotificationRecord record = recordsByKey.remove(groupSummaryKey); + mPackages.remove(groupSummaryKey); + mUsers.remove(groupSummaryKey); + + if (record != null && !record.isCanceled) { + Runnable runnable = () -> { + MetricsLogger.action(record.getLogMaker() + .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) + .setType(MetricsProto.MetricsEvent.TYPE_OPEN)); + mCallback.repost(userId, record, false); + }; + runnable.run(); + } } } } protected void clearData(int userId, String pkg) { - ArrayMap records = - mSnoozedNotifications.get(getPkgKey(userId, pkg)); - if (records == null) { - return; - } - for (int i = records.size() - 1; i >= 0; i--) { - final NotificationRecord r = records.removeAt(i); - if (r != null) { - mPackages.remove(r.getKey()); - mUsers.remove(r.getKey()); - final PendingIntent pi = createPendingIntent(pkg, r.getKey(), userId); - mAm.cancel(pi); - MetricsLogger.action(r.getLogMaker() - .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) - .setType(MetricsProto.MetricsEvent.TYPE_DISMISS)); + synchronized (mLock) { + ArrayMap records = + mSnoozedNotifications.get(getPkgKey(userId, pkg)); + if (records == null) { + return; + } + for (int i = records.size() - 1; i >= 0; i--) { + final NotificationRecord r = records.removeAt(i); + if (r != null) { + mPackages.remove(r.getKey()); + mUsers.remove(r.getKey()); + Runnable runnable = () -> { + final PendingIntent pi = createPendingIntent(pkg, r.getKey(), userId); + mAm.cancel(pi); + MetricsLogger.action(r.getLogMaker() + .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED) + .setType(MetricsProto.MetricsEvent.TYPE_DISMISS)); + }; + runnable.run(); + } } } } @@ -425,93 +471,102 @@ public class SnoozeHelper { } public void scheduleRepostsForPersistedNotifications(long currentTime) { - for (ArrayMap snoozed : mPersistedSnoozedNotifications.values()) { - for (int i = 0; i < snoozed.size(); i++) { - String key = snoozed.keyAt(i); - Long time = snoozed.valueAt(i); - String pkg = mPackages.get(key); - Integer userId = mUsers.get(key); - if (time == null || pkg == null || userId == null) { - Slog.w(TAG, "data out of sync: " + time + "|" + pkg + "|" + userId); - continue; - } - if (time != null && time > currentTime) { - scheduleRepostAtTime(pkg, key, userId, time); + synchronized (mLock) { + for (ArrayMap snoozed : mPersistedSnoozedNotifications.values()) { + for (int i = 0; i < snoozed.size(); i++) { + String key = snoozed.keyAt(i); + Long time = snoozed.valueAt(i); + String pkg = mPackages.get(key); + Integer userId = mUsers.get(key); + if (time == null || pkg == null || userId == null) { + Slog.w(TAG, "data out of sync: " + time + "|" + pkg + "|" + userId); + continue; + } + if (time != null && time > currentTime) { + scheduleRepostAtTime(pkg, key, userId, time); + } } } - } } private void scheduleRepost(String pkg, String key, int userId, long duration) { - scheduleRepostAtTime(pkg, key, userId, SystemClock.elapsedRealtime() + duration); + scheduleRepostAtTime(pkg, key, userId, System.currentTimeMillis() + duration); } private void scheduleRepostAtTime(String pkg, String key, int userId, long time) { - long identity = Binder.clearCallingIdentity(); - try { - final PendingIntent pi = createPendingIntent(pkg, key, userId); - mAm.cancel(pi); - if (DEBUG) Slog.d(TAG, "Scheduling evaluate for " + new Date(time)); - mAm.setExactAndAllowWhileIdle(AlarmManager.ELAPSED_REALTIME_WAKEUP, time, pi); - } finally { - Binder.restoreCallingIdentity(identity); - } + Runnable runnable = () -> { + long identity = Binder.clearCallingIdentity(); + try { + final PendingIntent pi = createPendingIntent(pkg, key, userId); + mAm.cancel(pi); + if (DEBUG) Slog.d(TAG, "Scheduling evaluate for " + new Date(time)); + mAm.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, time, pi); + } finally { + Binder.restoreCallingIdentity(identity); + } + }; + runnable.run(); } public void dump(PrintWriter pw, NotificationManagerService.DumpFilter filter) { - pw.println("\n Snoozed notifications:"); - for (String userPkgKey : mSnoozedNotifications.keySet()) { - pw.print(INDENT); - pw.println("key: " + userPkgKey); - ArrayMap snoozedRecords = - mSnoozedNotifications.get(userPkgKey); - Set snoozedKeys = snoozedRecords.keySet(); - for (String key : snoozedKeys) { + synchronized (mLock) { + pw.println("\n Snoozed notifications:"); + for (String userPkgKey : mSnoozedNotifications.keySet()) { pw.print(INDENT); - pw.print(INDENT); - pw.print(INDENT); - pw.println(key); + pw.println("key: " + userPkgKey); + ArrayMap snoozedRecords = + mSnoozedNotifications.get(userPkgKey); + Set snoozedKeys = snoozedRecords.keySet(); + for (String key : snoozedKeys) { + pw.print(INDENT); + pw.print(INDENT); + pw.print(INDENT); + pw.println(key); + } } - } - pw.println("\n Pending snoozed notifications"); - for (String userPkgKey : mPersistedSnoozedNotifications.keySet()) { - pw.print(INDENT); - pw.println("key: " + userPkgKey); - ArrayMap snoozedRecords = - mPersistedSnoozedNotifications.get(userPkgKey); - if (snoozedRecords == null) { - continue; - } - Set snoozedKeys = snoozedRecords.keySet(); - for (String key : snoozedKeys) { + pw.println("\n Pending snoozed notifications"); + for (String userPkgKey : mPersistedSnoozedNotifications.keySet()) { pw.print(INDENT); - pw.print(INDENT); - pw.print(INDENT); - pw.print(key); - pw.print(INDENT); - pw.println(snoozedRecords.get(key)); + pw.println("key: " + userPkgKey); + ArrayMap snoozedRecords = + mPersistedSnoozedNotifications.get(userPkgKey); + if (snoozedRecords == null) { + continue; + } + Set snoozedKeys = snoozedRecords.keySet(); + for (String key : snoozedKeys) { + pw.print(INDENT); + pw.print(INDENT); + pw.print(INDENT); + pw.print(key); + pw.print(INDENT); + pw.println(snoozedRecords.get(key)); + } } } } protected void writeXml(XmlSerializer out) throws IOException { - final long currentTime = System.currentTimeMillis(); - out.startTag(null, XML_TAG_NAME); - writeXml(out, mPersistedSnoozedNotifications, XML_SNOOZED_NOTIFICATION, - value -> { - if (value < currentTime) { - return; - } - out.attribute(null, XML_SNOOZED_NOTIFICATION_TIME, - value.toString()); - }); - writeXml(out, mPersistedSnoozedNotificationsWithContext, XML_SNOOZED_NOTIFICATION_CONTEXT, - value -> { - out.attribute(null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID, - value); - }); - out.endTag(null, XML_TAG_NAME); + synchronized (mLock) { + final long currentTime = System.currentTimeMillis(); + out.startTag(null, XML_TAG_NAME); + writeXml(out, mPersistedSnoozedNotifications, XML_SNOOZED_NOTIFICATION, + value -> { + if (value < currentTime) { + return; + } + out.attribute(null, XML_SNOOZED_NOTIFICATION_TIME, + value.toString()); + }); + writeXml(out, mPersistedSnoozedNotificationsWithContext, + XML_SNOOZED_NOTIFICATION_CONTEXT, + value -> { + out.attribute(null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID, + value); + }); + out.endTag(null, XML_TAG_NAME); + } } private interface Inserter { @@ -522,32 +577,35 @@ public class SnoozeHelper { ArrayMap> targets, String tag, Inserter attributeInserter) throws IOException { - synchronized (targets) { - final int M = targets.size(); - for (int i = 0; i < M; i++) { - // T is a String (snoozed until context) or Long (snoozed until time) - ArrayMap keyToValue = targets.valueAt(i); - for (int j = 0; j < keyToValue.size(); j++) { - String key = keyToValue.keyAt(j); - T value = keyToValue.valueAt(j); - String pkg = mPackages.get(key); - Integer userId = mUsers.get(key); + final int M = targets.size(); + for (int i = 0; i < M; i++) { + // T is a String (snoozed until context) or Long (snoozed until time) + ArrayMap keyToValue = targets.valueAt(i); + for (int j = 0; j < keyToValue.size(); j++) { + String key = keyToValue.keyAt(j); + T value = keyToValue.valueAt(j); + String pkg = mPackages.get(key); + Integer userId = mUsers.get(key); - out.startTag(null, tag); - - attributeInserter.insert(value); - - out.attribute(null, XML_SNOOZED_NOTIFICATION_VERSION_LABEL, - XML_SNOOZED_NOTIFICATION_VERSION); - out.attribute(null, XML_SNOOZED_NOTIFICATION_KEY, key); - - - out.attribute(null, XML_SNOOZED_NOTIFICATION_PKG, pkg); - out.attribute(null, XML_SNOOZED_NOTIFICATION_USER_ID, - String.valueOf(userId)); - - out.endTag(null, tag); + if (pkg == null || userId == null) { + Slog.w(TAG, "pkg " + pkg + " or user " + userId + " missing for " + key); + continue; } + + out.startTag(null, tag); + + attributeInserter.insert(value); + + out.attribute(null, XML_SNOOZED_NOTIFICATION_VERSION_LABEL, + XML_SNOOZED_NOTIFICATION_VERSION); + out.attribute(null, XML_SNOOZED_NOTIFICATION_KEY, key); + + + out.attribute(null, XML_SNOOZED_NOTIFICATION_PKG, pkg); + out.attribute(null, XML_SNOOZED_NOTIFICATION_USER_ID, + String.valueOf(userId)); + + out.endTag(null, tag); } } } @@ -575,16 +633,18 @@ public class SnoozeHelper { final Long time = XmlUtils.readLongAttribute( parser, XML_SNOOZED_NOTIFICATION_TIME, 0); if (time > currentTime) { //only read new stuff - synchronized (mPersistedSnoozedNotifications) { - storeRecord(pkg, key, userId, mPersistedSnoozedNotifications, time); + synchronized (mLock) { + storeRecordLocked( + pkg, key, userId, mPersistedSnoozedNotifications, time); } } } if (tag.equals(XML_SNOOZED_NOTIFICATION_CONTEXT)) { final String creationId = parser.getAttributeValue( null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID); - synchronized (mPersistedSnoozedNotificationsWithContext) { - storeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext, + synchronized (mLock) { + storeRecordLocked( + pkg, key, userId, mPersistedSnoozedNotificationsWithContext, creationId); } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 863bbbb0d8dbf..3deeea2d45772 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -250,7 +250,7 @@ public class SnoozeHelperTest extends UiServiceTestCase { ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); verify(mAm, times(1)).setExactAndAllowWhileIdle( anyInt(), captor.capture(), any(PendingIntent.class)); - long actualSnoozedUntilDuration = captor.getValue() - SystemClock.elapsedRealtime(); + long actualSnoozedUntilDuration = captor.getValue() - System.currentTimeMillis(); assertTrue(Math.abs(actualSnoozedUntilDuration - 1000) < 250); assertTrue(mSnoozeHelper.isSnoozed( UserHandle.USER_SYSTEM, r.getSbn().getPackageName(), r.getKey()));