Check disqualifying notifications synchronously.
Don't wait to check on the handler.
Test: runtest systemui-notification and
platform_testing/tests/functional/notificationtests
Change-Id: Ife1ad8bc5c40420cd6682329b8135744cfe38e4a
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 18ac76a..b543b73 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -2732,10 +2732,8 @@
summaries.put(pkg, summarySbn.getKey());
}
}
- if (summaryRecord != null) {
- synchronized (mNotificationLock) {
- mEnqueuedNotifications.add(summaryRecord);
- }
+ if (summaryRecord != null && checkDisqualifyingFeatures(userId, MY_UID,
+ summaryRecord.sbn.getId(), summaryRecord.sbn.getTag(), summaryRecord)) {
mHandler.post(new EnqueueNotificationRunnable(userId, summaryRecord));
}
}
@@ -2966,13 +2964,15 @@
+ " notification=" + notification);
}
checkCallerIsSystemOrSameApp(pkg);
- final boolean isSystemNotification = isUidSystem(callingUid) || ("android".equals(pkg));
- final boolean isNotificationFromListener = mListeners.isListenerPackage(pkg);
final int userId = ActivityManager.handleIncomingUser(callingPid,
callingUid, incomingUserId, true, false, "enqueueNotification", pkg);
final UserHandle user = new UserHandle(userId);
+ if (pkg == null || notification == null) {
+ throw new IllegalArgumentException("null not allowed: pkg=" + pkg
+ + " id=" + id + " notification=" + notification);
+ }
// Fix the notification as best we can.
try {
final ApplicationInfo ai = mPackageManagerClient.getApplicationInfoAsUser(
@@ -2986,11 +2986,7 @@
mUsageStats.registerEnqueuedByApp(pkg);
-
- if (pkg == null || notification == null) {
- throw new IllegalArgumentException("null not allowed: pkg=" + pkg
- + " id=" + id + " notification=" + notification);
- }
+ // setup local book-keeping
String channelId = notification.getChannel();
if (mIsTelevision && (new Notification.TvExtender(notification)).getChannel() != null) {
channelId = (new Notification.TvExtender(notification)).getChannel();
@@ -3000,44 +2996,10 @@
final StatusBarNotification n = new StatusBarNotification(
pkg, opPkg, id, tag, callingUid, callingPid, notification,
user, null, System.currentTimeMillis());
+ final NotificationRecord r = new NotificationRecord(getContext(), n, channel);
- // Limit the number of notifications that any given package except the android
- // package or a registered listener can enqueue. Prevents DOS attacks and deals with leaks.
- if (!isSystemNotification && !isNotificationFromListener) {
- synchronized (mNotificationLock) {
- if (mNotificationsByKey.get(n.getKey()) != null) {
- // this is an update, rate limit updates only
- final float appEnqueueRate = mUsageStats.getAppEnqueueRate(pkg);
- if (appEnqueueRate > mMaxPackageEnqueueRate) {
- mUsageStats.registerOverRateQuota(pkg);
- final long now = SystemClock.elapsedRealtime();
- if ((now - mLastOverRateLogTime) > MIN_PACKAGE_OVERRATE_LOG_INTERVAL) {
- Slog.e(TAG, "Package enqueue rate is " + appEnqueueRate
- + ". Shedding events. package=" + pkg);
- mLastOverRateLogTime = now;
- }
- return;
- }
- }
-
- int count = 0;
- final int N = mNotificationList.size();
- for (int i=0; i<N; i++) {
- final NotificationRecord r = mNotificationList.get(i);
- if (r.sbn.getPackageName().equals(pkg) && r.sbn.getUserId() == userId) {
- if (r.sbn.getId() == id && TextUtils.equals(r.sbn.getTag(), tag)) {
- break; // Allow updating existing notification
- }
- count++;
- if (count >= MAX_PACKAGE_NOTIFICATIONS) {
- mUsageStats.registerOverCountQuota(pkg);
- Slog.e(TAG, "Package has already posted " + count
- + " notifications. Not showing more. package=" + pkg);
- return;
- }
- }
- }
- }
+ if (!checkDisqualifyingFeatures(userId, callingUid, id,tag, r)) {
+ return;
}
// Whitelist pending intents.
@@ -3057,13 +3019,105 @@
}
}
- // setup local book-keeping
- final NotificationRecord r = new NotificationRecord(getContext(), n, channel);
mHandler.post(new EnqueueNotificationRunnable(userId, r));
idOut[0] = id;
}
+ /**
+ * Checks if a notification can be posted. checks rate limiter, snooze helper, and blocking.
+ *
+ * Has side effects.
+ */
+ private boolean checkDisqualifyingFeatures(int userId, int callingUid, int id, String tag,
+ NotificationRecord r) {
+ final String pkg = r.sbn.getPackageName();
+ final boolean isSystemNotification = isUidSystem(callingUid) || ("android".equals(pkg));
+ final boolean isNotificationFromListener = mListeners.isListenerPackage(pkg);
+
+ // Limit the number of notifications that any given package except the android
+ // package or a registered listener can enqueue. Prevents DOS attacks and deals with leaks.
+ if (!isSystemNotification && !isNotificationFromListener) {
+ synchronized (mNotificationLock) {
+ if (mNotificationsByKey.get(r.sbn.getKey()) != null) {
+ // this is an update, rate limit updates only
+ final float appEnqueueRate = mUsageStats.getAppEnqueueRate(pkg);
+ if (appEnqueueRate > mMaxPackageEnqueueRate) {
+ mUsageStats.registerOverRateQuota(pkg);
+ final long now = SystemClock.elapsedRealtime();
+ if ((now - mLastOverRateLogTime) > MIN_PACKAGE_OVERRATE_LOG_INTERVAL) {
+ Slog.e(TAG, "Package enqueue rate is " + appEnqueueRate
+ + ". Shedding events. package=" + pkg);
+ mLastOverRateLogTime = now;
+ }
+ return false;
+ }
+ }
+
+ int count = 0;
+ final int N = mNotificationList.size();
+ for (int i=0; i<N; i++) {
+ final NotificationRecord existing = mNotificationList.get(i);
+ if (existing.sbn.getPackageName().equals(pkg)
+ && existing.sbn.getUserId() == userId) {
+ if (existing.sbn.getId() == id
+ && TextUtils.equals(existing.sbn.getTag(), tag)) {
+ break; // Allow updating existing notification
+ }
+ count++;
+ if (count >= MAX_PACKAGE_NOTIFICATIONS) {
+ mUsageStats.registerOverCountQuota(pkg);
+ Slog.e(TAG, "Package has already posted " + count
+ + " notifications. Not showing more. package=" + pkg);
+ return false;
+ }
+ }
+ }
+ }
+ }
+
+ // snoozed apps
+ if (mSnoozeHelper.isSnoozed(userId, pkg, r.getKey())) {
+ // TODO: log to event log
+ if (DBG) {
+ Slog.d(TAG, "Ignored enqueue for snoozed notification " + r.getKey());
+ }
+ mSnoozeHelper.update(userId, r);
+ savePolicyFile();
+ return false;
+ }
+
+
+ // blocked apps
+ if (isBlocked(r, mUsageStats)) {
+ return false;
+ }
+
+ return true;
+ }
+
+ protected boolean isBlocked(NotificationRecord r, NotificationUsageStats usageStats) {
+ final String pkg = r.sbn.getPackageName();
+ final int callingUid = r.sbn.getUid();
+
+ final boolean isPackageSuspended = isPackageSuspendedForUser(pkg, callingUid);
+ if (isPackageSuspended) {
+ Slog.e(TAG, "Suppressing notification from package due to package "
+ + "suspended by administrator.");
+ usageStats.registerSuspendedByAdmin(r);
+ return isPackageSuspended;
+ }
+
+ final boolean isBlocked = r.getImportance() == NotificationManager.IMPORTANCE_NONE
+ || r.getChannel().getImportance() == NotificationManager.IMPORTANCE_NONE
+ || !noteNotificationOp(pkg, callingUid);
+ if (isBlocked) {
+ Slog.e(TAG, "Suppressing notification from package by user request.");
+ usageStats.registerBlocked(r);
+ }
+ return isBlocked;
+ }
+
protected class EnqueueNotificationRunnable implements Runnable {
private final NotificationRecord r;
private final int userId;
@@ -3079,16 +3133,6 @@
mEnqueuedNotifications.add(r);
scheduleTimeoutLocked(r);
- if (mSnoozeHelper.isSnoozed(userId, r.sbn.getPackageName(), r.getKey())) {
- // TODO: log to event log
- if (DBG) {
- Slog.d(TAG, "Ignored enqueue for snoozed notification " + r.getKey());
- }
- mSnoozeHelper.update(userId, r);
- savePolicyFile();
- return;
- }
-
final StatusBarNotification n = r.sbn;
if (DBG) Slog.d(TAG, "EnqueueNotificationRunnable.run for: " + n.getKey());
NotificationRecord old = mNotificationsByKey.get(n.getKey());
@@ -3123,51 +3167,22 @@
mRankingHelper.extractSignals(r);
- // blocked apps
- if (isBlocked(r, mUsageStats)) {
- return;
- }
-
// tell the assistant service about the notification
if (mNotificationAssistants.isEnabled()) {
mNotificationAssistants.onNotificationEnqueued(r);
- mHandler.postDelayed(new PostNotificationRunnable(userId, r.getKey()),
+ mHandler.postDelayed(new PostNotificationRunnable(r.getKey()),
DELAY_FOR_ASSISTANT_TIME);
} else {
- mHandler.post(new PostNotificationRunnable(userId, r.getKey()));
+ mHandler.post(new PostNotificationRunnable(r.getKey()));
}
}
}
-
- protected boolean isBlocked(NotificationRecord r, NotificationUsageStats usageStats) {
- final String pkg = r.sbn.getPackageName();
- final int callingUid = r.sbn.getUid();
-
- final boolean isPackageSuspended = isPackageSuspendedForUser(pkg, callingUid);
- if (isPackageSuspended) {
- Slog.e(TAG, "Suppressing notification from package due to package "
- + "suspended by administrator.");
- usageStats.registerSuspendedByAdmin(r);
- return isPackageSuspended;
- }
-
- final boolean isBlocked = r.getImportance() == NotificationManager.IMPORTANCE_NONE
- || r.getChannel().getImportance() == NotificationManager.IMPORTANCE_NONE
- || !noteNotificationOp(pkg, callingUid);
- if (isBlocked) {
- Slog.e(TAG, "Suppressing notification from package by user request.");
- usageStats.registerBlocked(r);
- }
- return isBlocked;
- }
}
protected class PostNotificationRunnable implements Runnable {
private final String key;
- private final int userId;
- PostNotificationRunnable(int userId, String key) {
- this.userId = userId;
+ PostNotificationRunnable(String key) {
this.key = key;
}
diff --git a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
index 51742d1..db010b8 100644
--- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -223,10 +223,7 @@
NotificationChannel channel = new NotificationChannel("id", "name",
NotificationManager.IMPORTANCE_HIGH);
NotificationRecord r = generateNotificationRecord(channel);
- NotificationManagerService.EnqueueNotificationRunnable enqueue =
- mNotificationManagerService.new EnqueueNotificationRunnable(UserHandle.USER_SYSTEM,
- r);
- assertTrue(enqueue.isBlocked(r, usageStats));
+ assertTrue(mNotificationManagerService.isBlocked(r, usageStats));
verify(usageStats, times(1)).registerSuspendedByAdmin(eq(r));
}
@@ -240,10 +237,7 @@
NotificationManager.IMPORTANCE_HIGH);
channel.setImportance(NotificationManager.IMPORTANCE_NONE);
NotificationRecord r = generateNotificationRecord(channel);
- NotificationManagerService.EnqueueNotificationRunnable enqueue =
- mNotificationManagerService.new EnqueueNotificationRunnable(UserHandle.USER_SYSTEM,
- r);
- assertTrue(enqueue.isBlocked(r, usageStats));
+ assertTrue(mNotificationManagerService.isBlocked(r, usageStats));
verify(usageStats, times(1)).registerBlocked(eq(r));
}
@@ -257,10 +251,7 @@
NotificationManager.IMPORTANCE_HIGH);
NotificationRecord r = generateNotificationRecord(channel);
r.setUserImportance(NotificationManager.IMPORTANCE_NONE);
- NotificationManagerService.EnqueueNotificationRunnable enqueue =
- mNotificationManagerService.new EnqueueNotificationRunnable(UserHandle.USER_SYSTEM,
- r);
- assertTrue(enqueue.isBlocked(r, usageStats));
+ assertTrue(mNotificationManagerService.isBlocked(r, usageStats));
verify(usageStats, times(1)).registerBlocked(eq(r));
}