From 4a468bd48c96e6680466c58020b8967b55b6a765 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 28 Jul 2021 14:46:59 -0700 Subject: [PATCH] Disable face haptics during coex non-bypass by default Face haptic should still be played when bypass is enabled. Adds test cases for both paths. Bug: 193089985 Test: adb shell dumpsys biometric Test: atest com.android.server.biometrics Change-Id: I0dd89ee9a63307cd5cef886aded5a7b618c580d7 --- .../server/biometrics/BiometricService.java | 7 ++ .../sensors/AuthenticationClient.java | 12 ++- .../biometrics/sensors/CoexCoordinator.java | 38 +++++++++- .../aidl/FingerprintAuthenticationClient.java | 4 +- .../hidl/FingerprintAuthenticationClient.java | 4 +- .../sensors/CoexCoordinatorTest.java | 75 ++++++++++++++++++- 6 files changed, 125 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index 4973d45293e3f..b1d300cd838e4 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -1106,6 +1106,11 @@ public class BiometricService extends SystemService { return Settings.Secure.getInt(context.getContentResolver(), CoexCoordinator.SETTING_ENABLE_NAME, 1) != 0; } + + public boolean isCoexFaceNonBypassHapticsDisabled(Context context) { + return Settings.Secure.getInt(context.getContentResolver(), + CoexCoordinator.FACE_HAPTIC_DISABLE, 1) != 0; + } } /** @@ -1137,6 +1142,8 @@ public class BiometricService extends SystemService { // by default. CoexCoordinator coexCoordinator = CoexCoordinator.getInstance(); coexCoordinator.setAdvancedLogicEnabled(injector.isAdvancedCoexLogicEnabled(context)); + coexCoordinator.setFaceHapticDisabledWhenNonBypass( + injector.isCoexFaceNonBypassHapticsDisabled(context)); try { injector.getActivityManagerService().registerUserSwitchObserver( diff --git a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java index 013c74d10d00e..f4327e8a104dc 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java @@ -54,12 +54,18 @@ public abstract class AuthenticationClient extends AcquisitionClient public static final int STATE_NEW = 0; // Framework/HAL have started this operation public static final int STATE_STARTED = 1; - // Operation is started, but requires some user action (such as finger lift & re-touch) + // Operation is started, but requires some user action to start (such as finger lift & re-touch) public static final int STATE_STARTED_PAUSED = 2; + // Same as above, except auth was attempted (rejected, timed out, etc). + public static final int STATE_STARTED_PAUSED_ATTEMPTED = 3; // Done, errored, canceled, etc. HAL/framework are not running this sensor anymore. - public static final int STATE_STOPPED = 3; + public static final int STATE_STOPPED = 4; - @IntDef({STATE_NEW, STATE_STARTED, STATE_STARTED_PAUSED, STATE_STOPPED}) + @IntDef({STATE_NEW, + STATE_STARTED, + STATE_STARTED_PAUSED, + STATE_STARTED_PAUSED_ATTEMPTED, + STATE_STOPPED}) @interface State {} private final boolean mIsStrongBiometric; diff --git a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java index b576673111aee..a15ecada3caed 100644 --- a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java +++ b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java @@ -45,6 +45,8 @@ public class CoexCoordinator { private static final String TAG = "BiometricCoexCoordinator"; public static final String SETTING_ENABLE_NAME = "com.android.server.biometrics.sensors.CoexCoordinator.enable"; + public static final String FACE_HAPTIC_DISABLE = + "com.android.server.biometrics.sensors.CoexCoordinator.disable_face_haptics"; private static final boolean DEBUG = true; // Successful authentications should be used within this amount of time. @@ -144,6 +146,10 @@ public class CoexCoordinator { mAdvancedLogicEnabled = enabled; } + public void setFaceHapticDisabledWhenNonBypass(boolean disabled) { + mFaceHapticDisabledWhenNonBypass = disabled; + } + @VisibleForTesting void reset() { mClientMap.clear(); @@ -153,6 +159,7 @@ public class CoexCoordinator { private final Map> mClientMap; @VisibleForTesting final LinkedList mSuccessfulAuths; private boolean mAdvancedLogicEnabled; + private boolean mFaceHapticDisabledWhenNonBypass; private final Handler mHandler; private CoexCoordinator() { @@ -225,7 +232,11 @@ public class CoexCoordinator { mSuccessfulAuths.add(new SuccessfulAuth(mHandler, mSuccessfulAuths, currentTimeMillis, SENSOR_TYPE_FACE, client, callback)); } else { - callback.sendHapticFeedback(); + if (mFaceHapticDisabledWhenNonBypass && !face.isKeyguardBypassEnabled()) { + Slog.w(TAG, "Skipping face success haptic"); + } else { + callback.sendHapticFeedback(); + } callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); callback.handleLifecycleAfterAuth(); } @@ -278,12 +289,23 @@ public class CoexCoordinator { // BiometricScheduler do not get stuck. Slog.d(TAG, "Face rejected in multi-sensor auth, udfps: " + udfps); callback.handleLifecycleAfterAuth(); - } else { - // UDFPS is not actively authenticating (finger not touching, already - // rejected, etc). + } else if (isUdfpsAuthAttempted(udfps)) { + // If UDFPS is STATE_STARTED_PAUSED (e.g. finger rejected but can still + // auth after pointer goes down, it means UDFPS encountered a rejection. In + // this case, we need to play the final reject haptic since face auth is + // also done now. callback.sendHapticFeedback(); callback.handleLifecycleAfterAuth(); } + else { + // UDFPS auth has never been attempted. + if (mFaceHapticDisabledWhenNonBypass && !face.isKeyguardBypassEnabled()) { + Slog.w(TAG, "Skipping face reject haptic"); + } else { + callback.sendHapticFeedback(); + } + callback.handleLifecycleAfterAuth(); + } } else if (isCurrentUdfps(client)) { // Face should either be running, or have already finished SuccessfulAuth auth = popSuccessfulFaceAuthIfExists(currentTimeMillis); @@ -374,6 +396,13 @@ public class CoexCoordinator { return false; } + private static boolean isUdfpsAuthAttempted(@Nullable AuthenticationClient client) { + if (client instanceof Udfps) { + return client.getState() == AuthenticationClient.STATE_STARTED_PAUSED_ATTEMPTED; + } + return false; + } + private boolean isUnknownClient(@NonNull AuthenticationClient client) { for (AuthenticationClient c : mClientMap.values()) { if (c == client) { @@ -400,6 +429,7 @@ public class CoexCoordinator { public String toString() { StringBuilder sb = new StringBuilder(); sb.append("Enabled: ").append(mAdvancedLogicEnabled); + sb.append(", Face Haptic Disabled: ").append(mFaceHapticDisabledWhenNonBypass); sb.append(", Queue size: " ).append(mSuccessfulAuths.size()); for (SuccessfulAuth auth : mSuccessfulAuths) { sb.append(", Auth: ").append(auth.toString()); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java index 99e6e626f5fec..8835c1e026104 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java @@ -114,7 +114,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp mState = STATE_STOPPED; UdfpsHelper.hideUdfpsOverlay(getSensorId(), mUdfpsOverlayController); } else { - mState = STATE_STARTED_PAUSED; + mState = STATE_STARTED_PAUSED_ATTEMPTED; } } @@ -188,7 +188,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp public void onPointerUp() { try { mIsPointerDown = false; - mState = STATE_STARTED_PAUSED; + mState = STATE_STARTED_PAUSED_ATTEMPTED; mALSProbeCallback.getProbe().disable(); getFreshDaemon().onPointerUp(0 /* pointerId */); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java index 7558d15fbe326..83f1480f46119 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java @@ -112,7 +112,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient faceClient = mock(AuthenticationClient.class); + when(faceClient.isKeyguard()).thenReturn(true); + when(faceClient.isKeyguardBypassEnabled()).thenReturn(bypassEnabled); + + AuthenticationClient udfpsClient = mock(AuthenticationClient.class, + withSettings().extraInterfaces(Udfps.class)); + when(udfpsClient.isKeyguard()).thenReturn(true); + when(((Udfps) udfpsClient).isPointerDown()).thenReturn(false); + + mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); + mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_UDFPS, udfpsClient); + + if (faceAccepted) { + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, faceClient, + mCallback); + } else { + mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, faceClient, + LockoutTracker.LOCKOUT_NONE, mCallback); + } + + if (shouldReceiveHaptics) { + verify(mCallback).sendHapticFeedback(); + } else { + verify(mCallback, never()).sendHapticFeedback(); + } + + verify(mCallback).sendAuthenticationResult(eq(faceAccepted) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); + } + @Test public void testKeyguard_faceAuth_udfpsTouching_faceSuccess_thenUdfpsRejectedWithinBounds() { testKeyguard_faceAuth_udfpsTouching_faceSuccess(false /* thenUdfpsAccepted */, @@ -294,12 +359,13 @@ public class CoexCoordinatorTest { } @Test - public void testKeyguard_udfpsRejected_thenFaceRejected() { + public void testKeyguard_udfpsRejected_thenFaceRejected_noKeyguardBypass() { mCoexCoordinator.reset(); AuthenticationClient faceClient = mock(AuthenticationClient.class); when(faceClient.isKeyguard()).thenReturn(true); when(faceClient.getState()).thenReturn(AuthenticationClient.STATE_STARTED); + when(faceClient.isKeyguardBypassEnabled()).thenReturn(false); // TODO: also test "true" case AuthenticationClient udfpsClient = mock(AuthenticationClient.class, withSettings().extraInterfaces(Udfps.class)); @@ -312,8 +378,9 @@ public class CoexCoordinatorTest { mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, udfpsClient, LockoutTracker.LOCKOUT_NONE, mCallback); - // Client becomes paused, but finger does not necessarily lift, since we suppress the haptic - when(udfpsClient.getState()).thenReturn(AuthenticationClient.STATE_STARTED_PAUSED); + // Auth was attempted + when(udfpsClient.getState()) + .thenReturn(AuthenticationClient.STATE_STARTED_PAUSED_ATTEMPTED); verify(mCallback, never()).sendHapticFeedback(); verify(mCallback).handleLifecycleAfterAuth();