From 21ba4bf2dc07093b717b843e5e6f7abd610a2ab9 Mon Sep 17 00:00:00 2001 From: Julia Tuttle Date: Wed, 12 Oct 2022 00:28:25 -0400 Subject: [PATCH] Suppress HUNs for notifications with old whens Right now, if a user turns their device (or work profile) on after it's been off for a while, they can get flooded with visual interruptions (like heads-up notifications) from 'stale' notifications -- recently posted, but pertaining to something that happened over a day ago. This change will, when the included feature flag is enabled, suppress visual interruption for notifications with a 'when' field set more than 24 hours in the past. Bug: 240552967 Bug: 253055301 Test: atest StatusBarNotificationPresenterTest Change-Id: I6e8b29cd9865370fb5ad34d4b74e0f8a40e63229 --- .../notification/NotifPipelineFlags.kt | 4 + .../NotificationInterruptLogger.kt | 30 ++++ ...otificationInterruptStateProviderImpl.java | 57 +++++++- ...icationInterruptStateProviderImplTest.java | 134 ++++++++++++++++++ 4 files changed, 224 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt index 7eb890677206e..39daa13ae168d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt @@ -44,4 +44,8 @@ class NotifPipelineFlags @Inject constructor( val shouldFilterUnseenNotifsOnKeyguard: Boolean by lazy { featureFlags.isEnabled(Flags.FILTER_UNSEEN_NOTIFS_ON_KEYGUARD) } + + val isNoHunForOldWhenEnabled: Boolean by lazy { + featureFlags.isEnabled(Flags.NO_HUN_FOR_OLD_WHEN) + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt index 073b6b041b814..13b3aca2a88fb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt @@ -106,6 +106,36 @@ class NotificationInterruptLogger @Inject constructor( }) } + fun logNoHeadsUpOldWhen( + entry: NotificationEntry, + notifWhen: Long, + notifAge: Long + ) { + buffer.log(TAG, DEBUG, { + str1 = entry.logKey + long1 = notifWhen + long2 = notifAge + }, { + "No heads up: old when $long1 (age=$long2 ms): $str1" + }) + } + + fun logMaybeHeadsUpDespiteOldWhen( + entry: NotificationEntry, + notifWhen: Long, + notifAge: Long, + reason: String + ) { + buffer.log(TAG, DEBUG, { + str1 = entry.logKey + str2 = reason + long1 = notifWhen + long2 = notifAge + }, { + "Maybe heads up: old when $long1 (age=$long2 ms) but $str2: $str1" + }) + } + fun logNoHeadsUpSuppressedBy( entry: NotificationEntry, suppressor: NotificationInterruptSuppressor diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java index c4f5a3a30608d..ec5bd68942833 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java @@ -20,6 +20,7 @@ import static com.android.systemui.statusbar.StatusBarState.SHADE; import static com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProviderImpl.NotificationInterruptEvent.FSI_SUPPRESSED_NO_HUN_OR_KEYGUARD; import static com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProviderImpl.NotificationInterruptEvent.FSI_SUPPRESSED_SUPPRESSIVE_GROUP_ALERT_BEHAVIOR; +import android.app.Notification; import android.app.NotificationManager; import android.content.ContentResolver; import android.database.ContentObserver; @@ -82,7 +83,10 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter FSI_SUPPRESSED_SUPPRESSIVE_GROUP_ALERT_BEHAVIOR(1235), @UiEvent(doc = "FSI suppressed for requiring neither HUN nor keyguard") - FSI_SUPPRESSED_NO_HUN_OR_KEYGUARD(1236); + FSI_SUPPRESSED_NO_HUN_OR_KEYGUARD(1236), + + @UiEvent(doc = "HUN suppressed for old when") + HUN_SUPPRESSED_OLD_WHEN(1237); private final int mId; @@ -346,6 +350,10 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter return false; } + if (shouldSuppressHeadsUpWhenAwakeForOldWhen(entry, log)) { + return false; + } + for (int i = 0; i < mSuppressors.size(); i++) { if (mSuppressors.get(i).suppressAwakeHeadsUp(entry)) { if (log) mLogger.logNoHeadsUpSuppressedBy(entry, mSuppressors.get(i)); @@ -470,4 +478,51 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter private boolean isSnoozedPackage(StatusBarNotification sbn) { return mHeadsUpManager.isSnoozed(sbn.getPackageName()); } + + private boolean shouldSuppressHeadsUpWhenAwakeForOldWhen(NotificationEntry entry, boolean log) { + if (!mFlags.isNoHunForOldWhenEnabled()) { + return false; + } + + final Notification notification = entry.getSbn().getNotification(); + if (notification == null) { + return false; + } + + final long when = notification.when; + final long now = System.currentTimeMillis(); + final long age = now - when; + + if (age < MAX_HUN_WHEN_AGE_MS) { + return false; + } + + if (when <= 0) { + // Some notifications (including many system notifications) are posted with the "when" + // field set to 0. Nothing in the Javadocs for Notification mentions a special meaning + // for a "when" of 0, but Android didn't even exist at the dawn of the Unix epoch. + // Therefore, assume that these notifications effectively don't have a "when" value, + // and don't suppress HUNs. + if (log) mLogger.logMaybeHeadsUpDespiteOldWhen(entry, when, age, "when <= 0"); + return false; + } + + if (notification.fullScreenIntent != null) { + if (log) mLogger.logMaybeHeadsUpDespiteOldWhen(entry, when, age, "full-screen intent"); + return false; + } + + if (notification.isForegroundService()) { + if (log) mLogger.logMaybeHeadsUpDespiteOldWhen(entry, when, age, "foreground service"); + return false; + } + + if (log) mLogger.logNoHeadsUpOldWhen(entry, when, age); + final int uid = entry.getSbn().getUid(); + final String packageName = entry.getSbn().getPackageName(); + mUiEventLogger.log(NotificationInterruptEvent.HUN_SUPPRESSED_OLD_WHEN, uid, packageName); + return true; + } + + public static final long MAX_HUN_WHEN_AGE_MS = 24 * 60 * 60 * 1000; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java index ea311da3e20b8..21aae00f12ba5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java @@ -17,6 +17,7 @@ package com.android.systemui.statusbar.notification.interruption; import static android.app.Notification.FLAG_BUBBLE; +import static android.app.Notification.FLAG_FOREGROUND_SERVICE; import static android.app.Notification.GROUP_ALERT_SUMMARY; import static android.app.NotificationManager.IMPORTANCE_DEFAULT; import static android.app.NotificationManager.IMPORTANCE_HIGH; @@ -33,6 +34,8 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -390,6 +393,127 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isFalse(); } + private long makeWhenHoursAgo(long hoursAgo) { + return System.currentTimeMillis() - (1000 * 60 * 60 * hoursAgo); + } + + @Test + public void testShouldHeadsUp_oldWhen_flagDisabled() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(false); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = makeWhenHoursAgo(25); + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger, never()).logMaybeHeadsUpDespiteOldWhen(any(), anyLong(), anyLong(), any()); + } + + @Test + public void testShouldHeadsUp_oldWhen_whenNow() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger, never()).logMaybeHeadsUpDespiteOldWhen(any(), anyLong(), anyLong(), any()); + } + + @Test + public void testShouldHeadsUp_oldWhen_whenRecent() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = makeWhenHoursAgo(13); + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger, never()).logMaybeHeadsUpDespiteOldWhen(any(), anyLong(), anyLong(), any()); + } + + @Test + public void testShouldHeadsUp_oldWhen_whenZero() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = 0L; + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger).logMaybeHeadsUpDespiteOldWhen(eq(entry), eq(0L), anyLong(), + eq("when <= 0")); + } + + @Test + public void testShouldHeadsUp_oldWhen_whenNegative() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = -1L; + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger).logMaybeHeadsUpDespiteOldWhen(eq(entry), eq(-1L), anyLong(), + eq("when <= 0")); + } + + @Test + public void testShouldHeadsUp_oldWhen_hasFullScreenIntent() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + long when = makeWhenHoursAgo(25); + + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silent= */ false); + entry.getSbn().getNotification().when = when; + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger).logMaybeHeadsUpDespiteOldWhen(eq(entry), eq(when), anyLong(), + eq("full-screen intent")); + } + + @Test + public void testShouldHeadsUp_oldWhen_isForegroundService() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + long when = makeWhenHoursAgo(25); + + NotificationEntry entry = createFgsNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = when; + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isTrue(); + + verify(mLogger, never()).logNoHeadsUpOldWhen(any(), anyLong(), anyLong()); + verify(mLogger).logMaybeHeadsUpDespiteOldWhen(eq(entry), eq(when), anyLong(), + eq("foreground service")); + } + + @Test + public void testShouldNotHeadsUp_oldWhen() throws Exception { + ensureStateForHeadsUpWhenAwake(); + when(mFlags.isNoHunForOldWhenEnabled()).thenReturn(true); + long when = makeWhenHoursAgo(25); + + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + entry.getSbn().getNotification().when = when; + + assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isFalse(); + + verify(mLogger).logNoHeadsUpOldWhen(eq(entry), eq(when), anyLong()); + verify(mLogger, never()).logMaybeHeadsUpDespiteOldWhen(any(), anyLong(), anyLong(), any()); + } + @Test public void testShouldNotFullScreen_notPendingIntent_withStrictFlag() throws Exception { when(mFlags.fullScreenIntentRequiresKeyguard()).thenReturn(true); @@ -763,6 +887,16 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { return createNotification(importance, n); } + private NotificationEntry createFgsNotification(int importance) { + Notification n = new Notification.Builder(getContext(), "a") + .setContentTitle("title") + .setContentText("content text") + .setFlag(FLAG_FOREGROUND_SERVICE, true) + .build(); + + return createNotification(importance, n); + } + private final NotificationInterruptSuppressor mSuppressAwakeHeadsUp = new NotificationInterruptSuppressor() {