From 35ee1a7b2f6ea72ce9bf209a6e1b06781ff2a1f9 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Fri, 14 Jul 2023 10:41:15 -0700 Subject: [PATCH] [flexiglass] Auto-confirm, isPatternVisible are "refreshing flow" The upstream of the changed flows (LockPatternUtils) doesn't offer a way to listen for changes to these configs which means that stale/old values are cached forever in the flows. This CL changes the flows to special "refreshing flows" which (1) provide an initial value (2) refresh in the background (3) refresh each time the selected user changes or when a new downstream subscriber is added. Included there are some minor changes to switch from SharingStarted.Eagerly to SharingStarted.WhileSubscribed (so the UI coming to the foreground counts as a new subscriber each time) and a bug fix to PatternBouncer to make it not draw the lines if isPatternVisible is false. Bug: 280883900 Test: unit tests all pass still Test: manually verified that the bouncer correctly switches between autoconfirm enabled and autoconfirm disabled when I change it in the settings. Same for pattern visible. Change-Id: Ia8a3fca84437d2ddea18ebf1e4ad7ac8fe1c4f31 --- .../bouncer/ui/composable/PatternBouncer.kt | 70 +++++------ .../repository/AuthenticationRepository.kt | 85 ++++++++----- .../interactor/AuthenticationInteractor.kt | 4 +- .../ui/viewmodel/PinBouncerViewModel.kt | 4 +- .../AuthenticationRepositoryTest.kt | 113 ++++++++++++++++++ 5 files changed, 210 insertions(+), 66 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/authentication/data/repository/AuthenticationRepositoryTest.kt diff --git a/packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/PatternBouncer.kt b/packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/PatternBouncer.kt index b3d2e350ed504..b6b2488af4ca5 100644 --- a/packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/PatternBouncer.kt +++ b/packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/PatternBouncer.kt @@ -228,43 +228,45 @@ internal fun PatternBouncer( } } ) { - // Draw lines between dots. - selectedDots.forEachIndexed { index, dot -> - if (index > 0) { - val previousDot = selectedDots[index - 1] - val lineFadeOutAnimationProgress = lineFadeOutAnimatables[previousDot]!!.value - val startLerp = 1 - lineFadeOutAnimationProgress - val from = pixelOffset(previousDot, spacing, verticalOffset) - val to = pixelOffset(dot, spacing, verticalOffset) - val lerpedFrom = - Offset( - x = from.x + (to.x - from.x) * startLerp, - y = from.y + (to.y - from.y) * startLerp, + if (isAnimationEnabled) { + // Draw lines between dots. + selectedDots.forEachIndexed { index, dot -> + if (index > 0) { + val previousDot = selectedDots[index - 1] + val lineFadeOutAnimationProgress = lineFadeOutAnimatables[previousDot]!!.value + val startLerp = 1 - lineFadeOutAnimationProgress + val from = pixelOffset(previousDot, spacing, verticalOffset) + val to = pixelOffset(dot, spacing, verticalOffset) + val lerpedFrom = + Offset( + x = from.x + (to.x - from.x) * startLerp, + y = from.y + (to.y - from.y) * startLerp, + ) + drawLine( + start = lerpedFrom, + end = to, + cap = StrokeCap.Round, + alpha = lineFadeOutAnimationProgress * lineAlpha(spacing), + color = lineColor, + strokeWidth = lineStrokeWidth, ) - drawLine( - start = lerpedFrom, - end = to, - cap = StrokeCap.Round, - alpha = lineFadeOutAnimationProgress * lineAlpha(spacing), - color = lineColor, - strokeWidth = lineStrokeWidth, - ) + } } - } - // Draw the line between the most recently-selected dot and the input pointer position. - inputPosition?.let { lineEnd -> - currentDot?.let { dot -> - val from = pixelOffset(dot, spacing, verticalOffset) - val lineLength = sqrt((from.y - lineEnd.y).pow(2) + (from.x - lineEnd.x).pow(2)) - drawLine( - start = from, - end = lineEnd, - cap = StrokeCap.Round, - alpha = lineAlpha(spacing, lineLength), - color = lineColor, - strokeWidth = lineStrokeWidth, - ) + // Draw the line between the most recently-selected dot and the input pointer position. + inputPosition?.let { lineEnd -> + currentDot?.let { dot -> + val from = pixelOffset(dot, spacing, verticalOffset) + val lineLength = sqrt((from.y - lineEnd.y).pow(2) + (from.x - lineEnd.x).pow(2)) + drawLine( + start = from, + end = lineEnd, + cap = StrokeCap.Round, + alpha = lineAlpha(spacing, lineLength), + color = lineColor, + strokeWidth = lineStrokeWidth, + ) + } } } diff --git a/packages/SystemUI/src/com/android/systemui/authentication/data/repository/AuthenticationRepository.kt b/packages/SystemUI/src/com/android/systemui/authentication/data/repository/AuthenticationRepository.kt index a9779663cc7c5..deb3d035d7532 100644 --- a/packages/SystemUI/src/com/android/systemui/authentication/data/repository/AuthenticationRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/authentication/data/repository/AuthenticationRepository.kt @@ -14,8 +14,6 @@ * limitations under the License. */ -@file:OptIn(ExperimentalCoroutinesApi::class) - package com.android.systemui.authentication.data.repository import com.android.internal.widget.LockPatternChecker @@ -29,6 +27,7 @@ import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.keyguard.data.repository.KeyguardRepository import com.android.systemui.user.data.repository.UserRepository +import com.android.systemui.util.kotlin.pairwise import com.android.systemui.util.time.SystemClock import dagger.Binds import dagger.Module @@ -38,16 +37,14 @@ import kotlin.coroutines.resume import kotlin.coroutines.suspendCoroutine import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.launch import kotlinx.coroutines.withContext /** Defines interface for classes that can access authentication-related application state. */ @@ -156,32 +153,18 @@ constructor( } override val isAutoConfirmEnabled: StateFlow = - userRepository.selectedUserInfo - .map { it.id } - .flatMapLatest { userId -> - flow { emit(lockPatternUtils.isAutoPinConfirmEnabled(userId)) } - .flowOn(backgroundDispatcher) - } - .stateIn( - scope = applicationScope, - started = SharingStarted.WhileSubscribed(), - initialValue = false, - ) + refreshingFlow( + initialValue = false, + getFreshValue = lockPatternUtils::isAutoPinConfirmEnabled, + ) override val hintedPinLength: Int = 6 override val isPatternVisible: StateFlow = - userRepository.selectedUserInfo - .map { it.id } - .flatMapLatest { userId -> - flow { emit(lockPatternUtils.isVisiblePatternEnabled(userId)) } - .flowOn(backgroundDispatcher) - } - .stateIn( - scope = applicationScope, - started = SharingStarted.WhileSubscribed(), - initialValue = true, - ) + refreshingFlow( + initialValue = true, + getFreshValue = lockPatternUtils::isVisiblePatternEnabled, + ) private val _throttling = MutableStateFlow(AuthenticationThrottlingModel()) override val throttling: StateFlow = _throttling.asStateFlow() @@ -276,6 +259,48 @@ constructor( ) } } + + /** + * Returns a [StateFlow] that's automatically kept fresh. The passed-in [getFreshValue] is + * invoked on a background thread every time the selected user is changed and every time a new + * downstream subscriber is added to the flow. + * + * Initially, the flow will emit [initialValue] while it refreshes itself in the background by + * invoking the [getFreshValue] function and emitting the fresh value when that's done. + * + * Every time the selected user is changed, the flow will re-invoke [getFreshValue] and emit the + * new value. + * + * Every time a new downstream subscriber is added to the flow it first receives the latest + * cached value that's either the [initialValue] or the latest previously fetched value. In + * addition, adding a new downstream subscriber also triggers another [getFreshValue] call and a + * subsequent emission of that newest value. + */ + private fun refreshingFlow( + initialValue: T, + getFreshValue: suspend (selectedUserId: Int) -> T, + ): StateFlow { + val flow = MutableStateFlow(initialValue) + applicationScope.launch { + combine( + // Emits a value initially and every time the selected user is changed. + userRepository.selectedUserInfo.map { it.id }.distinctUntilChanged(), + // Emits a value only when the number of downstream subscribers of this flow + // increases. + flow.subscriptionCount.pairwise(initialValue = 0).filter { (previous, current) + -> + current > previous + }, + ) { selectedUserId, _ -> + selectedUserId + } + .collect { selectedUserId -> + flow.value = withContext(backgroundDispatcher) { getFreshValue(selectedUserId) } + } + } + + return flow.asStateFlow() + } } @Module diff --git a/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt b/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt index b482977bde679..d4371bf30e0ea 100644 --- a/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt @@ -104,7 +104,9 @@ constructor( } .stateIn( scope = applicationScope, - started = SharingStarted.Eagerly, + // Make sure this is kept as WhileSubscribed or we can run into a bug where the + // downstream continues to receive old/stale/cached values. + started = SharingStarted.WhileSubscribed(), initialValue = null, ) diff --git a/packages/SystemUI/src/com/android/systemui/bouncer/ui/viewmodel/PinBouncerViewModel.kt b/packages/SystemUI/src/com/android/systemui/bouncer/ui/viewmodel/PinBouncerViewModel.kt index 1b14acc7fabc9..38fb8b968775a 100644 --- a/packages/SystemUI/src/com/android/systemui/bouncer/ui/viewmodel/PinBouncerViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/bouncer/ui/viewmodel/PinBouncerViewModel.kt @@ -60,7 +60,9 @@ class PinBouncerViewModel( } .stateIn( scope = applicationScope, - started = SharingStarted.Eagerly, + // Make sure this is kept as WhileSubscribed or we can run into a bug where the + // downstream continues to receive old/stale/cached values. + started = SharingStarted.WhileSubscribed(), initialValue = ActionButtonAppearance.Hidden, ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/authentication/data/repository/AuthenticationRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/authentication/data/repository/AuthenticationRepositoryTest.kt new file mode 100644 index 0000000000000..005697044c0fb --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/authentication/data/repository/AuthenticationRepositoryTest.kt @@ -0,0 +1,113 @@ +/* + * Copyright 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +@file:OptIn(ExperimentalCoroutinesApi::class) + +package com.android.systemui.authentication.data.repository + +import android.content.pm.UserInfo +import androidx.test.filters.SmallTest +import com.android.internal.widget.LockPatternUtils +import com.android.keyguard.KeyguardSecurityModel +import com.android.systemui.SysuiTestCase +import com.android.systemui.coroutines.collectValues +import com.android.systemui.scene.SceneTestUtils +import com.android.systemui.user.data.repository.FakeUserRepository +import com.android.systemui.util.mockito.whenever +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.mockito.Mock +import org.mockito.MockitoAnnotations + +@SmallTest +@RunWith(JUnit4::class) +class AuthenticationRepositoryTest : SysuiTestCase() { + + @Mock private lateinit var lockPatternUtils: LockPatternUtils + + private val testUtils = SceneTestUtils(this) + private val testScope = testUtils.testScope + private val userRepository = FakeUserRepository() + + private lateinit var underTest: AuthenticationRepository + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + userRepository.setUserInfos(USER_INFOS) + runBlocking { userRepository.setSelectedUserInfo(USER_INFOS[0]) } + + underTest = + AuthenticationRepositoryImpl( + applicationScope = testScope.backgroundScope, + getSecurityMode = { KeyguardSecurityModel.SecurityMode.PIN }, + backgroundDispatcher = testUtils.testDispatcher, + userRepository = userRepository, + keyguardRepository = testUtils.keyguardRepository, + lockPatternUtils = lockPatternUtils, + ) + } + + @Test + fun isAutoConfirmEnabled() = + testScope.runTest { + whenever(lockPatternUtils.isAutoPinConfirmEnabled(USER_INFOS[0].id)).thenReturn(true) + whenever(lockPatternUtils.isAutoPinConfirmEnabled(USER_INFOS[1].id)).thenReturn(false) + + val values by collectValues(underTest.isAutoConfirmEnabled) + assertThat(values.first()).isFalse() + assertThat(values.last()).isTrue() + + userRepository.setSelectedUserInfo(USER_INFOS[1]) + assertThat(values.last()).isFalse() + } + + @Test + fun isPatternVisible() = + testScope.runTest { + whenever(lockPatternUtils.isVisiblePatternEnabled(USER_INFOS[0].id)).thenReturn(false) + whenever(lockPatternUtils.isVisiblePatternEnabled(USER_INFOS[1].id)).thenReturn(true) + + val values by collectValues(underTest.isPatternVisible) + assertThat(values.first()).isTrue() + assertThat(values.last()).isFalse() + + userRepository.setSelectedUserInfo(USER_INFOS[1]) + assertThat(values.last()).isTrue() + } + + companion object { + private val USER_INFOS = + listOf( + UserInfo( + /* id= */ 100, + /* name= */ "First user", + /* flags= */ 0, + ), + UserInfo( + /* id= */ 101, + /* name= */ "Second user", + /* flags= */ 0, + ), + ) + } +}