diff options
| author | 2023-03-27 16:37:28 +0200 | |
|---|---|---|
| committer | 2023-04-24 12:33:41 +0200 | |
| commit | fd671b59d43cdecc8e1ba8ce076054c053244013 (patch) | |
| tree | 986b9dc1fe27a07731e0124916925adca268e68c | |
| parent | 844c5aec4826d6e12e6f25546a2cdd5b73205f2c (diff) | |
Rework how notification posts are logged to statsd
Instead of writing to statsd during PostNotificationRunnable, do it after all listeners have been notified. This prepares the code to also log timing information (i.e. total time elapsed from notify() until SystemUI has been told about it).
Test: atest, manual, & statsd_testdrive
Bug: 275044361
Change-Id: I9ceb48e67c493cb434bcaac9f14734ba30271e8c
6 files changed, 528 insertions, 168 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 1301cd476c26..f8d05192f293 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -124,6 +124,7 @@ import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_C import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_CHANNEL_PREFERENCES; import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES; import static com.android.internal.util.Preconditions.checkArgument; +import static com.android.internal.util.Preconditions.checkNotNull; import static com.android.server.am.PendingIntentRecord.FLAG_ACTIVITY_SENDER; import static com.android.server.am.PendingIntentRecord.FLAG_BROADCAST_SENDER; import static com.android.server.am.PendingIntentRecord.FLAG_SERVICE_SENDER; @@ -277,7 +278,6 @@ import android.widget.Toast; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.internal.app.IAppOpsService; import com.android.internal.compat.IPlatformCompat; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.internal.config.sysui.SystemUiSystemPropertiesFlags; @@ -306,7 +306,6 @@ import com.android.server.EventLogTags; import com.android.server.IoThread; import com.android.server.LocalServices; import com.android.server.SystemService; -import com.android.server.UiThread; import com.android.server.job.JobSchedulerInternal; import com.android.server.lights.LightsManager; import com.android.server.lights.LogicalLight; @@ -560,10 +559,10 @@ public class NotificationManagerService extends SystemService { private PermissionHelper mPermissionHelper; private UsageStatsManagerInternal mUsageStatsManagerInternal; private TelecomManager mTelecomManager; + private PostNotificationTrackerFactory mPostNotificationTrackerFactory; final IBinder mForegroundToken = new Binder(); private WorkerHandler mHandler; - private Handler mUiHandler; private final HandlerThread mRankingThread = new HandlerThread("ranker", Process.THREAD_PRIORITY_BACKGROUND); @@ -572,7 +571,6 @@ public class NotificationManagerService extends SystemService { private boolean mUseAttentionLight; boolean mHasLight = true; - boolean mLightEnabled; boolean mSystemReady; private boolean mDisableNotificationEffects; @@ -629,7 +627,6 @@ public class NotificationManagerService extends SystemService { ArrayList<String> mLights = new ArrayList<>(); private AppOpsManager mAppOps; - private IAppOpsService mAppOpsService; private UsageStatsManagerInternal mAppUsageStats; private DevicePolicyManagerInternal mDpm; private StatsManager mStatsManager; @@ -925,7 +922,7 @@ public class NotificationManagerService extends SystemService { if (oldFlags != flags) { summary.getSbn().getNotification().flags = flags; mHandler.post(new EnqueueNotificationRunnable(userId, summary, isAppForeground, - SystemClock.elapsedRealtime())); + mPostNotificationTrackerFactory.newTracker())); } } @@ -1458,7 +1455,8 @@ public class NotificationManagerService extends SystemService { // Force isAppForeground true here, because for sysui's purposes we // want to adjust the flag behaviour. mHandler.post(new EnqueueNotificationRunnable(r.getUser().getIdentifier(), - r, true /* isAppForeground*/, SystemClock.elapsedRealtime())); + r, true /* isAppForeground*/, + mPostNotificationTrackerFactory.newTracker())); } } } @@ -1488,7 +1486,8 @@ public class NotificationManagerService extends SystemService { // want to be able to adjust the flag behaviour. mHandler.post( new EnqueueNotificationRunnable(r.getUser().getIdentifier(), r, - true /* isAppForeground */, SystemClock.elapsedRealtime())); + /* foreground= */ true, + mPostNotificationTrackerFactory.newTracker())); } } } @@ -2172,11 +2171,6 @@ public class NotificationManagerService extends SystemService { } @VisibleForTesting - void setPackageManager(IPackageManager packageManager) { - mPackageManager = packageManager; - } - - @VisibleForTesting void setRankingHelper(RankingHelper rankingHelper) { mRankingHelper = rankingHelper; } @@ -2230,15 +2224,15 @@ public class NotificationManagerService extends SystemService { ActivityManager activityManager, GroupHelper groupHelper, IActivityManager am, ActivityTaskManagerInternal atm, UsageStatsManagerInternal appUsageStats, DevicePolicyManagerInternal dpm, IUriGrantsManager ugm, - UriGrantsManagerInternal ugmInternal, AppOpsManager appOps, IAppOpsService iAppOps, - UserManager userManager, + UriGrantsManagerInternal ugmInternal, AppOpsManager appOps, UserManager userManager, NotificationHistoryManager historyManager, StatsManager statsManager, TelephonyManager telephonyManager, ActivityManagerInternal ami, MultiRateLimiter toastRateLimiter, PermissionHelper permissionHelper, UsageStatsManagerInternal usageStatsManagerInternal, TelecomManager telecomManager, NotificationChannelLogger channelLogger, SystemUiSystemPropertiesFlags.FlagResolver flagResolver, - PermissionManager permissionManager) { + PermissionManager permissionManager, + PostNotificationTrackerFactory postNotificationTrackerFactory) { mHandler = handler; Resources resources = getContext().getResources(); mMaxPackageEnqueueRate = Settings.Global.getFloat(getContext().getContentResolver(), @@ -2260,7 +2254,6 @@ public class NotificationManagerService extends SystemService { mUmInternal = LocalServices.getService(UserManagerInternal.class); mUsageStatsManagerInternal = usageStatsManagerInternal; mAppOps = appOps; - mAppOpsService = iAppOps; mAppUsageStats = appUsageStats; mAlarmManager = (AlarmManager) getContext().getSystemService(Context.ALARM_SERVICE); mCompanionManager = companionManager; @@ -2270,11 +2263,11 @@ public class NotificationManagerService extends SystemService { mDpm = dpm; mUm = userManager; mTelecomManager = telecomManager; + mPostNotificationTrackerFactory = postNotificationTrackerFactory; mPlatformCompat = IPlatformCompat.Stub.asInterface( ServiceManager.getService(Context.PLATFORM_COMPAT_SERVICE)); mStrongAuthTracker = new StrongAuthTracker(getContext()); - mUiHandler = new Handler(UiThread.get().getLooper()); String[] extractorNames; try { extractorNames = resources.getStringArray(R.array.config_notificationSignalExtractors); @@ -2557,7 +2550,6 @@ public class NotificationManagerService extends SystemService { UriGrantsManager.getService(), LocalServices.getService(UriGrantsManagerInternal.class), getContext().getSystemService(AppOpsManager.class), - IAppOpsService.Stub.asInterface(ServiceManager.getService(Context.APP_OPS_SERVICE)), getContext().getSystemService(UserManager.class), new NotificationHistoryManager(getContext(), handler), mStatsManager = (StatsManager) getContext().getSystemService( @@ -2570,7 +2562,8 @@ public class NotificationManagerService extends SystemService { LocalServices.getService(UsageStatsManagerInternal.class), getContext().getSystemService(TelecomManager.class), new NotificationChannelLoggerImpl(), SystemUiSystemPropertiesFlags.getResolver(), - getContext().getSystemService(PermissionManager.class)); + getContext().getSystemService(PermissionManager.class), + new PostNotificationTrackerFactory() {}); publishBinderService(Context.NOTIFICATION_SERVICE, mService, /* allowIsolated= */ false, DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_NORMAL); @@ -2686,7 +2679,7 @@ public class NotificationManagerService extends SystemService { final boolean isAppForeground = mActivityManager.getPackageImportance(pkg) == IMPORTANCE_FOREGROUND; mHandler.post(new EnqueueNotificationRunnable(userId, r, isAppForeground, - SystemClock.elapsedRealtime())); + mPostNotificationTrackerFactory.newTracker())); } } @@ -6559,6 +6552,26 @@ 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, boolean postSilently) { + PostNotificationTracker tracker = mPostNotificationTrackerFactory.newTracker(); + boolean enqueued = false; + try { + enqueued = enqueueNotificationInternal(pkg, opPkg, callingUid, callingPid, tag, id, + notification, incomingUserId, postSilently, tracker); + } finally { + if (!enqueued) { + tracker.cancel(); + } + } + } + + /** + * @return True if we successfully processed the notification and handed off the task of + * enqueueing it to a background thread; false otherwise. + */ + private boolean 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, + PostNotificationTracker tracker) { if (DBG) { Slog.v(TAG, "enqueueNotificationInternal: pkg=" + pkg + " id=" + id + " notification=" + notification); @@ -6607,7 +6620,7 @@ public class NotificationManagerService extends SystemService { throw new SecurityException("Invalid FGS notification", e); } Slog.e(TAG, "Cannot fix notification", e); - return; + return false; } if (policy == ServiceNotificationPolicy.UPDATE_ONLY) { @@ -6616,7 +6629,7 @@ public class NotificationManagerService extends SystemService { // handling. if (!isNotificationShownInternal(pkg, tag, id, userId)) { reportForegroundServiceUpdate(false, notification, id, pkg, userId); - return; + return false; } } @@ -6657,7 +6670,7 @@ public class NotificationManagerService extends SystemService { "Failed to post notification on channel \"" + channelId + "\"\n" + "See log for more details"); } - return; + return false; } final NotificationRecord r = new NotificationRecord(getContext(), n, channel); @@ -6704,7 +6717,7 @@ public class NotificationManagerService extends SystemService { if (!checkDisqualifyingFeatures(userId, notificationUid, id, tag, r, r.getSbn().getOverrideGroupKey() != null)) { - return; + return false; } if (info != null) { @@ -6745,8 +6758,8 @@ public class NotificationManagerService extends SystemService { } finally { Binder.restoreCallingIdentity(token); } - mHandler.post(new EnqueueNotificationRunnable(userId, r, isAppForeground, - SystemClock.elapsedRealtime())); + mHandler.post(new EnqueueNotificationRunnable(userId, r, isAppForeground, tracker)); + return true; } private void onConversationRemovedInternal(String pkg, int uid, Set<String> shortcuts) { @@ -7061,7 +7074,7 @@ public class NotificationManagerService extends SystemService { mHandler.post( new NotificationManagerService.EnqueueNotificationRunnable( r.getUser().getIdentifier(), r, isAppForeground, - SystemClock.elapsedRealtime())); + mPostNotificationTrackerFactory.newTracker())); } } } @@ -7402,7 +7415,6 @@ public class NotificationManagerService extends SystemService { } else { Log.w(TAG, "Cannot snooze " + r.getKey() + ": too many snoozed notifications"); } - } @GuardedBy("mNotificationLock") @@ -7599,28 +7611,43 @@ public class NotificationManagerService extends SystemService { private final NotificationRecord r; private final int userId; private final boolean isAppForeground; - private final long enqueueElapsedTimeMs; + private final PostNotificationTracker mTracker; EnqueueNotificationRunnable(int userId, NotificationRecord r, boolean foreground, - @ElapsedRealtimeLong long enqueueElapsedTimeMs) { + PostNotificationTracker tracker) { this.userId = userId; this.r = r; this.isAppForeground = foreground; - this.enqueueElapsedTimeMs = enqueueElapsedTimeMs; + this.mTracker = checkNotNull(tracker); } @Override public void run() { + boolean enqueued = false; + try { + enqueued = enqueueNotification(); + } finally { + if (!enqueued) { + mTracker.cancel(); + } + } + } + + /** + * @return True if we successfully enqueued the notification and handed off the task of + * posting it to a background thread; false otherwise. + */ + private boolean enqueueNotification() { synchronized (mNotificationLock) { - final Long snoozeAt = + final long snoozeAt = mSnoozeHelper.getSnoozeTimeForUnpostedNotification( r.getUser().getIdentifier(), r.getSbn().getPackageName(), r.getSbn().getKey()); final long currentTime = System.currentTimeMillis(); - if (snoozeAt.longValue() > currentTime) { + if (snoozeAt > currentTime) { (new SnoozeNotificationRunnable(r.getSbn().getKey(), - snoozeAt.longValue() - currentTime, null)).snoozeLocked(r); - return; + snoozeAt - currentTime, null)).snoozeLocked(r); + return false; } final String contextId = @@ -7630,7 +7657,7 @@ public class NotificationManagerService extends SystemService { if (contextId != null) { (new SnoozeNotificationRunnable(r.getSbn().getKey(), 0, contextId)).snoozeLocked(r); - return; + return false; } mEnqueuedNotifications.add(r); @@ -7681,12 +7708,14 @@ public class NotificationManagerService extends SystemService { mAssistants.onNotificationEnqueuedLocked(r); mHandler.postDelayed( new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), enqueueElapsedTimeMs), + r.getUid(), mTracker), DELAY_FOR_ASSISTANT_TIME); } else { - mHandler.post(new PostNotificationRunnable(r.getKey(), - r.getSbn().getPackageName(), r.getUid(), enqueueElapsedTimeMs)); + mHandler.post( + new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), + r.getUid(), mTracker)); } + return true; } } } @@ -7708,22 +7737,37 @@ public class NotificationManagerService extends SystemService { protected class PostNotificationRunnable implements Runnable { private final String key; - private final long postElapsedTimeMs; private final String pkg; private final int uid; + private final PostNotificationTracker mTracker; - PostNotificationRunnable(String key, String pkg, int uid, - @ElapsedRealtimeLong long postElapsedTimeMs) { + PostNotificationRunnable(String key, String pkg, int uid, PostNotificationTracker tracker) { this.key = key; this.pkg = pkg; this.uid = uid; - this.postElapsedTimeMs = postElapsedTimeMs; + this.mTracker = checkNotNull(tracker); } @Override public void run() { + boolean posted = false; + try { + posted = postNotification(); + } finally { + if (!posted) { + mTracker.cancel(); + } + } + } + + /** + * @return True if we successfully processed the notification and handed off the task of + * notifying all listeners to a background thread; false otherwise. + */ + private boolean postNotification() { boolean appBanned = !areNotificationsEnabledForPackageInt(pkg, uid); boolean isCallNotification = isCallNotification(pkg, uid); + boolean posted = false; synchronized (mNotificationLock) { try { NotificationRecord r = null; @@ -7737,7 +7781,7 @@ public class NotificationManagerService extends SystemService { } if (r == null) { Slog.i(TAG, "Cannot find enqueued record for key: " + key); - return; + return false; } final StatusBarNotification n = r.getSbn(); @@ -7751,7 +7795,7 @@ public class NotificationManagerService extends SystemService { if (DBG) { Slog.e(TAG, "Suppressing notification from package " + pkg); } - return; + return false; } final boolean isPackageSuspended = @@ -7774,7 +7818,7 @@ public class NotificationManagerService extends SystemService { mNotificationList.add(r); mUsageStats.registerPostedByApp(r); mUsageStatsManagerInternal.reportNotificationPosted(r.getSbn().getOpPkg(), - r.getSbn().getUser(), postElapsedTimeMs); + r.getSbn().getUser(), mTracker.getStartTime()); final boolean isInterruptive = isVisuallyInterruptive(null, r); r.setInterruptive(isInterruptive); r.setTextChanged(isInterruptive); @@ -7783,7 +7827,7 @@ public class NotificationManagerService extends SystemService { mNotificationList.set(index, r); mUsageStats.registerUpdatedByApp(r, old); mUsageStatsManagerInternal.reportNotificationUpdated(r.getSbn().getOpPkg(), - r.getSbn().getUser(), postElapsedTimeMs); + r.getSbn().getUser(), mTracker.getStartTime()); // Make sure we don't lose the foreground service state. notification.flags |= old.getNotification().flags & FLAG_FOREGROUND_SERVICE; @@ -7810,8 +7854,14 @@ public class NotificationManagerService extends SystemService { } if (notification.getSmallIcon() != null) { + mTracker.setReport( + mNotificationRecordLogger.prepareToLogNotificationPosted(r, old, + position, buzzBeepBlinkLoggingCode, + getGroupInstanceId(r.getSbn().getGroupKey()))); + notifyListenersPostedAndLogLocked(r, old, mTracker); + posted = true; + StatusBarNotification oldSbn = (old != null) ? old.getSbn() : null; - mListeners.notifyPostedLocked(r, old); if (oldSbn == null || !Objects.equals(oldSbn.getGroup(), n.getGroup()) || oldSbn.getNotification().flags != n.getNotification().flags) { @@ -7852,10 +7902,6 @@ public class NotificationManagerService extends SystemService { maybeRecordInterruptionLocked(r); maybeRegisterMessageSent(r); maybeReportForegroundServiceUpdate(r, true); - - // Log event to statsd - mNotificationRecordLogger.maybeLogNotificationPosted(r, old, position, - buzzBeepBlinkLoggingCode, getGroupInstanceId(r.getSbn().getGroupKey())); } finally { int N = mEnqueuedNotifications.size(); for (int i = 0; i < N; i++) { @@ -7867,6 +7913,7 @@ public class NotificationManagerService extends SystemService { } } } + return posted; } } @@ -10782,6 +10829,34 @@ public class NotificationManagerService extends SystemService { } } + /** + * Asynchronously notify all listeners about a posted (new or updated) notification. This + * should be called from {@link PostNotificationRunnable} to "complete" the post (since SysUI is + * one of the NLSes, and will display it to the user). + * + * <p>This method will call {@link PostNotificationTracker#finish} on the supplied tracker + * when every {@link NotificationListenerService} has received the news. + * + * <p>Also takes care of removing a notification that has been visible to a listener before, + * but isn't anymore. + */ + @GuardedBy("mNotificationLock") + private void notifyListenersPostedAndLogLocked(NotificationRecord r, NotificationRecord old, + @NonNull PostNotificationTracker tracker) { + List<Runnable> listenerCalls = mListeners.prepareNotifyPostedLocked(r, old, true); + mHandler.post(() -> { + for (Runnable listenerCall : listenerCalls) { + listenerCall.run(); + } + + tracker.finish(); + NotificationRecordLogger.NotificationReported report = tracker.getReport(); + if (report != null) { + mNotificationRecordLogger.logNotificationPosted(report); + } + }); + } + public class NotificationListeners extends ManagedServices { static final String TAG_ENABLED_NOTIFICATION_LISTENERS = "enabled_listeners"; static final String TAG_REQUESTED_LISTENERS = "request_listeners"; @@ -11135,28 +11210,59 @@ public class NotificationManagerService extends SystemService { } /** - * asynchronously notify all listeners about a new notification + * Asynchronously notify all listeners about a new or updated notification. Note that the + * notification is new or updated from the point of view of the NLS, but might not be + * "strictly new" <em>from the point of view of NMS itself</em> -- for example, this method + * is also invoked after exiting lockdown mode. * * <p> * Also takes care of removing a notification that has been visible to a listener before, * but isn't anymore. */ + @VisibleForTesting @GuardedBy("mNotificationLock") - public void notifyPostedLocked(NotificationRecord r, NotificationRecord old) { + void notifyPostedLocked(NotificationRecord r, NotificationRecord old) { notifyPostedLocked(r, old, true); } /** + * Asynchronously notify all listeners about a new or updated notification. Note that the + * notification is new or updated from the point of view of the NLS, but might not be + * "strictly new" <em>from the point of view of NMS itself</em> -- for example, this method + * is invoked after exiting lockdown mode. + * * @param notifyAllListeners notifies all listeners if true, else only notifies listeners - * targetting <= O_MR1 + * targeting <= O_MR1 */ + @VisibleForTesting @GuardedBy("mNotificationLock") void notifyPostedLocked(NotificationRecord r, NotificationRecord old, boolean notifyAllListeners) { + for (Runnable listenerCall : prepareNotifyPostedLocked(r, old, notifyAllListeners)) { + mHandler.post(listenerCall); + } + } + + /** + * "Prepares" to notify all listeners about the posted notification. + * + * <p>This method <em>does not invoke</em> the listeners; the caller should post each + * returned {@link Runnable} on a suitable thread to do so. + * + * @param notifyAllListeners notifies all listeners if true, else only notifies listeners + * targeting <= O_MR1 + * @return A list of {@link Runnable} operations to notify all listeners about the posted + * notification. + */ + @VisibleForTesting + @GuardedBy("mNotificationLock") + List<Runnable> prepareNotifyPostedLocked(NotificationRecord r, + NotificationRecord old, boolean notifyAllListeners) { if (isInLockDownMode(r.getUser().getIdentifier())) { - return; + return new ArrayList<>(); } + ArrayList<Runnable> listenerCalls = new ArrayList<>(); try { // Lazily initialized snapshots of the notification. StatusBarNotification sbn = r.getSbn(); @@ -11190,7 +11296,7 @@ public class NotificationManagerService extends SystemService { // This notification became invisible -> remove the old one. if (oldSbnVisible && !sbnVisible) { final StatusBarNotification oldSbnLightClone = oldSbn.cloneLight(); - mHandler.post(() -> notifyRemoved( + listenerCalls.add(() -> notifyRemoved( info, oldSbnLightClone, update, null, REASON_USER_STOPPED)); continue; } @@ -11206,11 +11312,12 @@ public class NotificationManagerService extends SystemService { false /* direct */, false /* retainOnUpdate */); final StatusBarNotification sbnToPost = trimCache.ForListener(info); - mHandler.post(() -> notifyPosted(info, sbnToPost, update)); + listenerCalls.add(() -> notifyPosted(info, sbnToPost, update)); } } catch (Exception e) { Slog.e(TAG, "Could not notify listeners for " + r.getKey(), e); } + return listenerCalls; } /** @@ -11392,7 +11499,7 @@ public class NotificationManagerService extends SystemService { int numChangedNotifications = changedNotifications.size(); for (int i = 0; i < numChangedNotifications; i++) { NotificationRecord rec = changedNotifications.get(i); - mListeners.notifyPostedLocked(rec, rec, false); + notifyPostedLocked(rec, rec, false); } } @@ -11991,4 +12098,76 @@ public class NotificationManagerService extends SystemService { && !CompatChanges.isChangeEnabled(NOTIFICATION_TRAMPOLINE_BLOCK, uid); } } + + interface PostNotificationTrackerFactory { + default PostNotificationTracker newTracker() { + return new PostNotificationTracker(); + } + } + + static class PostNotificationTracker { + @ElapsedRealtimeLong private final long mStartTime; + @Nullable private NotificationRecordLogger.NotificationReported mReport; + private boolean mOngoing; + + @VisibleForTesting + PostNotificationTracker() { + // TODO(b/275044361): (Conditionally) receive a wakelock. + mStartTime = SystemClock.elapsedRealtime(); + mOngoing = true; + if (DBG) { + Slog.d(TAG, "PostNotification: Started"); + } + } + + void setReport(@Nullable NotificationRecordLogger.NotificationReported report) { + mReport = report; + } + + NotificationRecordLogger.NotificationReported getReport() { + return mReport; + } + + @ElapsedRealtimeLong + long getStartTime() { + return mStartTime; + } + + @VisibleForTesting + boolean isOngoing() { + return mOngoing; + } + + void cancel() { + if (!isOngoing()) { + Log.wtfStack(TAG, "cancel() called on already-finished tracker"); + return; + } + mOngoing = false; + + // TODO(b/275044361): Release wakelock. + + if (DBG) { + long elapsedTime = SystemClock.elapsedRealtime() - mStartTime; + Slog.d(TAG, TextUtils.formatSimple("PostNotification: Abandoned after %d ms", + elapsedTime)); + } + } + + void finish() { + if (!isOngoing()) { + Log.wtfStack(TAG, "finish() called on already-finished tracker"); + return; + } + mOngoing = false; + + // TODO(b/275044361): Release wakelock. + + long elapsedTime = SystemClock.elapsedRealtime() - mStartTime; + if (DBG) { + Slog.d(TAG, + TextUtils.formatSimple("PostNotification: Finished in %d ms", elapsedTime)); + } + } + } } diff --git a/services/core/java/com/android/server/notification/NotificationRecordLogger.java b/services/core/java/com/android/server/notification/NotificationRecordLogger.java index 25d619dea296..71ebf7a39610 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLogger.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLogger.java @@ -42,26 +42,45 @@ import java.util.Objects; * in production. Use NotificationRecordLoggerFake for testing. * @hide */ -public interface NotificationRecordLogger { +interface NotificationRecordLogger { // The high-level interface used by clients. /** - * May log a NotificationReported atom reflecting the posting or update of a notification. - * @param r The new NotificationRecord. If null, no action is taken. - * @param old The previous NotificationRecord. Null if there was no previous record. + * Prepare to log an atom reflecting the posting or update of a notification. + * + * The returned {@link NotificationReported} object, if any, should be supplied to + * {@link #logNotificationPosted}. Because only some updates are considered "interesting + * enough" to log, this method may return {@code null}. In that case, the follow-up call + * should not be performed. + * + * @param r The new {@link NotificationRecord}. + * @param old The previous {@link NotificationRecord}. Null if there was no previous record. * @param position The position at which this notification is ranked. * @param buzzBeepBlink Logging code reflecting whether this notification alerted the user. - * @param groupId The instance Id of the group summary notification, or null. + * @param groupId The {@link InstanceId} of the group summary notification, or null. */ - void maybeLogNotificationPosted(@Nullable NotificationRecord r, + @Nullable + default NotificationReported prepareToLogNotificationPosted(@Nullable NotificationRecord r, @Nullable NotificationRecord old, int position, int buzzBeepBlink, - InstanceId groupId); + InstanceId groupId) { + NotificationRecordPair p = new NotificationRecordPair(r, old); + if (!p.shouldLogReported(buzzBeepBlink)) { + return null; + } + return new NotificationReported(p, NotificationReportedEvent.fromRecordPair(p), position, + buzzBeepBlink, groupId); + } + + /** + * Log a NotificationReported atom reflecting the posting or update of a notification. + */ + void logNotificationPosted(NotificationReported nr); /** * Logs a NotificationReported atom reflecting an adjustment to a notification. - * Unlike maybeLogNotificationPosted, this method is guaranteed to log a notification update, + * Unlike for posted notifications, this method is guaranteed to log a notification update, * so the caller must take responsibility for checking that that logging update is necessary, * and that the notification is meaningfully changed. * @param r The NotificationRecord. If null, no action is taken. @@ -125,7 +144,7 @@ public interface NotificationRecordLogger { public static NotificationReportedEvent fromRecordPair(NotificationRecordPair p) { return (p.old != null) ? NotificationReportedEvent.NOTIFICATION_UPDATED : - NotificationReportedEvent.NOTIFICATION_POSTED; + NotificationReportedEvent.NOTIFICATION_POSTED; } } @@ -450,6 +469,68 @@ public interface NotificationRecordLogger { } + /** Data object corresponding to a NotificationReported atom. + * + * Fields must be kept in sync with frameworks/proto_logging/stats/atoms.proto. + */ + class NotificationReported { + final int event_id; + final int uid; + final String package_name; + final int instance_id; + final int notification_id_hash; + final int channel_id_hash; + final int group_id_hash; + final int group_instance_id; + final boolean is_group_summary; + final String category; + final int style; + final int num_people; + final int position; + final int importance; + final int alerting; + final int importance_source; + final int importance_initial; + final int importance_initial_source; + final int importance_asst; + final int assistant_hash; + final float assistant_ranking_score; + final boolean is_ongoing; + final boolean is_foreground_service; + final long timeout_millis; + final boolean is_non_dismissible; + + NotificationReported(NotificationRecordPair p, + NotificationReportedEvent eventType, int position, int buzzBeepBlink, + InstanceId groupId) { + this.event_id = eventType.getId(); + this.uid = p.r.getUid(); + this.package_name = p.r.getSbn().getPackageName(); + this.instance_id = p.getInstanceId(); + this.notification_id_hash = p.getNotificationIdHash(); + this.channel_id_hash = p.getChannelIdHash(); + this.group_id_hash = p.getGroupIdHash(); + this.group_instance_id = (groupId == null) ? 0 : groupId.getId(); + this.is_group_summary = p.r.getSbn().getNotification().isGroupSummary(); + this.category = p.r.getSbn().getNotification().category; + this.style = p.getStyle(); + this.num_people = p.getNumPeople(); + this.position = position; + this.importance = NotificationRecordLogger.getLoggingImportance(p.r); + this.alerting = buzzBeepBlink; + this.importance_source = p.r.getImportanceExplanationCode(); + this.importance_initial = p.r.getInitialImportance(); + this.importance_initial_source = p.r.getInitialImportanceExplanationCode(); + this.importance_asst = p.r.getAssistantImportance(); + this.assistant_hash = p.getAssistantHash(); + this.assistant_ranking_score = p.r.getRankingScore(); + this.is_ongoing = p.r.getSbn().isOngoing(); + this.is_foreground_service = NotificationRecordLogger.isForegroundService(p.r); + this.timeout_millis = p.r.getSbn().getNotification().getTimeoutAfter(); + this.is_non_dismissible = NotificationRecordLogger.isNonDismissible(p.r); + } + } + /** * @param r NotificationRecord * @return Logging importance of record, taking important conversation channels into account. diff --git a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java index 9a1f19d0c514..cd457b793390 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java @@ -27,20 +27,13 @@ import com.android.internal.util.FrameworkStatsLog; * Standard implementation of NotificationRecordLogger interface. * @hide */ -public class NotificationRecordLoggerImpl implements NotificationRecordLogger { +class NotificationRecordLoggerImpl implements NotificationRecordLogger { private UiEventLogger mUiEventLogger = new UiEventLoggerImpl(); @Override - public void maybeLogNotificationPosted(NotificationRecord r, NotificationRecord old, - int position, int buzzBeepBlink, - InstanceId groupId) { - NotificationRecordPair p = new NotificationRecordPair(r, old); - if (!p.shouldLogReported(buzzBeepBlink)) { - return; - } - writeNotificationReportedAtom(p, NotificationReportedEvent.fromRecordPair(p), - position, buzzBeepBlink, groupId); + public void logNotificationPosted(NotificationReported nr) { + writeNotificationReportedAtom(nr); } @Override @@ -48,48 +41,40 @@ public class NotificationRecordLoggerImpl implements NotificationRecordLogger { int position, int buzzBeepBlink, InstanceId groupId) { NotificationRecordPair p = new NotificationRecordPair(r, null); - writeNotificationReportedAtom(p, NotificationReportedEvent.NOTIFICATION_ADJUSTED, - position, buzzBeepBlink, groupId); + writeNotificationReportedAtom( + new NotificationReported(p, NotificationReportedEvent.NOTIFICATION_ADJUSTED, + position, buzzBeepBlink, groupId)); } - private void writeNotificationReportedAtom(NotificationRecordPair p, - NotificationReportedEvent eventType, int position, int buzzBeepBlink, - InstanceId groupId) { - FrameworkStatsLog.write(FrameworkStatsLog.NOTIFICATION_REPORTED, - /* int32 event_id = 1 */ eventType.getId(), - /* int32 uid = 2 */ p.r.getUid(), - /* string package_name = 3 */ p.r.getSbn().getPackageName(), - /* int32 instance_id = 4 */ p.getInstanceId(), - /* int32 notification_id_hash = 5 */ p.getNotificationIdHash(), - /* int32 channel_id_hash = 6 */ p.getChannelIdHash(), - /* string group_id_hash = 7 */ p.getGroupIdHash(), - /* int32 group_instance_id = 8 */ (groupId == null) ? 0 : groupId.getId(), - /* bool is_group_summary = 9 */ p.r.getSbn().getNotification().isGroupSummary(), - /* string category = 10 */ p.r.getSbn().getNotification().category, - /* int32 style = 11 */ p.getStyle(), - /* int32 num_people = 12 */ p.getNumPeople(), - /* int32 position = 13 */ position, - /* android.stats.sysui.NotificationImportance importance = 14 */ - NotificationRecordLogger.getLoggingImportance(p.r), - /* int32 alerting = 15 */ buzzBeepBlink, - /* NotificationImportanceExplanation importance_source = 16 */ - p.r.getImportanceExplanationCode(), - /* android.stats.sysui.NotificationImportance importance_initial = 17 */ - p.r.getInitialImportance(), - /* NotificationImportanceExplanation importance_initial_source = 18 */ - p.r.getInitialImportanceExplanationCode(), - /* android.stats.sysui.NotificationImportance importance_asst = 19 */ - p.r.getAssistantImportance(), - /* int32 assistant_hash = 20 */ p.getAssistantHash(), - /* float assistant_ranking_score = 21 */ p.r.getRankingScore(), - /* bool is_ongoing = 22 */ p.r.getSbn().isOngoing(), - /* bool is_foreground_service = 23 */ - NotificationRecordLogger.isForegroundService(p.r), - /* optional int64 timeout_millis = 24 */ - p.r.getSbn().getNotification().getTimeoutAfter(), - /* bool is_nondismissible = 25 */ - NotificationRecordLogger.isNonDismissible(p.r) - ); + private void writeNotificationReportedAtom( + NotificationReported notificationReported) { + FrameworkStatsLog.write( + FrameworkStatsLog.NOTIFICATION_REPORTED, + notificationReported.event_id, + notificationReported.uid, + notificationReported.package_name, + notificationReported.instance_id, + notificationReported.notification_id_hash, + notificationReported.channel_id_hash, + notificationReported.group_id_hash, + notificationReported.group_instance_id, + notificationReported.is_group_summary, + notificationReported.category, + notificationReported.style, + notificationReported.num_people, + notificationReported.position, + notificationReported.importance, + notificationReported.alerting, + notificationReported.importance_source, + notificationReported.importance_initial, + notificationReported.importance_initial_source, + notificationReported.importance_asst, + notificationReported.assistant_hash, + notificationReported.assistant_ranking_score, + notificationReported.is_ongoing, + notificationReported.is_foreground_service, + notificationReported.timeout_millis, + notificationReported.is_non_dismissible); } @Override diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 9cfdaa7cad0c..4d27a9b3e3d2 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -40,7 +40,6 @@ import static android.app.NotificationManager.IMPORTANCE_HIGH; import static android.app.NotificationManager.IMPORTANCE_LOW; import static android.app.NotificationManager.IMPORTANCE_MAX; import static android.app.NotificationManager.IMPORTANCE_NONE; -import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED; import static android.app.NotificationManager.Policy.PRIORITY_CATEGORY_CALLS; import static android.app.NotificationManager.Policy.PRIORITY_CATEGORY_CONVERSATIONS; import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_AMBIENT; @@ -95,6 +94,7 @@ import static junit.framework.Assert.assertSame; import static junit.framework.Assert.assertTrue; import static junit.framework.Assert.fail; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyLong; @@ -213,7 +213,6 @@ import android.widget.RemoteViews; import androidx.test.InstrumentationRegistry; -import com.android.internal.app.IAppOpsService; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.Flag; import com.android.internal.config.sysui.TestableFlagResolver; @@ -233,6 +232,8 @@ import com.android.server.lights.LightsManager; import com.android.server.lights.LogicalLight; import com.android.server.notification.NotificationManagerService.NotificationAssistants; import com.android.server.notification.NotificationManagerService.NotificationListeners; +import com.android.server.notification.NotificationManagerService.PostNotificationTracker; +import com.android.server.notification.NotificationManagerService.PostNotificationTrackerFactory; import com.android.server.pm.PackageManagerService; import com.android.server.pm.UserManagerInternal; import com.android.server.policy.PermissionPolicyInternal; @@ -346,6 +347,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { private PermissionManager mPermissionManager; @Mock private DevicePolicyManagerInternal mDevicePolicyManager; + private final TestPostNotificationTrackerFactory mPostNotificationTrackerFactory = + new TestPostNotificationTrackerFactory(); @Mock IIntentSender pi1; @@ -382,8 +385,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Mock AppOpsManager mAppOpsManager; @Mock - IAppOpsService mAppOpsService; - @Mock private TestableNotificationManagerService.NotificationAssistantAccessGrantedCallback mNotificationAssistantAccessGrantedCallback; @Mock @@ -421,6 +422,18 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } } + private class TestPostNotificationTrackerFactory implements PostNotificationTrackerFactory { + + private final List<PostNotificationTracker> mCreatedTrackers = new ArrayList<>(); + + @Override + public PostNotificationTracker newTracker() { + PostNotificationTracker tracker = PostNotificationTrackerFactory.super.newTracker(); + mCreatedTrackers.add(tracker); + return tracker; + } + } + @Before public void setUp() throws Exception { // Shell permisssions will override permissions of our app, so add all necessary permissions @@ -554,10 +567,11 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mockLightsManager, mListeners, mAssistants, mConditionProviders, mCompanionMgr, mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager, mGroupHelper, mAm, mAtm, mAppUsageStats, mDevicePolicyManager, mUgm, mUgmInternal, - mAppOpsManager, mAppOpsService, mUm, mHistoryManager, mStatsManager, + mAppOpsManager, mUm, mHistoryManager, mStatsManager, mock(TelephonyManager.class), mAmi, mToastRateLimiter, mPermissionHelper, mock(UsageStatsManagerInternal.class), - mTelecomManager, mLogger, mTestFlagResolver, mPermissionManager); + mTelecomManager, mLogger, mTestFlagResolver, mPermissionManager, + mPostNotificationTrackerFactory); // Return first true for RoleObserver main-thread check when(mMainLooper.isCurrentThread()).thenReturn(true).thenReturn(false); mService.onBootPhase(SystemService.PHASE_SYSTEM_SERVICES_READY, mMainLooper); @@ -640,11 +654,22 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @After public void assertNotificationRecordLoggerCallsValid() { + waitForIdle(); // Finish async work, including all logging calls done by Runnables. for (NotificationRecordLoggerFake.CallRecord call : mNotificationRecordLogger.getCalls()) { if (call.wasLogged) { assertNotNull(call.event); } } + assertThat(mNotificationRecordLogger.getPendingLogs()).isEmpty(); + } + + @After + public void assertAllTrackersFinishedOrCancelled() { + // Verify that no trackers were left dangling. + for (PostNotificationTracker tracker : mPostNotificationTrackerFactory.mCreatedTrackers) { + assertThat(tracker.isOngoing()).isFalse(); + } + mPostNotificationTrackerFactory.mCreatedTrackers.clear(); } @After @@ -1448,7 +1473,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -1469,7 +1494,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -1709,6 +1734,68 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } @Test + public void enqueueNotificationWithTag_usesAndFinishesTracker() throws Exception { + mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testEnqueueNotificationWithTag_PopulatesGetActiveNotifications", 0, + generateNotificationRecord(null).getNotification(), 0); + + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers).hasSize(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers.get(0).isOngoing()).isTrue(); + + waitForIdle(); + + assertThat(mBinderService.getActiveNotifications(PKG)).hasLength(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers).hasSize(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers.get(0).isOngoing()).isFalse(); + } + + @Test + public void enqueueNotificationWithTag_throws_usesAndCancelsTracker() throws Exception { + // Simulate not enqueued due to rejected inputs. + assertThrows(Exception.class, + () -> mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testEnqueueNotificationWithTag_PopulatesGetActiveNotifications", 0, + /* notification= */ null, 0)); + + waitForIdle(); + + assertThat(mBinderService.getActiveNotifications(PKG)).hasLength(0); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers).hasSize(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers.get(0).isOngoing()).isFalse(); + } + + @Test + public void enqueueNotificationWithTag_notEnqueued_usesAndCancelsTracker() throws Exception { + // Simulate not enqueued due to snoozing inputs. + when(mSnoozeHelper.getSnoozeContextForUnpostedNotification(anyInt(), any(), any())) + .thenReturn("zzzzzzz"); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testEnqueueNotificationWithTag_PopulatesGetActiveNotifications", 0, + generateNotificationRecord(null).getNotification(), 0); + waitForIdle(); + + assertThat(mBinderService.getActiveNotifications(PKG)).hasLength(0); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers).hasSize(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers.get(0).isOngoing()).isFalse(); + } + + @Test + public void enqueueNotificationWithTag_notPosted_usesAndCancelsTracker() throws Exception { + // Simulate not posted due to blocked app. + when(mPermissionHelper.hasPermission(anyInt())).thenReturn(false); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testEnqueueNotificationWithTag_PopulatesGetActiveNotifications", 0, + generateNotificationRecord(null).getNotification(), 0); + waitForIdle(); + + assertThat(mBinderService.getActiveNotifications(PKG)).hasLength(0); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers).hasSize(1); + assertThat(mPostNotificationTrackerFactory.mCreatedTrackers.get(0).isOngoing()).isFalse(); + } + + @Test public void testCancelNonexistentNotification() throws Exception { mBinderService.cancelNotificationWithTag(PKG, PKG, "testCancelNonexistentNotification", 0, 0); @@ -1905,8 +1992,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.mSummaryByGroupKey.put("pkg", summary); mService.mAutobundledSummaries.put(0, new ArrayMap<>()); mService.mAutobundledSummaries.get(0).put("pkg", summary.getKey()); + mService.updateAutobundledSummaryFlags( 0, "pkg", GroupHelper.BASE_FLAGS | FLAG_ONGOING_EVENT, false); + waitForIdle(); assertTrue(summary.getSbn().isOngoing()); } @@ -1923,6 +2012,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.mSummaryByGroupKey.put("pkg", summary); mService.updateAutobundledSummaryFlags(0, "pkg", GroupHelper.BASE_FLAGS, false); + waitForIdle(); assertFalse(summary.getSbn().isOngoing()); } @@ -4225,7 +4315,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addEnqueuedNotification(r); NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -4244,7 +4334,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(update.getKey(), update.getSbn().getPackageName(), update.getUid(), - SystemClock.elapsedRealtime()); + mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -4264,7 +4354,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(update.getKey(), update.getSbn().getPackageName(), update.getUid(), - SystemClock.elapsedRealtime()); + mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -4284,7 +4374,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(update.getKey(), update.getSbn().getPackageName(), - update.getUid(), SystemClock.elapsedRealtime()); + update.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -4298,13 +4388,13 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addEnqueuedNotification(r); NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); r = generateNotificationRecord(mTestNotificationChannel, 1, null, false); r.setCriticality(CriticalNotificationExtractor.CRITICAL); runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); mService.addEnqueuedNotification(r); runnable.run(); @@ -4953,7 +5043,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.new PostNotificationRunnable(original.getKey(), original.getSbn().getPackageName(), original.getUid(), - SystemClock.elapsedRealtime()); + mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -4977,7 +5067,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.new PostNotificationRunnable(update.getKey(), update.getSbn().getPackageName(), update.getUid(), - SystemClock.elapsedRealtime()); + mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -7260,8 +7350,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(update.getKey(), r.getSbn().getPackageName(), - r.getUid(), - SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -7569,13 +7658,12 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void clearDefaultDnDPackageShouldEnableIt() throws RemoteException { - ComponentName deviceConfig = new ComponentName("device", "config"); ArrayMap<Boolean, ArrayList<ComponentName>> changed = generateResetComponentValues(); when(mAssistants.resetComponents(anyString(), anyInt())).thenReturn(changed); when(mListeners.resetComponents(anyString(), anyInt())).thenReturn(changed); - mService.getBinderService().clearData("device", 0, false); + mService.getBinderService().clearData("pkgName", 0, false); verify(mConditionProviders, times(1)).resetPackage( - eq("device"), eq(0)); + eq("pkgName"), eq(0)); } @Test @@ -8419,7 +8507,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(0, notifsBefore.length); Uri uri = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); - int uid = 0; // sysui on primary user mService.mNotificationDelegate.grantInlineReplyUriPermission( nr.getKey(), uri, nr.getSbn().getUser(), nr.getSbn().getPackageName(), @@ -8552,7 +8639,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { reset(mPackageManager); Uri uri1 = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); - Uri uri2 = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 2); // create an inline record a uri in it mService.mNotificationDelegate.grantInlineReplyUriPermission( @@ -9956,7 +10042,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addEnqueuedNotification(r); NotificationManagerService.PostNotificationRunnable runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -9973,7 +10059,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addEnqueuedNotification(r); runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -9990,7 +10076,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addEnqueuedNotification(r); runnable = mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); + r.getUid(), mPostNotificationTrackerFactory.newTracker()); runnable.run(); waitForIdle(); @@ -10082,10 +10168,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { // normal blocked notifications - blocked mService.addEnqueuedNotification(r); - NotificationManagerService.PostNotificationRunnable runnable = - mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), - r.getUid(), SystemClock.elapsedRealtime()); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats).registerBlocked(any()); @@ -10102,9 +10186,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { r = new NotificationRecord(mContext, sbn, mTestNotificationChannel); mService.addEnqueuedNotification(r); - runnable = mService.new PostNotificationRunnable( - r.getKey(), r.getSbn().getPackageName(), r.getUid(), SystemClock.elapsedRealtime()); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats).registerBlocked(any()); @@ -10116,7 +10199,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { when(mTelecomManager.isInManagedCall()).thenReturn(true); mService.addEnqueuedNotification(r); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats, never()).registerBlocked(any()); @@ -10129,7 +10213,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .thenReturn(true); mService.addEnqueuedNotification(r); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats, never()).registerBlocked(any()); @@ -10142,7 +10227,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.setTelecomManager(null); mService.addEnqueuedNotification(r); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats).registerBlocked(any()); @@ -10156,7 +10242,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.setTelecomManager(null); mService.addEnqueuedNotification(r); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats).registerBlocked(any()); @@ -10168,7 +10255,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { reset(mUsageStats); mService.addEnqueuedNotification(r); - runnable.run(); + mService.new PostNotificationRunnable(r.getKey(), r.getSbn().getPackageName(), r.getUid(), + mPostNotificationTrackerFactory.newTracker()).run(); waitForIdle(); verify(mUsageStats).registerBlocked(any()); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java index 8a11798bbf19..1bb35021d76c 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java @@ -16,11 +16,15 @@ package com.android.server.notification; +import androidx.annotation.Nullable; + import com.android.internal.logging.InstanceId; import com.android.internal.logging.UiEventLogger; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * Fake implementation of NotificationRecordLogger, for testing. @@ -60,7 +64,8 @@ class NotificationRecordLoggerFake implements NotificationRecordLogger { this.event = event; } } - private List<CallRecord> mCalls = new ArrayList<>(); + private final List<CallRecord> mCalls = new ArrayList<>(); + private final Map<NotificationReported, CallRecord> mPendingLogs = new HashMap<>(); public int numCalls() { return mCalls.size(); @@ -70,6 +75,10 @@ class NotificationRecordLoggerFake implements NotificationRecordLogger { return mCalls; } + List<NotificationReported> getPendingLogs() { + return new ArrayList<>(mPendingLogs.keySet()); + } + CallRecord get(int index) { return mCalls.get(index); } @@ -77,10 +86,31 @@ class NotificationRecordLoggerFake implements NotificationRecordLogger { return mCalls.get(index).event; } + @Nullable + @Override + public NotificationReported prepareToLogNotificationPosted(@Nullable NotificationRecord r, + @Nullable NotificationRecord old, int position, int buzzBeepBlink, InstanceId groupId) { + NotificationReported nr = NotificationRecordLogger.super.prepareToLogNotificationPosted(r, + old, position, buzzBeepBlink, groupId); + CallRecord callRecord = new CallRecord(r, old, position, buzzBeepBlink, groupId); + callRecord.wasLogged = false; + mCalls.add(callRecord); + if (nr != null) { + mPendingLogs.put(nr, callRecord); + } + return nr; + } + @Override - public void maybeLogNotificationPosted(NotificationRecord r, NotificationRecord old, - int position, int buzzBeepBlink, InstanceId groupId) { - mCalls.add(new CallRecord(r, old, position, buzzBeepBlink, groupId)); + public void logNotificationPosted(NotificationReported nr) { + CallRecord callRecord = mPendingLogs.get(nr); + if (callRecord == null) { + throw new IllegalStateException( + "Didn't find corresponding CallRecord in mPreparedCalls. Did you call " + + "logNotificationPosted() twice!?"); + } + mPendingLogs.remove(nr); + callRecord.wasLogged = true; } @Override diff --git a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java index 34bb664c9598..d4a2e9aa4f5b 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java @@ -28,7 +28,6 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -56,7 +55,6 @@ import android.telecom.TelecomManager; import android.telephony.TelephonyManager; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; -import android.testing.TestableContext; import android.testing.TestableLooper.RunWithLooper; import android.util.ArraySet; import android.util.AtomicFile; @@ -64,7 +62,6 @@ import android.util.Pair; import androidx.test.InstrumentationRegistry; -import com.android.internal.app.IAppOpsService; import com.android.internal.config.sysui.TestableFlagResolver; import com.android.internal.logging.InstanceIdSequence; import com.android.internal.logging.InstanceIdSequenceFake; @@ -162,14 +159,14 @@ public class RoleObserverTest extends UiServiceTestCase { mock(UsageStatsManagerInternal.class), mock(DevicePolicyManagerInternal.class), mock(IUriGrantsManager.class), mock(UriGrantsManagerInternal.class), - mock(AppOpsManager.class), mock(IAppOpsService.class), - mUm, mock(NotificationHistoryManager.class), + mock(AppOpsManager.class), mUm, mock(NotificationHistoryManager.class), mock(StatsManager.class), mock(TelephonyManager.class), mock(ActivityManagerInternal.class), mock(MultiRateLimiter.class), mock(PermissionHelper.class), - mock(UsageStatsManagerInternal.class), mock (TelecomManager.class), + mock(UsageStatsManagerInternal.class), mock(TelecomManager.class), mock(NotificationChannelLogger.class), new TestableFlagResolver(), - mock(PermissionManager.class)); + mock(PermissionManager.class), + new NotificationManagerService.PostNotificationTrackerFactory() {}); } catch (SecurityException e) { if (!e.getMessage().contains("Permission Denial: not allowed to send broadcast")) { throw e; |