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

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

[SB][Notif] Have notif chips be isHidden=true if app is visible.

Previously, if a notification's app was visible, we changed its model to
be OngoingActivityChipModel.Inactive. But there's 2 reasons I'd rather
model it as OngoingActivityChipModel.Active(isHidden=true):

1) For UiEvent metric logging, it's better to keep the chip as Active
   the entire time so that we don't log a "chip removed" event whenever
   the notification's app is open.

2) When we bring return animations to notification chips (b/202516970),
   we'll need to make this update anyway.

Bug: 202516970
Bug: 386821418
Fixes: 396636550
Bug: 364653005
Flag: com.android.systemui.status_bar_notification_chips

Test: Have status bar notification chip, then open app -> verify chip
still disappears. Close app -> verify chip still re-appears.
Test: atest StatusBarNotificationChipsInteractorTest
NotifChipsViewModelTest

Change-Id: I87af8943acc82989fb54c11660f222adcb49da0b
parent fcf9d086
Loading
Loading
Loading
Loading
+44 −37
Original line number Diff line number Diff line
@@ -56,9 +56,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @DisableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_flagOff_noNotifs() =
    fun allNotificationChips_flagOff_noNotifs() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(
                listOf(
@@ -75,9 +75,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_noNotifs_empty() =
    fun allNotificationChips_noNotifs_empty() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(emptyList())

@@ -87,9 +87,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {
    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    @DisableFlags(StatusBarConnectedDisplays.FLAG_NAME)
    fun shownNotificationChips_notifMissingStatusBarChipIconView_cdFlagOff_empty() =
    fun allNotificationChips_notifMissingStatusBarChipIconView_cdFlagOff_empty() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(
                listOf(
@@ -106,9 +106,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME, StatusBarConnectedDisplays.FLAG_NAME)
    fun shownNotificationChips_notifMissingStatusBarChipIconView_cdFlagOn_notEmpty() =
    fun allNotificationChips_notifMissingStatusBarChipIconView_cdFlagOn_notEmpty() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(
                listOf(
@@ -125,9 +125,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_onePromotedNotif_statusBarIconViewMatches() =
    fun allNotificationChips_onePromotedNotif_statusBarIconViewMatches() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val icon = mock<StatusBarIconView>()
            setNotifs(
@@ -147,9 +147,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_onlyForPromotedNotifs() =
    fun allNotificationChips_onlyForPromotedNotifs() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val firstIcon = mock<StatusBarIconView>()
            val secondIcon = mock<StatusBarIconView>()
@@ -182,11 +182,11 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_onlyForNotVisibleApps() =
    fun allNotificationChips_appVisibilityInfoCorrect() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = false

            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val uid = 433
            setNotifs(
@@ -195,27 +195,30 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {
                        key = "notif",
                        uid = uid,
                        statusBarChipIcon = mock<StatusBarIconView>(),
                        promotedContent = PromotedNotificationContentBuilder("notif1").build(),
                        promotedContent = PromotedNotificationContentBuilder("notif").build(),
                    )
                )
            )

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = false)
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isAppVisible).isFalse()

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = true)
            assertThat(latest).isEmpty()
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isAppVisible).isTrue()

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = false)
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isAppVisible).isFalse()
        }

    /** Regression test for b/388521980. */
    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_callNotifIsAlsoPromoted_callNotifExcluded() =
    fun allNotificationChips_callNotifIsAlsoPromoted_callNotifExcluded() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(
                listOf(
@@ -243,9 +246,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_notifUpdatesGoThrough() =
    fun allNotificationChips_notifUpdatesGoThrough() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val firstIcon = mock<StatusBarIconView>()
            val secondIcon = mock<StatusBarIconView>()
@@ -293,9 +296,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_promotedNotifDisappearsThenReappears() =
    fun allNotificationChips_promotedNotifDisappearsThenReappears() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            setNotifs(
                listOf(
@@ -335,9 +338,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_sortedByFirstAppearanceTime() =
    fun allNotificationChips_sortedByFirstAppearanceTime() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val firstIcon = mock<StatusBarIconView>()
            val secondIcon = mock<StatusBarIconView>()
@@ -411,9 +414,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_lastAppVisibleTimeMaintainedAcrossNotifAddsAndRemoves() =
    fun allNotificationChips_lastAppVisibleTimeMaintainedAcrossNotifAddsAndRemoves() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val notif1Info = NotifInfo("notif1", mock<StatusBarIconView>(), uid = 100)
            val notif2Info = NotifInfo("notif2", mock<StatusBarIconView>(), uid = 200)
@@ -466,9 +469,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_sortedByLastAppVisibleTime() =
    fun allNotificationChips_sortedByLastAppVisibleTime() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val notif1Info = NotifInfo("notif1", mock<StatusBarIconView>(), uid = 100)
            val notif2Info = NotifInfo("notif2", mock<StatusBarIconView>(), uid = 200)
@@ -496,15 +499,19 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {
            fakeSystemClock.advanceTime(1000)
            activityManagerRepository.fake.setIsAppVisible(notif2Info.uid, isAppVisible = true)

            // THEN notif2 is no longer shown
            assertThat(latest!!.map { it.key }).containsExactly("notif1").inOrder()
            // THEN notif2 is ranked above notif1 because it was more recently visible
            assertThat(latest!!.map { it.key }).containsExactly("notif2", "notif1").inOrder()
            assertThat(latest!![0].isAppVisible).isTrue() // notif2
            assertThat(latest!![1].isAppVisible).isFalse() // notif1

            // WHEN notif2's app is no longer visible
            fakeSystemClock.advanceTime(1000)
            activityManagerRepository.fake.setIsAppVisible(notif2Info.uid, isAppVisible = false)

            // THEN notif2 is ranked above notif1 because it was more recently visible
            // THEN notif2 is still ranked above notif1
            assertThat(latest!!.map { it.key }).containsExactly("notif2", "notif1").inOrder()
            assertThat(latest!![0].isAppVisible).isFalse() // notif2
            assertThat(latest!![1].isAppVisible).isFalse() // notif1

            // WHEN the app associated with notif1 becomes visible then un-visible
            fakeSystemClock.advanceTime(1000)
@@ -518,9 +525,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_newNotificationTakesPriorityOverLastAppVisible() =
    fun allNotificationChips_newNotificationTakesPriorityOverLastAppVisible() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val notif1Info = NotifInfo("notif1", mock<StatusBarIconView>(), uid = 100)
            val notif2Info = NotifInfo("notif2", mock<StatusBarIconView>(), uid = 200)
@@ -574,9 +581,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_fullSort() =
    fun allNotificationChips_fullSort() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val notif1Info = NotifInfo("notif1", mock<StatusBarIconView>(), uid = 100)
            val notif2Info = NotifInfo("notif2", mock<StatusBarIconView>(), uid = 200)
@@ -689,9 +696,9 @@ class StatusBarNotificationChipsInteractorTest : SysuiTestCase() {

    @Test
    @EnableFlags(StatusBarNotifChips.FLAG_NAME)
    fun shownNotificationChips_notifChangesKey() =
    fun allNotificationChips_notifChangesKey() =
        kosmos.runTest {
            val latest by collectLastValue(underTest.shownNotificationChips)
            val latest by collectLastValue(underTest.allNotificationChips)

            val firstIcon = mock<StatusBarIconView>()
            val secondIcon = mock<StatusBarIconView>()
+158 −0
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.Flags.FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY
import com.android.systemui.SysuiTestCase
import com.android.systemui.activity.data.repository.activityManagerRepository
import com.android.systemui.activity.data.repository.fake
import com.android.systemui.common.shared.model.ContentDescription.Companion.loadContentDescription
import com.android.systemui.keyguard.data.repository.FakeKeyguardTransitionRepository
import com.android.systemui.keyguard.data.repository.fakeKeyguardTransitionRepository
@@ -285,6 +287,84 @@ class NotifChipsViewModelTest : SysuiTestCase() {
            assertIsNotifKey(latest!![1], secondKey)
        }

    @Test
    fun chips_appStartsAsVisible_isHiddenTrue() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = true

            val latest by collectLastValue(underTest.chips)

            val uid = 433
            setNotifs(
                listOf(
                    activeNotificationModel(
                        key = "notif",
                        uid = uid,
                        statusBarChipIcon = createStatusBarIconViewOrNull(),
                        promotedContent = PromotedNotificationContentBuilder("notif").build(),
                    )
                )
            )

            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isHidden).isTrue()
        }

    @Test
    fun chips_appStartsAsNotVisible_isHiddenFalse() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = false

            val latest by collectLastValue(underTest.chips)

            val uid = 433
            setNotifs(
                listOf(
                    activeNotificationModel(
                        key = "notif",
                        uid = uid,
                        statusBarChipIcon = createStatusBarIconViewOrNull(),
                        promotedContent = PromotedNotificationContentBuilder("notif").build(),
                    )
                )
            )

            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isHidden).isFalse()
        }

    @Test
    fun chips_isHidden_changesBasedOnAppVisibility() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = false

            val latest by collectLastValue(underTest.chips)

            val uid = 433
            setNotifs(
                listOf(
                    activeNotificationModel(
                        key = "notif",
                        uid = uid,
                        statusBarChipIcon = createStatusBarIconViewOrNull(),
                        promotedContent = PromotedNotificationContentBuilder("notif").build(),
                    )
                )
            )

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = false)
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isHidden).isFalse()

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = true)
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isHidden).isTrue()

            activityManagerRepository.fake.setIsAppVisible(uid, isAppVisible = false)
            assertThat(latest).hasSize(1)
            assertThat(latest!![0].isHidden).isFalse()
        }

    @Test
    @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY)
    fun chips_hasShortCriticalText_usesTextInsteadOfTime() =
