From 4b48ec4489daf60cad7df86a336cfce61e6e8494 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Mon, 28 Nov 2022 14:10:45 -0500 Subject: [PATCH] Send FSIs on ranking update when they are no longer suppressed by DND. This change allows notifications with full screen intents to show that FSI on a ranking update if they meet specific criteria (previously suppressed only by DND; not older than a few seconds; now no longer suppressed). FSI on update only happens when the flag FSI_ON_DND_UPDATE is set to true. Bug: 248325248 Test: HeadsUpCoordinatorTest, manual verifying the FSI actually shows up on update Change-Id: I810ae951bc96908cc66a8831f88730021bdb64b6 --- .../notification/NotifPipelineFlags.kt | 2 + .../coordinator/HeadsUpCoordinator.kt | 78 +++++++++++++-- .../coordinator/HeadsUpCoordinatorLogger.kt | 8 ++ .../coordinator/HeadsUpCoordinatorTest.kt | 95 ++++++++++++++++++- 4 files changed, 170 insertions(+), 13 deletions(-) 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..8c55c138eea2e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt @@ -33,6 +33,8 @@ class NotifPipelineFlags @Inject constructor( fun fullScreenIntentRequiresKeyguard(): Boolean = featureFlags.isEnabled(Flags.FSI_REQUIRES_KEYGUARD) + fun fsiOnDNDUpdate(): Boolean = featureFlags.isEnabled(Flags.FSI_ON_DND_UPDATE) + val isStabilityIndexFixEnabled: Boolean by lazy { featureFlags.isEnabled(Flags.STABILITY_INDEX_FIX) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt index 5dbb4f9d70dfb..1004ec195493b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt @@ -22,6 +22,7 @@ import android.util.ArraySet import com.android.internal.annotations.VisibleForTesting import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.statusbar.NotificationRemoteInputManager +import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.ListEntry import com.android.systemui.statusbar.notification.collection.NotifPipeline @@ -38,6 +39,7 @@ import com.android.systemui.statusbar.notification.collection.render.NodeControl import com.android.systemui.statusbar.notification.dagger.IncomingHeader import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinder import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider +import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider.FullScreenIntentDecision import com.android.systemui.statusbar.notification.logKey import com.android.systemui.statusbar.notification.stack.BUCKET_HEADS_UP import com.android.systemui.statusbar.policy.HeadsUpManager @@ -70,11 +72,13 @@ class HeadsUpCoordinator @Inject constructor( private val mNotificationInterruptStateProvider: NotificationInterruptStateProvider, private val mRemoteInputManager: NotificationRemoteInputManager, private val mLaunchFullScreenIntentProvider: LaunchFullScreenIntentProvider, + private val mFlags: NotifPipelineFlags, @IncomingHeader private val mIncomingHeaderController: NodeController, @Main private val mExecutor: DelayableExecutor, ) : Coordinator { private val mEntriesBindingUntil = ArrayMap() private val mEntriesUpdateTimes = ArrayMap() + private val mFSIUpdateCandidates = ArrayMap() private var mEndLifetimeExtension: OnEndLifetimeExtensionCallback? = null private lateinit var mNotifPipeline: NotifPipeline private var mNow: Long = -1 @@ -278,7 +282,7 @@ class HeadsUpCoordinator @Inject constructor( mPostedEntries.clear() // Also take this opportunity to clean up any stale entry update times - cleanUpEntryUpdateTimes() + cleanUpEntryTimes() } /** @@ -384,8 +388,15 @@ class HeadsUpCoordinator @Inject constructor( override fun onEntryAdded(entry: NotificationEntry) { // First check whether this notification should launch a full screen intent, and // launch it if needed. - if (mNotificationInterruptStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) { + val fsiDecision = mNotificationInterruptStateProvider.getFullScreenIntentDecision(entry) + if (fsiDecision != null && fsiDecision.shouldLaunch) { + mNotificationInterruptStateProvider.logFullScreenIntentDecision(entry, fsiDecision) mLaunchFullScreenIntentProvider.launchFullScreenIntent(entry) + } else if (mFlags.fsiOnDNDUpdate() && + fsiDecision.equals(FullScreenIntentDecision.NO_FSI_SUPPRESSED_ONLY_BY_DND)) { + // If DND was the only reason this entry was suppressed, note it for potential + // reconsideration on later ranking updates. + addForFSIReconsideration(entry, mSystemClock.currentTimeMillis()) } // shouldHeadsUp includes check for whether this notification should be filtered @@ -488,11 +499,32 @@ class HeadsUpCoordinator @Inject constructor( if (!isNewEnoughForRankingUpdate(entry)) continue // The only entries we consider alerting for here are entries that have never - // interrupted and that now say they should heads up; if they've alerted in the - // past, we don't want to incorrectly alert a second time if there wasn't an + // interrupted and that now say they should heads up or FSI; if they've alerted in + // the past, we don't want to incorrectly alert a second time if there wasn't an // explicit notification update. if (entry.hasInterrupted()) continue + // Before potentially allowing heads-up, check for any candidates for a FSI launch. + // Any entry that is a candidate meets two criteria: + // - was suppressed from FSI launch only by a DND suppression + // - is within the recency window for reconsideration + // If any of these entries are no longer suppressed, launch the FSI now. + if (mFlags.fsiOnDNDUpdate() && isCandidateForFSIReconsideration(entry)) { + val decision = + mNotificationInterruptStateProvider.getFullScreenIntentDecision(entry) + if (decision.shouldLaunch) { + // Log both the launch of the full screen and also that this was via a + // ranking update. + mLogger.logEntryUpdatedToFullScreen(entry.key) + mNotificationInterruptStateProvider.logFullScreenIntentDecision( + entry, decision) + mLaunchFullScreenIntentProvider.launchFullScreenIntent(entry) + + // if we launch the FSI then this is no longer a candidate for HUN + continue + } + } + // The cases where we should consider this notification to be updated: // - if this entry is not present in PostedEntries, and is now in a shouldHeadsUp // state @@ -527,6 +559,15 @@ class HeadsUpCoordinator @Inject constructor( mEntriesUpdateTimes[entry.key] = time } + /** + * Add the entry to the list of entries potentially considerable for FSI ranking update, where + * the provided time is the time the entry was added. + */ + @VisibleForTesting + fun addForFSIReconsideration(entry: NotificationEntry, time: Long) { + mFSIUpdateCandidates[entry.key] = time + } + /** * Checks whether the entry is new enough to be updated via ranking update. * We want to avoid updating an entry too long after it was originally posted/updated when we're @@ -541,17 +582,38 @@ class HeadsUpCoordinator @Inject constructor( return (mSystemClock.currentTimeMillis() - updateTime) <= MAX_RANKING_UPDATE_DELAY_MS } - private fun cleanUpEntryUpdateTimes() { + /** + * Checks whether the entry is present new enough for reconsideration for full screen launch. + * The time window is the same as for ranking update, but this doesn't allow a potential update + * to an entry with full screen intent to count for timing purposes. + */ + private fun isCandidateForFSIReconsideration(entry: NotificationEntry): Boolean { + val addedTime = mFSIUpdateCandidates[entry.key] ?: return false + return (mSystemClock.currentTimeMillis() - addedTime) <= MAX_RANKING_UPDATE_DELAY_MS + } + + private fun cleanUpEntryTimes() { // Because we won't update entries that are older than this amount of time anyway, clean - // up any entries that are too old to notify. + // up any entries that are too old to notify from both the general and FSI specific lists. + + // Anything newer than this time is still within the window. + val timeThreshold = mSystemClock.currentTimeMillis() - MAX_RANKING_UPDATE_DELAY_MS + val toRemove = ArraySet() for ((key, updateTime) in mEntriesUpdateTimes) { - if (updateTime == null || - (mSystemClock.currentTimeMillis() - updateTime) > MAX_RANKING_UPDATE_DELAY_MS) { + if (updateTime == null || timeThreshold > updateTime) { toRemove.add(key) } } mEntriesUpdateTimes.removeAll(toRemove) + + val toRemoveForFSI = ArraySet() + for ((key, addedTime) in mFSIUpdateCandidates) { + if (addedTime == null || timeThreshold > addedTime) { + toRemoveForFSI.add(key) + } + } + mFSIUpdateCandidates.removeAll(toRemoveForFSI) } /** When an action is pressed on a notification, end HeadsUp lifetime extension. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorLogger.kt index 473c35d6095a5..2c6bf6b51451f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorLogger.kt @@ -70,6 +70,14 @@ class HeadsUpCoordinatorLogger constructor( }) } + fun logEntryUpdatedToFullScreen(key: String) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = key + }, { + "updating entry to launch full screen intent: $str1" + }) + } + fun logSummaryMarkedInterrupted(summaryKey: String, childKey: String) { buffer.log(TAG, LogLevel.DEBUG, { str1 = summaryKey diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt index aa1114b8736e1..cb4f119dce0a2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt @@ -23,6 +23,7 @@ import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.dump.logcatLogBuffer import com.android.systemui.statusbar.NotificationRemoteInputManager +import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.NotificationEntry @@ -38,6 +39,7 @@ import com.android.systemui.statusbar.notification.collection.provider.LaunchFul import com.android.systemui.statusbar.notification.collection.render.NodeController import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinder import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider +import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider.FullScreenIntentDecision import com.android.systemui.statusbar.notification.row.NotifBindPipeline.BindCallback import com.android.systemui.statusbar.phone.NotificationGroupTestHelper import com.android.systemui.statusbar.policy.HeadsUpManager @@ -88,6 +90,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { private val mEndLifetimeExtension: OnEndLifetimeExtensionCallback = mock() private val mHeaderController: NodeController = mock() private val mLaunchFullScreenIntentProvider: LaunchFullScreenIntentProvider = mock() + private val mFlags: NotifPipelineFlags = mock() private lateinit var mEntry: NotificationEntry private lateinit var mGroupSummary: NotificationEntry @@ -113,6 +116,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { mNotificationInterruptStateProvider, mRemoteInputManager, mLaunchFullScreenIntentProvider, + mFlags, mHeaderController, mExecutor) mCoordinator.attach(mNotifPipeline) @@ -246,14 +250,14 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { @Test fun testOnEntryAdded_shouldFullScreen() { - setShouldFullScreen(mEntry) + setShouldFullScreen(mEntry, FullScreenIntentDecision.FSI_EXPECTED_NOT_TO_HUN) mCollectionListener.onEntryAdded(mEntry) verify(mLaunchFullScreenIntentProvider).launchFullScreenIntent(mEntry) } @Test fun testOnEntryAdded_shouldNotFullScreen() { - setShouldFullScreen(mEntry, should = false) + setShouldFullScreen(mEntry, FullScreenIntentDecision.NO_FULL_SCREEN_INTENT) mCollectionListener.onEntryAdded(mEntry) verify(mLaunchFullScreenIntentProvider, never()).launchFullScreenIntent(any()) } @@ -805,15 +809,96 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { verify(mHeadsUpManager, never()).showNotification(any()) } + @Test + fun testOnRankingApplied_noFSIOnUpdateWhenFlagOff() { + // Ensure the feature flag is off + whenever(mFlags.fsiOnDNDUpdate()).thenReturn(false) + + // GIVEN that mEntry was previously suppressed from full-screen only by DND + setShouldFullScreen(mEntry, FullScreenIntentDecision.NO_FSI_SUPPRESSED_ONLY_BY_DND) + mCollectionListener.onEntryAdded(mEntry) + + // and it is then updated to allow full screen + setShouldFullScreen(mEntry, FullScreenIntentDecision.FSI_DEVICE_NOT_INTERACTIVE) + whenever(mNotifPipeline.allNotifs).thenReturn(listOf(mEntry)) + mCollectionListener.onRankingApplied() + + // THEN it should not full screen because the feature is off + verify(mLaunchFullScreenIntentProvider, never()).launchFullScreenIntent(mEntry) + } + + @Test + fun testOnRankingApplied_updateToFullScreen() { + // Turn on the feature + whenever(mFlags.fsiOnDNDUpdate()).thenReturn(true) + + // GIVEN that mEntry was previously suppressed from full-screen only by DND + setShouldFullScreen(mEntry, FullScreenIntentDecision.NO_FSI_SUPPRESSED_ONLY_BY_DND) + mCollectionListener.onEntryAdded(mEntry) + + // at this point, it should not have full screened + verify(mLaunchFullScreenIntentProvider, never()).launchFullScreenIntent(mEntry) + + // and it is then updated to allow full screen AND HUN + setShouldFullScreen(mEntry, FullScreenIntentDecision.FSI_DEVICE_NOT_INTERACTIVE) + setShouldHeadsUp(mEntry) + whenever(mNotifPipeline.allNotifs).thenReturn(listOf(mEntry)) + mCollectionListener.onRankingApplied() + mBeforeTransformGroupsListener.onBeforeTransformGroups(listOf(mEntry)) + mBeforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(mEntry)) + + // THEN it should full screen but it should NOT HUN + verify(mLaunchFullScreenIntentProvider).launchFullScreenIntent(mEntry) + verify(mHeadsUpViewBinder, never()).bindHeadsUpView(any(), any()) + verify(mHeadsUpManager, never()).showNotification(any()) + } + + @Test + fun testOnRankingApplied_noFSIWhenAlsoSuppressedForOtherReasons() { + // Feature on + whenever(mFlags.fsiOnDNDUpdate()).thenReturn(true) + + // GIVEN that mEntry is suppressed by DND (functionally), but not *only* DND + setShouldFullScreen(mEntry, FullScreenIntentDecision.NO_FSI_SUPPRESSED_BY_DND) + mCollectionListener.onEntryAdded(mEntry) + + // and it is updated to full screen later + setShouldFullScreen(mEntry, FullScreenIntentDecision.FSI_DEVICE_NOT_INTERACTIVE) + mCollectionListener.onRankingApplied() + + // THEN it should still not full screen because something else was blocking it before + verify(mLaunchFullScreenIntentProvider, never()).launchFullScreenIntent(mEntry) + } + + @Test + fun testOnRankingApplied_noFSIWhenTooOld() { + // Feature on + whenever(mFlags.fsiOnDNDUpdate()).thenReturn(true) + + // GIVEN that mEntry is suppressed only by DND + setShouldFullScreen(mEntry, FullScreenIntentDecision.NO_FSI_SUPPRESSED_ONLY_BY_DND) + mCollectionListener.onEntryAdded(mEntry) + + // but it's >10s old + mCoordinator.addForFSIReconsideration(mEntry, mSystemClock.currentTimeMillis() - 10000) + + // and it is updated to full screen later + setShouldFullScreen(mEntry, FullScreenIntentDecision.FSI_EXPECTED_NOT_TO_HUN) + mCollectionListener.onRankingApplied() + + // THEN it should still not full screen because it's too old + verify(mLaunchFullScreenIntentProvider, never()).launchFullScreenIntent(mEntry) + } + private fun setShouldHeadsUp(entry: NotificationEntry, should: Boolean = true) { whenever(mNotificationInterruptStateProvider.shouldHeadsUp(entry)).thenReturn(should) whenever(mNotificationInterruptStateProvider.checkHeadsUp(eq(entry), any())) .thenReturn(should) } - private fun setShouldFullScreen(entry: NotificationEntry, should: Boolean = true) { - whenever(mNotificationInterruptStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) - .thenReturn(should) + private fun setShouldFullScreen(entry: NotificationEntry, decision: FullScreenIntentDecision) { + whenever(mNotificationInterruptStateProvider.getFullScreenIntentDecision(entry)) + .thenReturn(decision) } private fun finishBind(entry: NotificationEntry) {