[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

3 files changed, 95 insertions, 20 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractorTest.kt
index 7ed2bd38bcd2..0b9b297130a2 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractorTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractorTest.kt
@@ -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
@@ -409,6 +411,63 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {
 
     @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() =
         kosmos.runTest {
             val latest by collectLastValue(underTest.shownNotificationChips)
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/SingleNotificationChipInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/SingleNotificationChipInteractor.kt
index d8c3e2546a8f..a0de879845d3 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/SingleNotificationChipInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/SingleNotificationChipInteractor.kt
@@ -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) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractor.kt
index edb44185459c..1ddecedeaafd 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/domain/interactor/StatusBarNotificationChipsInteractor.kt
@@ -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
@@ -147,28 +149,42 @@ 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...
-                    val allNotificationChips: List<Flow<NotificationChipModel?>> =
-                        interactors.map { interactor -> interactor.notificationChip }
-                    combine(allNotificationChips) {
+                // For all our current interactors...
+                promotedNotificationInteractors.flatMapLatest { interactors ->
+                    if (interactors.isNotEmpty()) {
+                        // Combine each interactor's [notificationChip] flow...
+                        val allNotificationChips: List<Flow<NotificationChipModel?>> =
+                            interactors.map { interactor -> interactor.notificationChip }
+                        combine(allNotificationChips) {
                             // ... and emit just the non-null & sorted chips
                             it.filterNotNull().sortedWith(chipComparator)
                         }
-                        .logSort()
-                } else {
-                    flowOf(emptyList())
+                    } else {
+                        flowOf(emptyList())
+                    }
                 }
+            } else {
+                flowOf(emptyList())
             }
-        } 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 }
         }
     }
 }