@@ -421,6 +501,42 @@ class NotifChipsViewModelTest : SysuiTestCase() {
                .isInstanceOf(OngoingActivityChipModel.Active.ShortTimeDelta::class.java)
        }

    @Test
    fun chips_basicTime_respectsIsAppVisible() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = false

            val latest by collectLastValue(underTest.chips)
            val currentTime = 3.minutes.inWholeMilliseconds
            fakeSystemClock.setCurrentTimeMillis(currentTime)

            val promotedContentBuilder =
                PromotedNotificationContentBuilder("notif").applyToShared {
                    this.time = When.Time(currentTime + 13.minutes.inWholeMilliseconds)
                }
            val uid = 3

            setNotifs(
                listOf(
                    activeNotificationModel(
                        key = "notif",
                        uid = 3,
                        statusBarChipIcon = createStatusBarIconViewOrNull(),
                        promotedContent = promotedContentBuilder.build(),
                    )
                )
            )

            assertThat(latest).hasSize(1)
            assertThat(latest!![0])
                .isInstanceOf(OngoingActivityChipModel.Active.ShortTimeDelta::class.java)
            assertThat(latest!![0].isHidden).isFalse()

            activityManagerRepository.fake.setIsAppVisible(uid = uid, isAppVisible = true)

            assertThat(latest!![0].isHidden).isTrue()
        }

    @Test
    @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY)
    fun chips_basicTime_timeLessThanOneMinInFuture_isIconOnly() =
