From 835653f67f89fca05a49c45cb50ceabf9ecf7292 Mon Sep 17 00:00:00 2001 From: Chandru S Date: Tue, 18 Jul 2023 14:58:37 -0700 Subject: [PATCH 1/2] Repeated calls to cancel should cancel any previously scheduled cancellation timeout jobs Fixes: 291161749 Test: atest DeviceEntryFaceAuthRepositoryTest Change-Id: I128b0cd68b29dfa9428df938f8dd03b336cd9c48 --- .../DeviceEntryFaceAuthRepository.kt | 2 ++ .../shared/model/FaceAuthenticationModels.kt | 8 +++++++ .../DeviceEntryFaceAuthRepositoryTest.kt | 24 +++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt index 6edf40f475218..b6a296aec3617 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt @@ -574,6 +574,7 @@ constructor( if (authCancellationSignal == null) return authCancellationSignal?.cancel() + cancelNotReceivedHandlerJob?.cancel() cancelNotReceivedHandlerJob = applicationScope.launch { delay(DEFAULT_CANCEL_SIGNAL_TIMEOUT) @@ -583,6 +584,7 @@ constructor( cancellationInProgress, faceAuthRequestedWhileCancellation ) + _authenticationStatus.value = ErrorFaceAuthenticationStatus.cancelNotReceivedError() onFaceAuthRequestCompleted() } cancellationInProgress = true diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt b/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt index 0f6d82ed4b5ce..dcd6ac4919c42 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt @@ -63,6 +63,14 @@ data class ErrorFaceAuthenticationStatus( fun isHardwareError() = msgId == FaceManager.FACE_ERROR_HW_UNAVAILABLE || msgId == FaceManager.FACE_ERROR_UNABLE_TO_PROCESS + + companion object { + /** + * Error message that is created when cancel confirmation is not received from FaceManager + * after we request for a cancellation of face auth. + */ + fun cancelNotReceivedError() = ErrorFaceAuthenticationStatus(-1, "") + } } /** Face detection success message. */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt index 47c662c3b4b02..a09f7b9e787e6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt @@ -44,6 +44,7 @@ import com.android.systemui.bouncer.data.repository.FakeKeyguardBouncerRepositor import com.android.systemui.bouncer.domain.interactor.AlternateBouncerInteractor import com.android.systemui.coroutines.FlowValue import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.coroutines.collectValues import com.android.systemui.dump.DumpManager import com.android.systemui.dump.logcatLogBuffer import com.android.systemui.flags.FakeFeatureFlags @@ -450,6 +451,29 @@ class DeviceEntryFaceAuthRepositoryTest : SysuiTestCase() { faceAuthenticateIsCalled() } + @Test + fun multipleCancelCallsShouldNotCauseMultipleCancellationStatusBeingEmitted() = + testScope.runTest { + initCollectors() + allPreconditionsToRunFaceAuthAreTrue() + val emittedValues by collectValues(underTest.authenticationStatus) + + underTest.authenticate(FACE_AUTH_TRIGGERED_SWIPE_UP_ON_BOUNCER) + underTest.cancel() + advanceTimeBy(100) + underTest.cancel() + + advanceTimeBy(DeviceEntryFaceAuthRepositoryImpl.DEFAULT_CANCEL_SIGNAL_TIMEOUT) + runCurrent() + advanceTimeBy(DeviceEntryFaceAuthRepositoryImpl.DEFAULT_CANCEL_SIGNAL_TIMEOUT) + runCurrent() + + assertThat(emittedValues.size).isEqualTo(1) + assertThat(emittedValues.first()) + .isInstanceOf(ErrorFaceAuthenticationStatus::class.java) + assertThat((emittedValues.first() as ErrorFaceAuthenticationStatus).msgId).isEqualTo(-1) + } + @Test fun faceHelpMessagesAreIgnoredBasedOnConfig() = testScope.runTest { From 320701e45810d2887f45881bb6cf8a92589976ca Mon Sep 17 00:00:00 2001 From: Chandru S Date: Tue, 18 Jul 2023 15:52:52 -0700 Subject: [PATCH 2/2] Resolve face detection related issues 1. Add createdAt so that repeated updates are still propagated 2. Do not use detection cancellation signal to skip running detection. Unlike face auth, detection doesn't have a callback to indicate when detection is done cancelling. 3. Change KeyguardUpdateMonitor to run the same code it is running for the old face auth system Fixes: 285526875 Test: atest DeviceEntryFaceAuthRepositoryTest Test: manually 1. Enable Extend Unlock/Smart Unlock and add the current location as trusted location 2. Enroll face and fp, enable bypass lockscreen under face. 3. Go to sleep by pressing the power button 4. Wake up the device and go to the lockscreen. 5. Device should be unlocked based on current location and show the unlocked icon 6. Tap on the lockscreen while keeping the face in front of the screen 7. Face detection should run and dismiss the lockscreen. Change-Id: Ia4b541e0ec5c6e6f876f4a21e788691e96e38870 --- .../android/keyguard/KeyguardUpdateMonitor.java | 2 +- .../repository/DeviceEntryFaceAuthRepository.kt | 4 ++-- .../shared/model/FaceAuthenticationModels.kt | 15 +++++++++++---- .../DeviceEntryFaceAuthRepositoryTest.kt | 5 ++++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 84f1da01c5c1d..f5022ab64add1 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -1496,7 +1496,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab @Override public void onDetectionStatusChanged(@NonNull FaceDetectionStatus status) { - handleFaceAuthenticated(status.getUserId(), status.isStrongBiometric()); + handleBiometricDetected(status.getUserId(), FACE, status.isStrongBiometric()); } }; diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt index b6a296aec3617..d1f011ea4f9b3 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepository.kt @@ -545,11 +545,11 @@ constructor( faceAuthLogger.detectionNotSupported(faceManager, faceManager?.sensorPropertiesInternal) return } - if (_isAuthRunning.value || detectCancellationSignal != null) { + if (_isAuthRunning.value) { faceAuthLogger.skippingDetection(_isAuthRunning.value, detectCancellationSignal != null) return } - + detectCancellationSignal?.cancel() detectCancellationSignal = CancellationSignal() withContext(mainDispatcher) { // We always want to invoke face detect in the main thread. diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt b/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt index dcd6ac4919c42..d9792cf704c8a 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/shared/model/FaceAuthenticationModels.kt @@ -23,7 +23,10 @@ import android.os.SystemClock.elapsedRealtime * Authentication status provided by * [com.android.systemui.keyguard.data.repository.DeviceEntryFaceAuthRepository] */ -sealed class FaceAuthenticationStatus +sealed class FaceAuthenticationStatus( + // present to break equality check if the same error occurs repeatedly. + val createdAt: Long = elapsedRealtime() +) /** Success authentication status. */ data class SuccessFaceAuthenticationStatus(val successResult: FaceManager.AuthenticationResult) : @@ -43,8 +46,6 @@ object FailedFaceAuthenticationStatus : FaceAuthenticationStatus() data class ErrorFaceAuthenticationStatus( val msgId: Int, val msg: String? = null, - // present to break equality check if the same error occurs repeatedly. - val createdAt: Long = elapsedRealtime() ) : FaceAuthenticationStatus() { /** * Method that checks if [msgId] is a lockout error. A lockout error means that face @@ -74,4 +75,10 @@ data class ErrorFaceAuthenticationStatus( } /** Face detection success message. */ -data class FaceDetectionStatus(val sensorId: Int, val userId: Int, val isStrongBiometric: Boolean) +data class FaceDetectionStatus( + val sensorId: Int, + val userId: Int, + val isStrongBiometric: Boolean, + // present to break equality check if the same error occurs repeatedly. + val createdAt: Long = elapsedRealtime() +) diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt index a09f7b9e787e6..8127ac6257480 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/DeviceEntryFaceAuthRepositoryTest.kt @@ -386,7 +386,10 @@ class DeviceEntryFaceAuthRepositoryTest : SysuiTestCase() { detectionCallback.value.onFaceDetected(1, 1, true) - assertThat(detectStatus()).isEqualTo(FaceDetectionStatus(1, 1, true)) + val status = detectStatus()!! + assertThat(status.sensorId).isEqualTo(1) + assertThat(status.userId).isEqualTo(1) + assertThat(status.isStrongBiometric).isEqualTo(true) } @Test