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

Commit f127a153 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[SB][Notif] Maintain notification chip state for a timeout period. 

The `flatMapLatest` that collects all the individual notification chip
flows means that the flows stop & immediately restart collection each
time the list of notification chips changes. This can cause problems,
like the `appVisibility` flow starting UID observation multiple times.

This CL uses a `stopTimeoutMillis` on the notification chips StateFlow
so that all the flow values are maintained for the brief period where
flatMapLatest stops then restarts collection.

Fixes: 396471436
Bug: 364653005
Flag: com.android.systemui.status_bar_notification_chips

Test: trigger a bunch of different notification chips, opening & closing
their apps -> dump `StatusBarChips` log and verify it's not spammy

Change-Id: Idcf4777c896ac45af5d157409f2560942384369e
parent aa680d7d

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractorTest.kt

+59 −0
Original line number Diff line number Diff line
@@ -34,6 +34,8 @@ import com.android.systemui.statusbar.core.StatusBarConnectedDisplays 
import com.android.systemui.statusbar.notification.data.model.activeNotificationModel

import com.android.systemui.statusbar.notification.data.repository.ActiveNotificationsStore

import com.android.systemui.statusbar.notification.data.repository.activeNotificationListRepository

import com.android.systemui.statusbar.notification.data.repository.addNotif

import com.android.systemui.statusbar.notification.data.repository.removeNotif

import com.android.systemui.statusbar.notification.promoted.shared.model.PromotedNotificationContentModel

import com.android.systemui.statusbar.notification.shared.ActiveNotificationModel

import com.android.systemui.statusbar.notification.shared.CallType
@@ -407,6 +409,63 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() { 
            assertThat(latest!!.map { it.key }).containsExactly("notif1", "notif2").inOrder()

        }



    @Test

    @EnableFlags(StatusBarNotifChips.FLAG_NAME)

    fun shownNotificationChips_lastAppVisibleTimeMaintainedAcrossNotifAddsAndRemoves() =

        kosmos.runTest {

            val latest by collectLastValue(underTest.shownNotificationChips)



            val notif1Info = NotifInfo("notif1", mock<StatusBarIconView>(), uid = 100)

            val notif2Info = NotifInfo("notif2", mock<StatusBarIconView>(), uid = 200)



            // Have notif1's app start as showing and then hide later so we get the chip

            activityManagerRepository.fake.startingIsAppVisibleValue = true

            fakeSystemClock.setCurrentTimeMillis(9_000)

            activeNotificationListRepository.addNotif(

                activeNotificationModel(

                    key = notif1Info.key,

                    uid = notif1Info.uid,

                    statusBarChipIcon = notif1Info.icon,

                    promotedContent =

                        PromotedNotificationContentModel.Builder(notif1Info.key).build(),

                )

            )

            activityManagerRepository.fake.setIsAppVisible(notif1Info.uid, isAppVisible = false)



            assertThat(latest!![0].key).isEqualTo(notif1Info.key)

            assertThat(latest!![0].lastAppVisibleTime).isEqualTo(9_000)



            // WHEN a new notification is added

            activityManagerRepository.fake.startingIsAppVisibleValue = true

            fakeSystemClock.setCurrentTimeMillis(10_000)

            activeNotificationListRepository.addNotif(

                activeNotificationModel(

                    key = notif2Info.key,

                    uid = notif2Info.uid,

                    statusBarChipIcon = notif2Info.icon,

                    promotedContent =

                        PromotedNotificationContentModel.Builder(notif2Info.key).build(),

                )

            )

            activityManagerRepository.fake.setIsAppVisible(notif2Info.uid, isAppVisible = false)



            // THEN the new notification is first

            assertThat(latest!![0].key).isEqualTo(notif2Info.key)

            assertThat(latest!![0].lastAppVisibleTime).isEqualTo(10_000)



            // And THEN the original notification maintains its lastAppVisibleTime

            assertThat(latest!![1].key).isEqualTo(notif1Info.key)

            assertThat(latest!![1].lastAppVisibleTime).isEqualTo(9_000)



            // WHEN notif1 is removed

            fakeSystemClock.setCurrentTimeMillis(11_000)

            activeNotificationListRepository.removeNotif(notif1Info.key)



            // THEN notif2 still has its lastAppVisibleTime

            assertThat(latest!![0].key).isEqualTo(notif2Info.key)

            assertThat(latest!![0].lastAppVisibleTime).isEqualTo(10_000)

        }



    @Test

    @EnableFlags(StatusBarNotifChips.FLAG_NAME)

    fun shownNotificationChips_sortedByLastAppVisibleTime() =

packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/SingleNotificationChipInteractor.kt

+1 −1
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ constructor( 
    private val logger = Logger(logBuffer, "Notif".pad())

    // [StatusBarChipLogTag] recommends a max tag length of 20, so [extraLogTag] should NOT be the

    // top-level tag. It should instead be provided as the first string in each log message.

    private val extraLogTag = "SingleChipInteractor[key=$key]"

    private val extraLogTag = "SingleNotifChipInteractor[key=$key][id=${hashCode()}]"



    init {

        if (startingModel.promotedContent == null) {

packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractor.kt

+35 −19
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import kotlinx.coroutines.flow.Flow 
import kotlinx.coroutines.flow.MutableSharedFlow

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.SharedFlow

import kotlinx.coroutines.flow.SharingStarted

import kotlinx.coroutines.flow.asSharedFlow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.distinctUntilChanged
@@ -45,6 +46,7 @@ import kotlinx.coroutines.flow.flatMapLatest 
import kotlinx.coroutines.flow.flowOf

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.onEach

import kotlinx.coroutines.flow.stateIn



/** An interactor for the notification chips shown in the status bar. */

@SysUISingleton
@@ -148,10 +150,6 @@ constructor( 
    val allNotificationChips: Flow<List<NotificationChipModel>> =

        if (StatusBarNotifChips.isEnabled) {

                // For all our current interactors...

            // TODO(b/364653005): When a promoted notification is added or removed, each individual

            // interactor's [notificationChip] flow becomes un-collected then re-collected, which

            // can cause some flows to remove then add callbacks when they don't need to. Is there a

            // better structure for this? Maybe Channels or a StateFlow with a short timeout?

                promotedNotificationInteractors.flatMapLatest { interactors ->

                    if (interactors.isNotEmpty()) {

                        // Combine each interactor's [notificationChip] flow...
@@ -161,7 +159,6 @@ constructor( 
                            // ... and emit just the non-null & sorted chips

                            it.filterNotNull().sortedWith(chipComparator)

                        }

                        .logSort()

                    } else {

                        flowOf(emptyList())

                    }
@@ -169,6 +166,25 @@ constructor( 
            } else {

                flowOf(emptyList())

            }

            .distinctUntilChanged()

            .logSort()

            .stateIn(

                backgroundScope,

                SharingStarted.WhileSubscribed(

                    // When a promoted notification is added or removed, the `.flatMapLatest` above

                    // will stop collection and then re-start collection on each individual

                    // interactor's flow. (It will happen even for a chip that didn't change.) We

                    // don't want the individual interactor flows to stop then re-start because they

                    // may be maintaining values that would get thrown away when collection stops

                    // (like an app's last visible time).

                    // stopTimeoutMillis ensures we maintain those values even during the brief

                    // moment (1-2ms) when `.flatMapLatest` causes collect to stop then immediately

                    // restart.

                    // Note: Channels could also work to solve this.

                    stopTimeoutMillis = 1000

                ),

                initialValue = emptyList(),

            )



    /** Emits the notifications that should actually be *shown* as chips in the status bar. */

    val shownNotificationChips: Flow<List<NotificationChipModel>> =
@@ -199,14 +215,14 @@ constructor( 
        }



    private fun Flow<List<NotificationChipModel>>.logSort(): Flow<List<NotificationChipModel>> {

        return this.distinctUntilChanged().onEach { chips ->

        return this.onEach { chips ->

            val logString =

                chips.joinToString {

                    "{key=${it.key}. " +

                        "lastVisibleAppTime=${it.lastAppVisibleTime}. " +

                        "creationTime=${it.creationTime}}"

                }

            logger.d({ "Sorted chips: $str1" }) { str1 = logString }

            logger.d({ "Sorted notif chips: $str1" }) { str1 = logString }

        }

    }

}