@@ -577,6 +693,48 @@ class NotifChipsViewModelTest : SysuiTestCase() {
                .isFalse()
        }

    @Test
    @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY)
    fun chips_countUpTime_respectsIsAppVisible() =
        kosmos.runTest {
            activityManagerRepository.fake.startingIsAppVisibleValue = true

            val latest by collectLastValue(underTest.chips)
            val currentTime = 30.minutes.inWholeMilliseconds
            fakeSystemClock.setCurrentTimeMillis(currentTime)

            val currentElapsed =
                currentTime + fakeSystemClock.elapsedRealtime() -
                    fakeSystemClock.currentTimeMillis()

            val whenElapsed = currentElapsed - 1.minutes.inWholeMilliseconds

            val promotedContentBuilder =
                PromotedNotificationContentBuilder("notif").applyToShared {
                    this.time =
                        When.Chronometer(elapsedRealtimeMillis = whenElapsed, isCountDown = false)
                }
            val uid = 6
            setNotifs(
                listOf(
                    activeNotificationModel(
                        key = "notif",
                        uid = uid,
                        statusBarChipIcon = createStatusBarIconViewOrNull(),
                        promotedContent = promotedContentBuilder.build(),
                    )
                )
            )

            assertThat(latest).hasSize(1)
            assertThat(latest!![0]).isInstanceOf(OngoingActivityChipModel.Active.Timer::class.java)
            assertThat(latest!![0].isHidden).isTrue()

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

            assertThat(latest!![0].isHidden).isFalse()
        }

    @Test
    @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY)
    fun chips_countDownTime_isTimer() =
