Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dd643ef9 authored by Yuri Lin's avatar Yuri Lin
Browse files

Add a timeout for alerting HUNs on ranking update

Without a timeout, there's a potential issue where (for example) coming out of DND causes everything that would've alerted in the interim to alert at once, and that's not desirable behavior.

Bug: 248325248
Test: manual, HeadsUpCoordinatorTest
Change-Id: I872959be88328feb08878289d0e6dd852195912f
parent a122bd8f
Loading
Loading
Loading
Loading
+57 −1
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ package com.android.systemui.statusbar.notification.collection.coordinator
import android.app.Notification
import android.app.Notification.GROUP_ALERT_SUMMARY
import android.util.ArrayMap
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.collection.GroupEntry
@@ -70,6 +72,7 @@ class HeadsUpCoordinator @Inject constructor(
    @Main private val mExecutor: DelayableExecutor,
) : Coordinator {
    private val mEntriesBindingUntil = ArrayMap<String, Long>()
    private val mEntriesUpdateTimes = ArrayMap<String, Long>()
    private var mEndLifetimeExtension: OnEndLifetimeExtensionCallback? = null
    private lateinit var mNotifPipeline: NotifPipeline
    private var mNow: Long = -1
@@ -264,6 +267,9 @@ class HeadsUpCoordinator @Inject constructor(
        }
        // After this method runs, all posted entries should have been handled (or skipped).
        mPostedEntries.clear()

        // Also take this opportunity to clean up any stale entry update times
        cleanUpEntryUpdateTimes()
    }

    /**
@@ -378,6 +384,9 @@ class HeadsUpCoordinator @Inject constructor(
                isAlerting = false,
                isBinding = false,
            )

            // Record the last updated time for this key
            setUpdateTime(entry, mSystemClock.currentTimeMillis())
        }

        /**
@@ -419,6 +428,9 @@ class HeadsUpCoordinator @Inject constructor(
                    cancelHeadsUpBind(posted.entry)
                }
            }

            // Update last updated time for this entry
            setUpdateTime(entry, mSystemClock.currentTimeMillis())
        }

        /**
@@ -426,6 +438,7 @@ class HeadsUpCoordinator @Inject constructor(
         */
        override fun onEntryRemoved(entry: NotificationEntry, reason: Int) {
            mPostedEntries.remove(entry.key)
            mEntriesUpdateTimes.remove(entry.key)
            cancelHeadsUpBind(entry)
            val entryKey = entry.key
            if (mHeadsUpManager.isAlerting(entryKey)) {
@@ -454,7 +467,12 @@ class HeadsUpCoordinator @Inject constructor(
            // never) in mPostedEntries to need to alert, we need to check every notification
            // known to the pipeline.
            for (entry in mNotifPipeline.allNotifs) {
                // The only entries we can consider alerting for here are entries that have never
                // Only consider entries that are recent enough, since we want to apply a fairly
                // strict threshold for when an entry should be updated via only ranking and not an
                // app-provided notification update.
                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
                // explicit notification update.
@@ -486,6 +504,41 @@ class HeadsUpCoordinator @Inject constructor(
                (entry.sbn.notification.flags and Notification.FLAG_ONLY_ALERT_ONCE) == 0)
    }

    /**
     * Sets the updated time for the given entry to the specified time.
     */
    @VisibleForTesting
    fun setUpdateTime(entry: NotificationEntry, time: Long) {
        mEntriesUpdateTimes[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
     * only reacting to a ranking change, as relevant ranking updates are expected to come in
     * fairly soon after the posting of a notification.
     */
    private fun isNewEnoughForRankingUpdate(entry: NotificationEntry): Boolean {
        // If we don't have an update time for this key, default to "too old"
        if (!mEntriesUpdateTimes.containsKey(entry.key)) return false

        val updateTime = mEntriesUpdateTimes[entry.key] ?: return false
        return (mSystemClock.currentTimeMillis() - updateTime) <= MAX_RANKING_UPDATE_DELAY_MS
    }

    private fun cleanUpEntryUpdateTimes() {
        // 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.
        val toRemove = ArraySet<String>()
        for ((key, updateTime) in mEntriesUpdateTimes) {
            if (updateTime == null ||
                    (mSystemClock.currentTimeMillis() - updateTime) > MAX_RANKING_UPDATE_DELAY_MS) {
                toRemove.add(key)
            }
        }
        mEntriesUpdateTimes.removeAll(toRemove)
    }

    /** When an action is pressed on a notification, end HeadsUp lifetime extension. */
    private val mActionPressListener = Consumer<NotificationEntry> { entry ->
        if (mNotifsExtendingLifetime.contains(entry)) {
@@ -597,6 +650,9 @@ class HeadsUpCoordinator @Inject constructor(
    companion object {
        private const val TAG = "HeadsUpCoordinator"
        private const val BIND_TIMEOUT = 1000L

        // This value is set to match MAX_SOUND_DELAY_MS in NotificationRecord.
        private const val MAX_RANKING_UPDATE_DELAY_MS: Long = 2000
    }

    data class PostedEntry(
+25 −1
Original line number Diff line number Diff line
@@ -674,7 +674,9 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
    @Test
    fun testOnRankingApplied_newEntryShouldAlert() {
        // GIVEN that mEntry has never interrupted in the past, and now should
        // and is new enough to do so
        assertFalse(mEntry.hasInterrupted())
        mCoordinator.setUpdateTime(mEntry, mSystemClock.currentTimeMillis())
        setShouldHeadsUp(mEntry)
        whenever(mNotifPipeline.allNotifs).thenReturn(listOf(mEntry))

@@ -690,8 +692,9 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {

    @Test
    fun testOnRankingApplied_alreadyAlertedEntryShouldNotAlertAgain() {
        // GIVEN that mEntry has alerted in the past
        // GIVEN that mEntry has alerted in the past, even if it's new
        mEntry.setInterruption()
        mCoordinator.setUpdateTime(mEntry, mSystemClock.currentTimeMillis())
        setShouldHeadsUp(mEntry)
        whenever(mNotifPipeline.allNotifs).thenReturn(listOf(mEntry))

@@ -725,6 +728,27 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
        verify(mHeadsUpManager).showNotification(mEntry)
    }

    @Test
    fun testOnRankingApplied_entryUpdatedButTooOld() {
        // GIVEN that mEntry is added in a state where it should not HUN
        setShouldHeadsUp(mEntry, false)
        mCollectionListener.onEntryAdded(mEntry)

        // and it was actually added 10s ago
        mCoordinator.setUpdateTime(mEntry, mSystemClock.currentTimeMillis() - 10000)

        // WHEN it is updated to HUN and then a ranking update occurs
        setShouldHeadsUp(mEntry)
        whenever(mNotifPipeline.allNotifs).thenReturn(listOf(mEntry))
        mCollectionListener.onRankingApplied()
        mBeforeTransformGroupsListener.onBeforeTransformGroups(listOf(mEntry))
        mBeforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(mEntry))

        // THEN the notification is never bound or shown
        verify(mHeadsUpViewBinder, never()).bindHeadsUpView(any(), any())
        verify(mHeadsUpManager, never()).showNotification(any())
    }

    private fun setShouldHeadsUp(entry: NotificationEntry, should: Boolean = true) {
        whenever(mNotificationInterruptStateProvider.shouldHeadsUp(entry)).thenReturn(should)
        whenever(mNotificationInterruptStateProvider.checkHeadsUp(eq(entry), any()))