summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--services/core/java/com/android/server/notification/NotificationManagerService.java57
-rw-r--r--services/core/java/com/android/server/notification/NotificationRecord.java12
-rw-r--r--services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java136
3 files changed, 193 insertions, 12 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index ffb2bb6a4528..54bc6fbfd930 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -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));
diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java
index 1392003a13e7..e54124608a26 100644
--- a/services/core/java/com/android/server/notification/NotificationRecord.java
+++ b/services/core/java/com/android/server/notification/NotificationRecord.java
@@ -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.
*/
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 5a8de58c14ae..0a52238671cd 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -3047,6 +3047,41 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
@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 {
final NotificationRecord notif = generateNotificationRecord(
mTestNotificationChannel, 1, "group", true);
@@ -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();
}