+3 −10
Original line number Diff line number Diff line
@@ -145,7 +145,9 @@ constructor(

    /**
     * Emits all notifications that are eligible to show as chips in the status bar. This is
     * different from which chips will *actually* show, see [shownNotificationChips] for that.
     * different from which chips will *actually* show, because
     * [com.android.systemui.statusbar.chips.notification.ui.viewmodel.NotifChipsViewModel] will
     * hide chips that have [NotificationChipModel.isAppVisible] as true.
     */
    val allNotificationChips: Flow<List<NotificationChipModel>> =
        if (StatusBarNotifChips.isEnabled) {
@@ -186,15 +188,6 @@ constructor(
                initialValue = emptyList(),
            )

    /** Emits the notifications that should actually be *shown* as chips in the status bar. */
    val shownNotificationChips: Flow<List<NotificationChipModel>> =
        allNotificationChips.map { chipsList ->
            // If the app that posted this notification is visible, we want to hide the chip
            // because information between the status bar chip and the app itself could be
            // out-of-sync (like a timer that's slightly off)
            chipsList.filter { !it.isAppVisible }
        }

    /*
    Stable sort the promoted notifications by two criteria:
    Criteria #1: Whichever app was most recently visible has higher ranking.
+14 −3
Original line number Diff line number Diff line
@@ -55,12 +55,12 @@ constructor(
    private val systemClock: SystemClock,
) {
    /**
     * A flow modeling the notification chips that should be shown. Emits an empty list if there are
     * no notifications that should show a status bar chip.
     * A flow modeling the current notification chips. Emits an empty list if there are no
     * notifications that are eligible to show a status bar chip.
     */
    val chips: Flow<List<OngoingActivityChipModel.Active>> =
        combine(
                notifChipsInteractor.shownNotificationChips,
                notifChipsInteractor.allNotificationChips,
                headsUpNotificationInteractor.statusBarHeadsUpState,
            ) { notifications, headsUpState ->
                notifications.map { it.toActivityChipModel(headsUpState) }
@@ -98,6 +98,10 @@ constructor(
                notifChipsInteractor.onPromotedNotificationChipTapped(this@toActivityChipModel.key)
            }
        }
        // If the app that posted this notification is visible, we want to hide the chip
        // because information between the status bar chip and the app itself could be
        // out-of-sync (like a timer that's slightly off)
        val isHidden = this.isAppVisible
        val onClickListenerLegacy =
            View.OnClickListener {
                StatusBarChipsModernization.assertInLegacyMode()
@@ -122,6 +126,7 @@ constructor(
                colors = colors,
                onClickListenerLegacy = onClickListenerLegacy,
                clickBehavior = clickBehavior,
                isHidden = isHidden,
            )
        }

@@ -133,6 +138,7 @@ constructor(
                text = chipContent.shortCriticalText,
                onClickListenerLegacy = onClickListenerLegacy,
                clickBehavior = clickBehavior,
                isHidden = isHidden,
            )
        }

@@ -147,6 +153,7 @@ constructor(
                colors = colors,
                onClickListenerLegacy = onClickListenerLegacy,
                clickBehavior = clickBehavior,
                isHidden = isHidden,
            )
        }

@@ -157,6 +164,7 @@ constructor(
                colors = colors,
                onClickListenerLegacy = onClickListenerLegacy,
                clickBehavior = clickBehavior,
                isHidden = isHidden,
            )
        }

@@ -173,6 +181,7 @@ constructor(
                        time = chipContent.time.currentTimeMillis,
                        onClickListenerLegacy = onClickListenerLegacy,
                        clickBehavior = clickBehavior,
                        isHidden = isHidden,
                    )
                } else {
                    // Don't show a `when` time that's close to now or in the past because it's
@@ -189,6 +198,7 @@ constructor(
                        colors = colors,
                        onClickListenerLegacy = onClickListenerLegacy,
                        clickBehavior = clickBehavior,
                        isHidden = isHidden,
                    )
                }
            }
@@ -201,6 +211,7 @@ constructor(
                    isEventInFuture = chipContent.time.isCountDown,
                    onClickListenerLegacy = onClickListenerLegacy,
                    clickBehavior = clickBehavior,
                    isHidden = isHidden,
                )
            }
        }