diff options
4 files changed, 280 insertions, 35 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index ce9a1fc2778e..bf01ca5ad5db 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -90,7 +90,8 @@ object Flags { // TODO(b/257315550): Tracking Bug val NO_HUN_FOR_OLD_WHEN = unreleasedFlag(118, "no_hun_for_old_when") - // next id: 119 + val FILTER_UNSEEN_NOTIFS_ON_KEYGUARD = + unreleasedFlag(254647461, "filter_unseen_notifs_on_keyguard", teamfood = true) // 200 - keyguard/lockscreen // ** Flag retired ** 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 2734511de78c..7eb890677206 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt @@ -40,4 +40,8 @@ class NotifPipelineFlags @Inject constructor( val isSemiStableSortEnabled: Boolean by lazy { featureFlags.isEnabled(Flags.SEMI_STABLE_SORT) } + + val shouldFilterUnseenNotifsOnKeyguard: Boolean by lazy { + featureFlags.isEnabled(Flags.FILTER_UNSEEN_NOTIFS_ON_KEYGUARD) + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.kt index 7e69fde3e6e9..6e5fcebf780f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.kt @@ -13,42 +13,117 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package com.android.systemui.statusbar.notification.collection.coordinator -import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope -import com.android.systemui.statusbar.notification.interruption.KeyguardNotificationVisibilityProvider -import com.android.systemui.statusbar.notification.collection.provider.SectionHeaderVisibilityProvider +import androidx.annotation.VisibleForTesting +import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.keyguard.data.repository.KeyguardRepository import com.android.systemui.plugins.statusbar.StatusBarStateController import com.android.systemui.statusbar.StatusBarState +import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.NotifPipeline -import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener +import com.android.systemui.statusbar.notification.collection.provider.SectionHeaderVisibilityProvider +import com.android.systemui.statusbar.notification.interruption.KeyguardNotificationVisibilityProvider import javax.inject.Inject +import kotlin.time.Duration.Companion.seconds +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.launch /** * Filters low priority and privacy-sensitive notifications from the lockscreen, and hides section * headers on the lockscreen. */ @CoordinatorScope -class KeyguardCoordinator @Inject constructor( +class KeyguardCoordinator +@Inject +constructor( private val keyguardNotificationVisibilityProvider: KeyguardNotificationVisibilityProvider, + private val keyguardRepository: KeyguardRepository, + private val notifPipelineFlags: NotifPipelineFlags, + @Application private val scope: CoroutineScope, private val sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider, - private val statusBarStateController: StatusBarStateController + private val statusBarStateController: StatusBarStateController, ) : Coordinator { + private val unseenNotifications = mutableSetOf<NotificationEntry>() + override fun attach(pipeline: NotifPipeline) { setupInvalidateNotifListCallbacks() // Filter at the "finalize" stage so that views remain bound by PreparationCoordinator pipeline.addFinalizeFilter(notifFilter) keyguardNotificationVisibilityProvider.addOnStateChangedListener(::invalidateListFromFilter) updateSectionHeadersVisibility() + if (notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard) { + attachUnseenFilter(pipeline) + } + } + + private fun attachUnseenFilter(pipeline: NotifPipeline) { + pipeline.addFinalizeFilter(unseenNotifFilter) + pipeline.addCollectionListener(collectionListener) + scope.launch { clearUnseenWhenKeyguardIsDismissed() } } - private val notifFilter: NotifFilter = object : NotifFilter(TAG) { - override fun shouldFilterOut(entry: NotificationEntry, now: Long): Boolean = - keyguardNotificationVisibilityProvider.shouldHideNotification(entry) + private suspend fun clearUnseenWhenKeyguardIsDismissed() { + // Use collectLatest so that the suspending block is cancelled if isKeyguardShowing changes + // during the timeout period + keyguardRepository.isKeyguardShowing.collectLatest { isKeyguardShowing -> + if (!isKeyguardShowing) { + unseenNotifFilter.invalidateList("keyguard no longer showing") + delay(SEEN_TIMEOUT) + unseenNotifications.clear() + } + } } + private val collectionListener = + object : NotifCollectionListener { + override fun onEntryAdded(entry: NotificationEntry) { + if (keyguardRepository.isKeyguardShowing()) { + unseenNotifications.add(entry) + } + } + + override fun onEntryUpdated(entry: NotificationEntry) { + if (keyguardRepository.isKeyguardShowing()) { + unseenNotifications.add(entry) + } + } + + override fun onEntryRemoved(entry: NotificationEntry, reason: Int) { + unseenNotifications.remove(entry) + } + } + + @VisibleForTesting + internal val unseenNotifFilter = + object : NotifFilter("$TAG-unseen") { + override fun shouldFilterOut(entry: NotificationEntry, now: Long): Boolean = + when { + // Don't apply filter if the keyguard isn't currently showing + !keyguardRepository.isKeyguardShowing() -> false + // Don't apply the filter if the notification is unseen + unseenNotifications.contains(entry) -> false + // Don't apply the filter to (non-promoted) group summaries + // - summary will be pruned if necessary, depending on if children are filtered + entry.parent?.summary == entry -> false + else -> true + } + } + + private val notifFilter: NotifFilter = + object : NotifFilter(TAG) { + override fun shouldFilterOut(entry: NotificationEntry, now: Long): Boolean = + keyguardNotificationVisibilityProvider.shouldHideNotification(entry) + } + // TODO(b/206118999): merge this class with SensitiveContentCoordinator which also depends on // these same updates private fun setupInvalidateNotifListCallbacks() {} @@ -67,5 +142,6 @@ class KeyguardCoordinator @Inject constructor( companion object { private const val TAG = "KeyguardCoordinator" + private val SEEN_TIMEOUT = 5.seconds } -}
\ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt index 7e2e6f619946..bdedd244abfa 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt @@ -13,57 +13,55 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@file:OptIn(ExperimentalCoroutinesApi::class) + package com.android.systemui.statusbar.notification.collection.coordinator import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase +import com.android.systemui.keyguard.data.repository.FakeKeyguardRepository import com.android.systemui.plugins.statusbar.StatusBarStateController import com.android.systemui.statusbar.StatusBarState +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.NotificationEntryBuilder import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener import com.android.systemui.statusbar.notification.collection.provider.SectionHeaderVisibilityProvider import com.android.systemui.statusbar.notification.interruption.KeyguardNotificationVisibilityProvider import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.withArgCaptor -import java.util.function.Consumer -import org.junit.Before +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.clearInvocations import org.mockito.Mockito.verify +import java.util.function.Consumer +import kotlin.time.Duration.Companion.seconds import org.mockito.Mockito.`when` as whenever @SmallTest @RunWith(AndroidTestingRunner::class) class KeyguardCoordinatorTest : SysuiTestCase() { - private val notifPipeline: NotifPipeline = mock() + private val keyguardNotifVisibilityProvider: KeyguardNotificationVisibilityProvider = mock() + private val keyguardRepository = FakeKeyguardRepository() + private val notifPipelineFlags: NotifPipelineFlags = mock() + private val notifPipeline: NotifPipeline = mock() private val sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider = mock() private val statusBarStateController: StatusBarStateController = mock() - private lateinit var onStateChangeListener: Consumer<String> - private lateinit var keyguardFilter: NotifFilter - - @Before - fun setup() { - val keyguardCoordinator = KeyguardCoordinator( - keyguardNotifVisibilityProvider, - sectionHeaderVisibilityProvider, - statusBarStateController - ) - keyguardCoordinator.attach(notifPipeline) - onStateChangeListener = withArgCaptor { - verify(keyguardNotifVisibilityProvider).addOnStateChangedListener(capture()) - } - keyguardFilter = withArgCaptor { - verify(notifPipeline).addFinalizeFilter(capture()) - } - } - @Test - fun testSetSectionHeadersVisibleInShade() { + fun testSetSectionHeadersVisibleInShade() = runKeyguardCoordinatorTest { clearInvocations(sectionHeaderVisibilityProvider) whenever(statusBarStateController.state).thenReturn(StatusBarState.SHADE) onStateChangeListener.accept("state change") @@ -71,10 +69,176 @@ class KeyguardCoordinatorTest : SysuiTestCase() { } @Test - fun testSetSectionHeadersNotVisibleOnKeyguard() { + fun testSetSectionHeadersNotVisibleOnKeyguard() = runKeyguardCoordinatorTest { clearInvocations(sectionHeaderVisibilityProvider) whenever(statusBarStateController.state).thenReturn(StatusBarState.KEYGUARD) onStateChangeListener.accept("state change") verify(sectionHeaderVisibilityProvider).sectionHeadersVisible = eq(false) } + + @Test + fun unseenFilterSuppressesSeenNotifWhileKeyguardShowing() { + whenever(notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard).thenReturn(true) + + // GIVEN: Keyguard is not showing, and a notification is present + keyguardRepository.setKeyguardShowing(false) + runKeyguardCoordinatorTest { + val fakeEntry = NotificationEntryBuilder().build() + collectionListener.onEntryAdded(fakeEntry) + + // WHEN: The keyguard is now showing + keyguardRepository.setKeyguardShowing(true) + testScheduler.runCurrent() + + // THEN: The notification is recognized as "seen" and is filtered out. + assertThat(unseenFilter.shouldFilterOut(fakeEntry, 0L)).isTrue() + + // WHEN: The keyguard goes away + keyguardRepository.setKeyguardShowing(false) + testScheduler.runCurrent() + + // THEN: The notification is shown regardless + assertThat(unseenFilter.shouldFilterOut(fakeEntry, 0L)).isFalse() + } + } + + @Test + fun unseenFilterAllowsNewNotif() { + whenever(notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard).thenReturn(true) + + // GIVEN: Keyguard is showing, no notifications present + keyguardRepository.setKeyguardShowing(true) + runKeyguardCoordinatorTest { + // WHEN: A new notification is posted + val fakeEntry = NotificationEntryBuilder().build() + collectionListener.onEntryAdded(fakeEntry) + + // THEN: The notification is recognized as "unseen" and is not filtered out. + assertThat(unseenFilter.shouldFilterOut(fakeEntry, 0L)).isFalse() + } + } + + @Test + fun unseenFilterSeenGroupSummaryWithUnseenChild() { + whenever(notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard).thenReturn(true) + + // GIVEN: Keyguard is not showing, and a notification is present + keyguardRepository.setKeyguardShowing(false) + runKeyguardCoordinatorTest { + // WHEN: A new notification is posted + val fakeSummary = NotificationEntryBuilder().build() + val fakeChild = NotificationEntryBuilder() + .setGroup(context, "group") + .setGroupSummary(context, false) + .build() + GroupEntryBuilder() + .setSummary(fakeSummary) + .addChild(fakeChild) + .build() + + collectionListener.onEntryAdded(fakeSummary) + collectionListener.onEntryAdded(fakeChild) + + // WHEN: Keyguard is now showing, both notifications are marked as seen + keyguardRepository.setKeyguardShowing(true) + testScheduler.runCurrent() + + // WHEN: The child notification is now unseen + collectionListener.onEntryUpdated(fakeChild) + + // THEN: The summary is not filtered out, because the child is unseen + assertThat(unseenFilter.shouldFilterOut(fakeSummary, 0L)).isFalse() + } + } + + @Test + fun unseenNotificationIsMarkedAsSeenWhenKeyguardGoesAway() { + whenever(notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard).thenReturn(true) + + // GIVEN: Keyguard is showing, unseen notification is present + keyguardRepository.setKeyguardShowing(true) + runKeyguardCoordinatorTest { + val fakeEntry = NotificationEntryBuilder().build() + collectionListener.onEntryAdded(fakeEntry) + + // WHEN: Keyguard is no longer showing for 5 seconds + keyguardRepository.setKeyguardShowing(false) + testScheduler.runCurrent() + testScheduler.advanceTimeBy(5.seconds.inWholeMilliseconds) + testScheduler.runCurrent() + + // WHEN: Keyguard is shown again + keyguardRepository.setKeyguardShowing(true) + testScheduler.runCurrent() + + // THEN: The notification is now recognized as "seen" and is filtered out. + assertThat(unseenFilter.shouldFilterOut(fakeEntry, 0L)).isTrue() + } + } + + @Test + fun unseenNotificationIsNotMarkedAsSeenIfTimeThresholdNotMet() { + whenever(notifPipelineFlags.shouldFilterUnseenNotifsOnKeyguard).thenReturn(true) + + // GIVEN: Keyguard is showing, unseen notification is present + keyguardRepository.setKeyguardShowing(true) + runKeyguardCoordinatorTest { + val fakeEntry = NotificationEntryBuilder().build() + collectionListener.onEntryAdded(fakeEntry) + + // WHEN: Keyguard is no longer showing for <5 seconds + keyguardRepository.setKeyguardShowing(false) + testScheduler.runCurrent() + testScheduler.advanceTimeBy(1.seconds.inWholeMilliseconds) + + // WHEN: Keyguard is shown again + keyguardRepository.setKeyguardShowing(true) + testScheduler.runCurrent() + + // THEN: The notification is not recognized as "seen" and is not filtered out. + assertThat(unseenFilter.shouldFilterOut(fakeEntry, 0L)).isFalse() + } + } + + private fun runKeyguardCoordinatorTest( + testBlock: suspend KeyguardCoordinatorTestScope.() -> Unit + ) { + val testScope = TestScope(UnconfinedTestDispatcher()) + val keyguardCoordinator = + KeyguardCoordinator( + keyguardNotifVisibilityProvider, + keyguardRepository, + notifPipelineFlags, + testScope.backgroundScope, + sectionHeaderVisibilityProvider, + statusBarStateController, + ) + keyguardCoordinator.attach(notifPipeline) + KeyguardCoordinatorTestScope(keyguardCoordinator, testScope).run { + testScheduler.advanceUntilIdle() + testScope.runTest(dispatchTimeoutMs = 1.seconds.inWholeMilliseconds) { testBlock() } + } + } + + private inner class KeyguardCoordinatorTestScope( + private val keyguardCoordinator: KeyguardCoordinator, + private val scope: TestScope, + ) : CoroutineScope by scope { + val testScheduler: TestCoroutineScheduler + get() = scope.testScheduler + + val onStateChangeListener: Consumer<String> = + withArgCaptor { + verify(keyguardNotifVisibilityProvider).addOnStateChangedListener(capture()) + } + + val unseenFilter: NotifFilter + get() = keyguardCoordinator.unseenNotifFilter + + // TODO(254647461): Remove lazy once Flags.FILTER_UNSEEN_NOTIFS_ON_KEYGUARD is enabled and + // removed + val collectionListener: NotifCollectionListener by lazy { + withArgCaptor { verify(notifPipeline).addCollectionListener(capture()) } + } + } } |