From eabd91006d070f04d00dc767813c968f2bee6f4a Mon Sep 17 00:00:00 2001 From: Chandru Date: Thu, 4 Aug 2022 17:26:39 +0000 Subject: [PATCH] Stop running face detection on bouncer if both face and fp are enrolled. Add s'more tests https://screencast.googleplex.com/cast/NTE1ODQ4MjYyMjgwODA2NHxkY2FkNzg5MC1jZg Fixes: 232876541 Test: atest KeyguardUpdateMonitorTest Test: verified manually Change-Id: Ia51bd773e90024159ce68d6b5536be440887eaa9 --- .../android/keyguard/KeyguardListenModel.kt | 3 +- .../keyguard/KeyguardUpdateMonitor.java | 20 ++- .../keyguard/KeyguardListenQueueTest.kt | 3 +- .../keyguard/KeyguardUpdateMonitorTest.java | 162 ++++++++++++++++-- 4 files changed, 170 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardListenModel.kt b/packages/SystemUI/src/com/android/keyguard/KeyguardListenModel.kt index 58e0fb968204d..0ea196537e725 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardListenModel.kt +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardListenModel.kt @@ -55,10 +55,11 @@ data class KeyguardFaceListenModel( val faceAuthenticated: Boolean, val faceDisabled: Boolean, val goingToSleep: Boolean, - val keyguardAwake: Boolean, + val keyguardAwakeExcludingBouncerShowing: Boolean, val keyguardGoingAway: Boolean, val listeningForFaceAssistant: Boolean, val occludingAppRequestingFaceAuth: Boolean, + val onlyFaceEnrolled: Boolean, val primaryUser: Boolean, val scanningAllowedByStrongAuth: Boolean, val secureCameraLaunched: Boolean, diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 2b44199ac9eef..f07d9ce215636 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -2474,8 +2474,11 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab } final boolean statusBarShadeLocked = mStatusBarState == StatusBarState.SHADE_LOCKED; - final boolean awakeKeyguard = mKeyguardIsVisible && mDeviceInteractive && !mGoingToSleep - && !statusBarShadeLocked; + // mKeyguardIsVisible is true even when the bouncer is shown, we don't want to run face auth + // on bouncer if both fp and fingerprint are enrolled. + final boolean awakeKeyguardExcludingBouncerShowing = mKeyguardIsVisible + && mDeviceInteractive && !mGoingToSleep + && !statusBarShadeLocked && !mBouncerFullyShown; final int user = getCurrentUser(); final int strongAuth = mStrongAuthTracker.getStrongAuthForUser(user); final boolean isLockDown = @@ -2515,14 +2518,15 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab final boolean faceDisabledForUser = isFaceDisabled(user); final boolean biometricEnabledForUser = mBiometricEnabledForUser.get(user); final boolean shouldListenForFaceAssistant = shouldListenForFaceAssistant(); + final boolean onlyFaceEnrolled = isOnlyFaceEnrolled(); // Only listen if this KeyguardUpdateMonitor belongs to the primary user. There is an // instance of KeyguardUpdateMonitor for each user but KeyguardUpdateMonitor is user-aware. final boolean shouldListen = - (mBouncerFullyShown && !mGoingToSleep + ((mBouncerFullyShown && !mGoingToSleep && onlyFaceEnrolled) || mAuthInterruptActive || mOccludingAppRequestingFace - || awakeKeyguard + || awakeKeyguardExcludingBouncerShowing || shouldListenForFaceAssistant || mAuthController.isUdfpsFingerDown() || mUdfpsBouncerShowing) @@ -2546,10 +2550,11 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab faceAuthenticated, faceDisabledForUser, mGoingToSleep, - awakeKeyguard, + awakeKeyguardExcludingBouncerShowing, mKeyguardGoingAway, shouldListenForFaceAssistant, mOccludingAppRequestingFace, + onlyFaceEnrolled, mIsPrimaryUser, strongAuthAllowsScanning, mSecureCameraLaunched, @@ -2559,6 +2564,11 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab return shouldListen; } + private boolean isOnlyFaceEnrolled() { + return isFaceAuthEnabledForUser(getCurrentUser()) + && !isUnlockWithFingerprintPossible(getCurrentUser()); + } + private void maybeLogListenerModelData(KeyguardListenModel model) { mLogger.logKeyguardListenerModel(model); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardListenQueueTest.kt b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardListenQueueTest.kt index ad6d146fb5f37..bb455da92ba2b 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardListenQueueTest.kt +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardListenQueueTest.kt @@ -86,10 +86,11 @@ private fun faceModel(user: Int) = KeyguardFaceListenModel( becauseCannotSkipBouncer = false, biometricSettingEnabledForUser = false, bouncerFullyShown = false, + onlyFaceEnrolled = false, faceAuthenticated = false, faceDisabled = false, goingToSleep = false, - keyguardAwake = false, + keyguardAwakeExcludingBouncerShowing = false, keyguardGoingAway = false, listeningForFaceAssistant = false, occludingAppRequestingFaceAuth = false, diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java index 035404ccbfe14..8d27f2422f66a 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java @@ -196,6 +196,8 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { mBiometricEnabledCallbackArgCaptor; @Captor private ArgumentCaptor mAuthenticationCallbackCaptor; + @Captor + private ArgumentCaptor mCancellationSignalCaptor; // Direct executor private Executor mBackgroundExecutor = Runnable::run; @@ -568,11 +570,12 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testTriesToAuthenticate_whenBouncer() { + fingerprintIsNotEnrolled(); setKeyguardBouncerVisibility(true); verify(mFaceManager).authenticate(any(), any(), any(), any(), anyInt(), anyBoolean()); - verify(mFaceManager).isHardwareDetected(); - verify(mFaceManager).hasEnrolledTemplates(anyInt()); + verify(mFaceManager, atLeastOnce()).isHardwareDetected(); + verify(mFaceManager, atLeastOnce()).hasEnrolledTemplates(anyInt()); } @Test @@ -1204,6 +1207,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { throws RemoteException { // Face auth should run when the following is true. bouncerFullyVisibleAndNotGoingToSleep(); + fingerprintIsNotEnrolled(); keyguardNotGoingAway(); currentUserIsPrimary(); strongAuthNotRequired(); @@ -1229,7 +1233,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { mKeyguardUpdateMonitor = new TestableKeyguardUpdateMonitor(mSpiedContext); - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); strongAuthNotRequired(); @@ -1246,7 +1250,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenStrongAuthDoesNotAllowScanning_returnsFalse() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); currentUserIsPrimary(); @@ -1267,9 +1271,10 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenBiometricsDisabledForUser_returnsFalse() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); + fingerprintIsNotEnrolled(); currentUserIsPrimary(); currentUserDoesNotHaveTrust(); biometricsNotDisabledThroughDevicePolicyManager(); @@ -1289,9 +1294,10 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenUserCurrentlySwitching_returnsFalse() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); + fingerprintIsNotEnrolled(); currentUserIsPrimary(); currentUserDoesNotHaveTrust(); biometricsNotDisabledThroughDevicePolicyManager(); @@ -1310,9 +1316,10 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenSecureCameraLaunched_returnsFalse() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); + fingerprintIsNotEnrolled(); currentUserIsPrimary(); currentUserDoesNotHaveTrust(); biometricsNotDisabledThroughDevicePolicyManager(); @@ -1331,7 +1338,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenOccludingAppRequestsFaceAuth_returnsTrue() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); bouncerFullyVisibleAndNotGoingToSleep(); currentUserIsPrimary(); @@ -1354,7 +1361,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenBouncerShowingAndDeviceIsAwake_returnsTrue() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); currentUserIsPrimary(); currentUserDoesNotHaveTrust(); @@ -1366,6 +1373,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); bouncerFullyVisibleAndNotGoingToSleep(); + fingerprintIsNotEnrolled(); mTestableLooper.processAllMessages(); assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); @@ -1374,7 +1382,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShouldListenForFace_whenAuthInterruptIsActive_returnsTrue() throws RemoteException { - // Face auth should run when the following is true. + // Preconditions for face auth to run keyguardNotGoingAway(); currentUserIsPrimary(); currentUserDoesNotHaveTrust(); @@ -1391,6 +1399,122 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); } + @Test + public void testShouldListenForFace_whenKeyguardIsAwake_returnsTrue() throws RemoteException { + // Preconditions for face auth to run + keyguardNotGoingAway(); + currentUserIsPrimary(); + currentUserDoesNotHaveTrust(); + biometricsNotDisabledThroughDevicePolicyManager(); + biometricsEnabledForCurrentUser(); + userNotCurrentlySwitching(); + bouncerFullyVisible(); + + statusBarShadeIsLocked(); + mTestableLooper.processAllMessages(); + + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + + deviceNotGoingToSleep(); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + deviceIsInteractive(); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + keyguardIsVisible(); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + statusBarShadeIsNotLocked(); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + bouncerNotFullyVisible(); + + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); + } + + @Test + public void testShouldListenForFace_whenUdfpsFingerDown_returnsTrue() throws RemoteException { + // Preconditions for face auth to run + keyguardNotGoingAway(); + currentUserIsPrimary(); + currentUserDoesNotHaveTrust(); + biometricsNotDisabledThroughDevicePolicyManager(); + biometricsEnabledForCurrentUser(); + userNotCurrentlySwitching(); + when(mAuthController.isUdfpsFingerDown()).thenReturn(false); + mTestableLooper.processAllMessages(); + + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + + when(mAuthController.isUdfpsFingerDown()).thenReturn(true); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); + } + + @Test + public void testShouldListenForFace_whenUdfpsBouncerIsShowing_returnsTrue() + throws RemoteException { + // Preconditions for face auth to run + keyguardNotGoingAway(); + currentUserIsPrimary(); + currentUserDoesNotHaveTrust(); + biometricsNotDisabledThroughDevicePolicyManager(); + biometricsEnabledForCurrentUser(); + userNotCurrentlySwitching(); + mTestableLooper.processAllMessages(); + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isFalse(); + + mKeyguardUpdateMonitor.setUdfpsBouncerShowing(true); + + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); + } + + @Test + public void testBouncerVisibility_whenBothFingerprintAndFaceIsEnrolled_stopsFaceAuth() + throws RemoteException { + // Both fingerprint and face are enrolled by default + // Preconditions for face auth to run + keyguardNotGoingAway(); + currentUserIsPrimary(); + currentUserDoesNotHaveTrust(); + biometricsNotDisabledThroughDevicePolicyManager(); + biometricsEnabledForCurrentUser(); + userNotCurrentlySwitching(); + deviceNotGoingToSleep(); + deviceIsInteractive(); + statusBarShadeIsNotLocked(); + keyguardIsVisible(); + + mTestableLooper.processAllMessages(); + + assertThat(mKeyguardUpdateMonitor.shouldListenForFace()).isTrue(); + + mKeyguardUpdateMonitor.requestFaceAuth(true); + verify(mFaceManager).authenticate(any(), + mCancellationSignalCaptor.capture(), + mAuthenticationCallbackCaptor.capture(), + any(), + anyInt(), + anyBoolean()); + CancellationSignal cancelSignal = mCancellationSignalCaptor.getValue(); + + bouncerFullyVisible(); + mTestableLooper.processAllMessages(); + + assertThat(cancelSignal.isCanceled()).isTrue(); + } + + private void fingerprintIsNotEnrolled() { + when(mFingerprintManager.hasEnrolledTemplates(mCurrentUserId)).thenReturn(false); + } + + private void statusBarShadeIsNotLocked() { + mStatusBarStateListener.onStateChanged(StatusBarState.KEYGUARD); + } + + private void statusBarShadeIsLocked() { + mStatusBarStateListener.onStateChanged(StatusBarState.SHADE_LOCKED); + } + + private void keyguardIsVisible() { + mKeyguardUpdateMonitor.onKeyguardVisibilityChanged(true); + } + private void triggerAuthInterrupt() { mKeyguardUpdateMonitor.onAuthInterruptDetected(true); } @@ -1468,10 +1592,26 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { } private void bouncerFullyVisibleAndNotGoingToSleep() { - mKeyguardUpdateMonitor.sendKeyguardBouncerChanged(true, true); + bouncerFullyVisible(); + deviceNotGoingToSleep(); + } + + private void deviceNotGoingToSleep() { mKeyguardUpdateMonitor.dispatchFinishedGoingToSleep(/* value doesn't matter */1); } + private void deviceIsInteractive() { + mKeyguardUpdateMonitor.dispatchStartedWakingUp(); + } + + private void bouncerNotFullyVisible() { + setKeyguardBouncerVisibility(false); + } + + private void bouncerFullyVisible() { + setKeyguardBouncerVisibility(true); + } + private void setKeyguardBouncerVisibility(boolean isVisible) { mKeyguardUpdateMonitor.sendKeyguardBouncerChanged(isVisible, isVisible); mTestableLooper.processAllMessages();