Merge "Lifetime extension to not update on multicancel" into main (5e73c1b5) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/notification/NotificationManagerService.java

+48 −9

Original line number	Diff line number	Diff line
		@@ -1583,6 +1583,8 @@ public class NotificationManagerService extends SystemService {
		// respond to direct replies with updates. So we need to update System UI
		// immediately.
		if (lifetimeExtensionRefactor()) {
		// We need to reset this to allow the notif to be updated again.
		r.setCanceledAfterLifetimeExtension(false);
		maybeNotifySystemUiListenerLifetimeExtendedLocked(r,
		r.getSbn().getPackageName(), packageImportance);
		}
		@@ -1639,9 +1641,12 @@ public class NotificationManagerService extends SystemService {
		// respond to direct replies with updates. So we need to update System UI
		// immediately.
		if (lifetimeExtensionRefactor()) {
		// We need to reset this to allow the notif to be updated again.
		r.setCanceledAfterLifetimeExtension(false);
		maybeNotifySystemUiListenerLifetimeExtendedLocked(r,
		r.getSbn().getPackageName(), packageImportance);
		}

		r.recordSmartReplied();
		LogMaker logMaker = r.getLogMaker()
		.setCategory(MetricsEvent.SMART_REPLY_ACTION)
		@@ -11741,17 +11746,37 @@ public class NotificationManagerService extends SystemService {
		private void maybeNotifySystemUiListenerLifetimeExtendedLocked(NotificationRecord record,
		String pkg, int packageImportance) {
		if (record != null && (record.getSbn().getNotification().flags
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY) > 0) {
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY) > 0
		&& !record.isCanceledAfterLifetimeExtension()) {
		boolean isAppForeground = pkg != null && packageImportance == IMPORTANCE_FOREGROUND;

		// Lifetime extended notifications don't need to alert on state change.
		// Save the original Record's post silently value, so we can restore it after we send
		// the SystemUI specific silent update.
		boolean savedPostSilentlyState = record.shouldPostSilently();
		boolean savedOnlyAlertOnceState = (record.getNotification().flags
		& FLAG_ONLY_ALERT_ONCE) > 0;
		// Lifetime extended notifications don't need to alert on new state change.
		record.setPostSilently(true);
		// We also set FLAG_ONLY_ALERT_ONCE to avoid the notification from HUN-ing again.
		record.getNotification().flags \|= FLAG_ONLY_ALERT_ONCE;

		PostNotificationTracker tracker = mPostNotificationTrackerFactory.newTracker(null);
		tracker.addCleanupRunnable(() -> {
		synchronized (mNotificationLock) {
		// Mark that the notification has been updated due to cancelation, so it won't
		// be updated again if the app cancels multiple times.
		record.setCanceledAfterLifetimeExtension(true);
		// Set the post silently status to the record's previous value.
		record.setPostSilently(savedPostSilentlyState);
		// Remove FLAG_ONLY_ALERT_ONCE if the notification did not previously have it.
		if (!savedOnlyAlertOnceState) {
		record.getNotification().flags &= ~FLAG_ONLY_ALERT_ONCE;
		}
		}
		});

		mHandler.post(new EnqueueNotificationRunnable(record.getUser().getIdentifier(),
		record, isAppForeground, /* isAppProvided= */ false,
		mPostNotificationTrackerFactory.newTracker(null)));
		record, isAppForeground, /* isAppProvided= */ false, tracker));
		}
		}

		@@ -13351,17 +13376,23 @@ public class NotificationManagerService extends SystemService {
		@ElapsedRealtimeLong private final long mStartTime;
		@Nullable private final WakeLock mWakeLock;
		private boolean mOngoing;
		private final List<Runnable> mCleanupRunnables;

		@VisibleForTesting
		PostNotificationTracker(@Nullable WakeLock wakeLock) {
		mStartTime = SystemClock.elapsedRealtime();
		mWakeLock = wakeLock;
		mOngoing = true;
		mCleanupRunnables = new ArrayList<Runnable>();
		if (DBG) {
		Slog.d(TAG, "PostNotification: Started");
		}
		}

		void addCleanupRunnable(Runnable runnable) {
		mCleanupRunnables.add(runnable);
		}

		@ElapsedRealtimeLong
		long getStartTime() {
		return mStartTime;
		@@ -13373,8 +13404,9 @@ public class NotificationManagerService extends SystemService {
		}

		/**
		* Cancels the tracker (releasing the acquired WakeLock). Either {@link #finish} or
		* {@link #cancel} (exclusively) should be called on this object before it's discarded.
		* Cancels the tracker (releasing the acquired WakeLock) and runs any set cleanup runnables.
		* Either {@link #finish} or {@link #cancel} (exclusively) should be called on this object
		* before it's discarded.
		*/
		void cancel() {
		if (!isOngoing()) {
		@@ -13385,6 +13417,9 @@ public class NotificationManagerService extends SystemService {
		if (mWakeLock != null) {
		Binder.withCleanCallingIdentity(() -> mWakeLock.release());
		}
		for (Runnable r : mCleanupRunnables) {
		r.run();
		}
		if (DBG) {
		long elapsedTime = SystemClock.elapsedRealtime() - mStartTime;
		Slog.d(TAG, TextUtils.formatSimple("PostNotification: Abandoned after %d ms",
		@@ -13393,9 +13428,10 @@ public class NotificationManagerService extends SystemService {
		}

		/**
		* Finishes the tracker (releasing the acquired WakeLock) and returns the time elapsed since
		* the operation started, in milliseconds. Either {@link #finish} or {@link #cancel}
		* (exclusively) should be called on this object before it's discarded.
		* Finishes the tracker (releasing the acquired WakeLock), runs any set cleanup runnables,
		* and returns the time elapsed since the operation started, in milliseconds.
		* Either {@link #finish} or {@link #cancel} (exclusively) should be called on this object
		* before it's discarded.
		*/
		@DurationMillisLong
		long finish() {
		@@ -13408,6 +13444,9 @@ public class NotificationManagerService extends SystemService {
		if (mWakeLock != null) {
		Binder.withCleanCallingIdentity(() -> mWakeLock.release());
		}
		for (Runnable r : mCleanupRunnables) {
		r.run();
		}
		if (DBG) {
		Slog.d(TAG,
		TextUtils.formatSimple("PostNotification: Finished in %d ms", elapsedTime));

services/core/java/com/android/server/notification/NotificationRecord.java

+12 −0

Original line number	Diff line number	Diff line
		@@ -222,6 +222,9 @@ public final class NotificationRecord {
		private boolean mPendingLogUpdate = false;
		private int mProposedImportance = IMPORTANCE_UNSPECIFIED;
		private boolean mSensitiveContent = false;
		// Whether an app has attempted to cancel this notification after it has been marked as
		// lifetime extended.
		private boolean mCanceledAfterLifetimeExtension = false;

		public NotificationRecord(Context context, StatusBarNotification sbn,
		NotificationChannel channel) {
		@@ -535,6 +538,7 @@ public final class NotificationRecord {
		+ NotificationListenerService.Ranking.importanceToString(mProposedImportance));
		pw.println(prefix + "mIsAppImportanceLocked=" + mIsAppImportanceLocked);
		pw.println(prefix + "mSensitiveContent=" + mSensitiveContent);
		pw.println(prefix + "mCanceledAfterLifetimeExtension=" + mCanceledAfterLifetimeExtension);
		pw.println(prefix + "mIntercept=" + mIntercept);
		pw.println(prefix + "mHidden==" + mHidden);
		pw.println(prefix + "mGlobalSortKey=" + mGlobalSortKey);
		@@ -1620,6 +1624,14 @@ public final class NotificationRecord {
		mPkgAllowedAsConvo = allowedAsConvo;
		}

		public boolean isCanceledAfterLifetimeExtension() {
		return mCanceledAfterLifetimeExtension;
		}

		public void setCanceledAfterLifetimeExtension(boolean canceledAfterLifetimeExtension) {
		mCanceledAfterLifetimeExtension = canceledAfterLifetimeExtension;
		}

		/**
		* Whether this notification is a conversation notification.
		*/

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+133 −3

Original line number	Diff line number	Diff line
		@@ -3045,6 +3045,41 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY);
		}

		@Test
		@EnableFlags(android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR)
		public void testMultipleCancelOfLifetimeExtendedSendsOneUpdate() throws Exception {
		final NotificationRecord notif = generateNotificationRecord(null);
		notif.getSbn().getNotification().flags =
		Notification.FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY;
		mService.addNotification(notif);
		final StatusBarNotification sbn = notif.getSbn();

		assertThat(mBinderService.getActiveNotifications(sbn.getPackageName()).length).isEqualTo(1);
		assertThat(mService.getNotificationRecordCount()).isEqualTo(1);

		// Send two cancelations.
		mBinderService.cancelNotificationWithTag(mPkg, mPkg, sbn.getTag(), sbn.getId(),
		sbn.getUserId());
		waitForIdle();
		mBinderService.cancelNotificationWithTag(mPkg, mPkg, sbn.getTag(), sbn.getId(),
		sbn.getUserId());
		waitForIdle();

		assertThat(mBinderService.getActiveNotifications(sbn.getPackageName()).length).isEqualTo(1);
		assertThat(mService.getNotificationRecordCount()).isEqualTo(1);

		// Checks that only one post update is sent.
		verify(mWorkerHandler, times(1))
		.post(any(NotificationManagerService.PostNotificationRunnable.class));
		ArgumentCaptor<NotificationRecord> captor =
		ArgumentCaptor.forClass(NotificationRecord.class);
		verify(mListeners, times(1)).prepareNotifyPostedLocked(captor.capture(), any(),
		anyBoolean());
		assertThat(captor.getValue().getNotification().flags
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY).isEqualTo(
		FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY);
		}

		@Test
		@EnableFlags(android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR)
		public void testCancelAllClearsLifetimeExtended() throws Exception {
		@@ -6419,12 +6454,31 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		@EnableFlags(android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR)
		public void testStats_DirectReplyLifetimeExtendedPostsUpdate() throws Exception {
		final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
		// Marks the notification as having already been lifetime extended and canceled.
		r.getSbn().getNotification().flags \|= FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY;
		r.setCanceledAfterLifetimeExtension(true);
		r.setPostSilently(true);
		mService.addNotification(r);

		mService.mNotificationDelegate.onNotificationDirectReplied(r.getKey());
		waitForIdle();

		// At the moment prepareNotifyPostedLocked is called on the listeners,
		// verify that FLAG_ONLY_ALERT_ONCE and shouldPostSilently are set, regardless of initial
		// values.
		doAnswer(
		invocation -> {
		int flags = ((NotificationRecord) invocation.getArgument(0))
		.getSbn().getNotification().flags;
		assertThat(flags & FLAG_ONLY_ALERT_ONCE).isEqualTo(FLAG_ONLY_ALERT_ONCE);
		boolean shouldPostSilently = ((NotificationRecord) invocation.getArgument(0))
		.shouldPostSilently();
		assertThat(shouldPostSilently).isTrue();
		return null;
		}
		).when(mListeners).prepareNotifyPostedLocked(any(), any(), anyBoolean());

		// Checks that the record gets marked as a direct reply having occurred.
		assertThat(mService.getNotificationRecord(r.getKey()).getStats().hasDirectReplied())
		.isTrue();
		// Checks that a post update is sent.
		@@ -6437,9 +6491,65 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		assertThat(captor.getValue().getNotification().flags
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY).isEqualTo(
		FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY);
		// FLAG_ONLY_ALERT_ONCE was not present on the original notification, so it's not here.
		assertThat(captor.getValue().getNotification().flags
		& FLAG_ONLY_ALERT_ONCE).isEqualTo(FLAG_ONLY_ALERT_ONCE);
		& FLAG_ONLY_ALERT_ONCE).isEqualTo(0);
		assertThat(captor.getValue().shouldPostSilently()).isTrue();
		assertThat(captor.getValue().isCanceledAfterLifetimeExtension()).isTrue();
		}

		@Test
		@EnableFlags(android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR)
		public void testStats_DirectReplyLifetimeExtendedPostsUpdate_RestorePostSilently()
		throws Exception {
		final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
		// Marks the notification as having already been lifetime extended and canceled.
		r.getSbn().getNotification().flags \|= FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY;
		r.setPostSilently(false);
		mService.addNotification(r);

		mService.mNotificationDelegate.onNotificationDirectReplied(r.getKey());
		waitForIdle();

		// Checks that a post update is sent with shouldPostSilently set to true.
		doAnswer(
		invocation -> {
		boolean shouldPostSilently = ((NotificationRecord) invocation.getArgument(0))
		.shouldPostSilently();
		assertThat(shouldPostSilently).isTrue();
		return null;
		}
		).when(mListeners).prepareNotifyPostedLocked(any(), any(), anyBoolean());

		// Checks that shouldPostSilently is restored to its false state afterward.
		assertThat(mService.getNotificationRecord(r.getKey()).shouldPostSilently()).isFalse();
		}

		@Test
		@EnableFlags(android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR)
		public void testStats_DirectReplyLifetimeExtendedPostsUpdate_RestoreOnlyAlertOnceFlag()
		throws Exception {
		final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
		// Marks the notification as having already been lifetime extended and canceled.
		r.getSbn().getNotification().flags \|= FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY;
		mService.addNotification(r);

		mService.mNotificationDelegate.onNotificationDirectReplied(r.getKey());
		waitForIdle();

		// Checks that a post update is sent with FLAG_ONLY_ALERT_ONCE set to true.
		doAnswer(
		invocation -> {
		int flags = ((NotificationRecord) invocation.getArgument(0))
		.getSbn().getNotification().flags;
		assertThat(flags & FLAG_ONLY_ALERT_ONCE).isEqualTo(FLAG_ONLY_ALERT_ONCE);
		return null;
		}
		).when(mListeners).prepareNotifyPostedLocked(any(), any(), anyBoolean());

		// Checks that the flag is removed afterward.
		assertThat(mService.getNotificationRecord(r.getKey()).getSbn().getNotification().flags
		& FLAG_ONLY_ALERT_ONCE).isEqualTo(0);
		}

		@Test
		@@ -6476,6 +6586,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		anyBoolean());
		assertThat(captor.getValue().getNotification().flags
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY).isEqualTo(0);
		assertThat(captor.getValue().isCanceledAfterLifetimeExtension()).isFalse();
		assertThat(captor.getValue()
		.getNotification().extras.getCharSequence(Notification.EXTRA_TITLE).toString())
		.isEqualTo("new title");
		@@ -9143,11 +9254,13 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		final int replyIndex = 2;
		final String reply = "Hello";
		final boolean modifiedBeforeSending = true;
		final boolean generatedByAssistant = true;

		NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
		r.getSbn().getNotification().flags \|= FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY;
		r.setSuggestionsGeneratedByAssistant(generatedByAssistant);
		r.getSbn().getNotification().flags \|= FLAG_ONLY_ALERT_ONCE;
		r.setSuggestionsGeneratedByAssistant(true);
		r.setCanceledAfterLifetimeExtension(true);
		r.setPostSilently(true);
		mService.addNotification(r);

		mService.mNotificationDelegate.onNotificationSmartReplySent(
		@@ -9155,6 +9268,21 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		modifiedBeforeSending);
		waitForIdle();

		// At the moment prepareNotifyPostedLocked is called on the listeners,
		// verify that FLAG_ONLY_ALERT_ONCE and shouldPostSilently are set, regardless of initial
		// values.
		doAnswer(
		invocation -> {
		int flags = ((NotificationRecord) invocation.getArgument(0))
		.getSbn().getNotification().flags;
		assertThat(flags & FLAG_ONLY_ALERT_ONCE).isEqualTo(FLAG_ONLY_ALERT_ONCE);
		boolean shouldPostSilently = ((NotificationRecord) invocation.getArgument(0))
		.shouldPostSilently();
		assertThat(shouldPostSilently).isTrue();
		return null;
		}
		).when(mListeners).prepareNotifyPostedLocked(any(), any(), anyBoolean());

		// Checks that a post update is sent.
		verify(mWorkerHandler, times(1))
		.post(any(NotificationManagerService.PostNotificationRunnable.class));
		@@ -9165,8 +9293,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
		assertThat(captor.getValue().getNotification().flags
		& FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY).isEqualTo(
		FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY);
		// Flag was present before, so it's set afterward
		assertThat(captor.getValue().getNotification().flags
		& FLAG_ONLY_ALERT_ONCE).isEqualTo(FLAG_ONLY_ALERT_ONCE);
		// Should post silently was set before, so it's set afterward.
		assertThat(captor.getValue().shouldPostSilently()).isTrue();
		}