From 2c904ea562b46a350f2d0578db2bfd79b87da88b Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 15 Jul 2021 18:19:05 -0700 Subject: [PATCH] 2/n: Update fingerprint vibration logic 1) adds "mShouldVibrate" as a property for AcquisitionClient. Philosophically, all clients that inherit this class should be able to decide if haptics occur in onAuthentication* 2) UDFPS AIDL enroll haptic occurs when ACQUIRED_GOOD is received 3) UDFPS AIDL auth haptic continues to happen when success/reject is known (documenting reason in the bug below) Test: manual enroll+auth Test: atest com.android.server.biometrics Test: adb shell dumpsys vibrator_manager Bug: 193089985 Change-Id: I8ae60f99d36a4f49985411001670df3ee108768f --- .../biometrics/sensors/AcquisitionClient.java | 20 +++++++++++++++---- .../sensors/AuthenticationClient.java | 10 ++++++---- .../biometrics/sensors/EnrollClient.java | 7 ++----- .../face/aidl/FaceAuthenticationClient.java | 2 +- .../sensors/face/aidl/FaceDetectClient.java | 4 ++-- .../face/hidl/FaceAuthenticationClient.java | 2 +- .../aidl/FingerprintAuthenticationClient.java | 6 ++++-- .../aidl/FingerprintDetectClient.java | 4 ++-- .../aidl/FingerprintEnrollClient.java | 4 +++- .../fingerprint/aidl/FingerprintProvider.java | 3 ++- .../hidl/FingerprintAuthenticationClient.java | 2 +- .../hidl/FingerprintDetectClient.java | 4 ++-- .../sensors/AcquisitionClientTest.java | 3 ++- .../sensors/BiometricSchedulerTest.java | 4 ++-- 14 files changed, 46 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java b/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java index f11fe8aee64f3..c2eb06262edd5 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java @@ -22,6 +22,7 @@ import android.hardware.biometrics.BiometricConstants; import android.media.AudioAttributes; import android.os.IBinder; import android.os.PowerManager; +import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; import android.os.VibrationEffect; @@ -49,6 +50,8 @@ public abstract class AcquisitionClient extends HalClientMonitor implement VibrationEffect.get(VibrationEffect.EFFECT_DOUBLE_CLICK); private final PowerManager mPowerManager; + // If haptics should occur when auth result (success/reject) is known + protected final boolean mShouldVibrate; private boolean mShouldSendErrorToClient = true; private boolean mAlreadyCancelled; @@ -59,11 +62,12 @@ public abstract class AcquisitionClient extends HalClientMonitor implement public AcquisitionClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId, - @NonNull String owner, int cookie, int sensorId, int statsModality, - int statsAction, int statsClient) { + @NonNull String owner, int cookie, int sensorId, boolean shouldVibrate, + int statsModality, int statsAction, int statsClient) { super(context, lazyDaemon, token, listener, userId, owner, cookie, sensorId, statsModality, statsAction, statsClient); mPowerManager = context.getSystemService(PowerManager.class); + mShouldVibrate = shouldVibrate; } @Override @@ -191,14 +195,22 @@ public abstract class AcquisitionClient extends HalClientMonitor implement protected final void vibrateSuccess() { Vibrator vibrator = getContext().getSystemService(Vibrator.class); if (vibrator != null) { - vibrator.vibrate(SUCCESS_VIBRATION_EFFECT, VIBRATION_SONIFICATION_ATTRIBUTES); + vibrator.vibrate(Process.myUid(), + getContext().getOpPackageName(), + SUCCESS_VIBRATION_EFFECT, + getClass().getSimpleName() + "::success", + VIBRATION_SONIFICATION_ATTRIBUTES); } } protected final void vibrateError() { Vibrator vibrator = getContext().getSystemService(Vibrator.class); if (vibrator != null) { - vibrator.vibrate(ERROR_VIBRATION_EFFECT, VIBRATION_SONIFICATION_ATTRIBUTES); + vibrator.vibrate(Process.myUid(), + getContext().getOpPackageName(), + ERROR_VIBRATION_EFFECT, + getClass().getSimpleName() + "::error", + VIBRATION_SONIFICATION_ATTRIBUTES); } } } 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 80e60e67f05a6..3e6602e3175ad 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java @@ -68,9 +68,11 @@ public abstract class AuthenticationClient extends AcquisitionClient int targetUserId, long operationId, boolean restricted, @NonNull String owner, int cookie, boolean requireConfirmation, int sensorId, boolean isStrongBiometric, int statsModality, int statsClient, @Nullable TaskStackListener taskStackListener, - @NonNull LockoutTracker lockoutTracker, boolean allowBackgroundAuthentication) { + @NonNull LockoutTracker lockoutTracker, boolean allowBackgroundAuthentication, + boolean shouldVibrate) { super(context, lazyDaemon, token, listener, targetUserId, owner, cookie, sensorId, - statsModality, BiometricsProtoEnums.ACTION_AUTHENTICATE, statsClient); + shouldVibrate, statsModality, BiometricsProtoEnums.ACTION_AUTHENTICATE, + statsClient); mIsStrongBiometric = isStrongBiometric; mOperationId = operationId; mRequireConfirmation = requireConfirmation; @@ -204,7 +206,7 @@ public abstract class AuthenticationClient extends AcquisitionClient mAlreadyDone = true; - if (listener != null) { + if (listener != null && mShouldVibrate) { vibrateSuccess(); } @@ -250,7 +252,7 @@ public abstract class AuthenticationClient extends AcquisitionClient Slog.w(TAG, "Client not listening"); } } else { - if (listener != null) { + if (listener != null && mShouldVibrate) { vibrateError(); } diff --git a/services/core/java/com/android/server/biometrics/sensors/EnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/EnrollClient.java index e1320d8e1a4f1..a15e14b79e301 100644 --- a/services/core/java/com/android/server/biometrics/sensors/EnrollClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/EnrollClient.java @@ -38,7 +38,6 @@ public abstract class EnrollClient extends AcquisitionClient { protected final byte[] mHardwareAuthToken; protected final int mTimeoutSec; protected final BiometricUtils mBiometricUtils; - private final boolean mShouldVibrate; private long mEnrollmentStartTimeMs; @@ -50,15 +49,13 @@ public abstract class EnrollClient extends AcquisitionClient { public EnrollClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId, @NonNull byte[] hardwareAuthToken, @NonNull String owner, @NonNull BiometricUtils utils, - int timeoutSec, int statsModality, int sensorId, - boolean shouldVibrate) { + int timeoutSec, int statsModality, int sensorId, boolean shouldVibrate) { super(context, lazyDaemon, token, listener, userId, owner, 0 /* cookie */, sensorId, - statsModality, BiometricsProtoEnums.ACTION_ENROLL, + shouldVibrate, statsModality, BiometricsProtoEnums.ACTION_ENROLL, BiometricsProtoEnums.CLIENT_UNKNOWN); mBiometricUtils = utils; mHardwareAuthToken = Arrays.copyOf(hardwareAuthToken, hardwareAuthToken.length); mTimeoutSec = timeoutSec; - mShouldVibrate = shouldVibrate; } public void onEnrollResult(BiometricAuthenticator.Identifier identifier, int remaining) { diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java index 3757404d226d9..0525d2da69880 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java @@ -73,7 +73,7 @@ class FaceAuthenticationClient extends AuthenticationClient implements super(context, lazyDaemon, token, listener, targetUserId, operationId, restricted, owner, cookie, requireConfirmation, sensorId, isStrongBiometric, BiometricsProtoEnums.MODALITY_FACE, statsClient, null /* taskStackListener */, - lockoutCache, allowBackgroundAuthentication); + lockoutCache, allowBackgroundAuthentication, true /* shouldVibrate */); mUsageStats = usageStats; mLockoutCache = lockoutCache; mNotificationManager = context.getSystemService(NotificationManager.class); diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceDetectClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceDetectClient.java index cb966e7b47a96..1e73ac528f088 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceDetectClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceDetectClient.java @@ -46,8 +46,8 @@ public class FaceDetectClient extends AcquisitionClient implements Det @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId, @NonNull String owner, int sensorId, boolean isStrongBiometric, int statsClient) { super(context, lazyDaemon, token, listener, userId, owner, 0 /* cookie */, sensorId, - BiometricsProtoEnums.MODALITY_FACE, BiometricsProtoEnums.ACTION_AUTHENTICATE, - statsClient); + true /* shouldVibrate */, BiometricsProtoEnums.MODALITY_FACE, + BiometricsProtoEnums.ACTION_AUTHENTICATE, statsClient); mIsStrongBiometric = isStrongBiometric; } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java index c3de7aa74d159..5731d73dfd492 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java @@ -65,7 +65,7 @@ class FaceAuthenticationClient extends AuthenticationClient { super(context, lazyDaemon, token, listener, targetUserId, operationId, restricted, owner, cookie, requireConfirmation, sensorId, isStrongBiometric, BiometricsProtoEnums.MODALITY_FACE, statsClient, null /* taskStackListener */, - lockoutTracker, allowBackgroundAuthentication); + lockoutTracker, allowBackgroundAuthentication, true /* shouldVibrate */); mUsageStats = usageStats; final Resources resources = getContext().getResources(); 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 19134e46f08fd..8681ad75b7c65 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 @@ -26,6 +26,7 @@ import android.hardware.biometrics.BiometricFingerprintConstants.FingerprintAcqu import android.hardware.biometrics.BiometricsProtoEnums; import android.hardware.biometrics.common.ICancellationSignal; import android.hardware.biometrics.fingerprint.ISession; +import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; import android.hardware.fingerprint.IUdfpsOverlayController; import android.os.IBinder; import android.os.RemoteException; @@ -62,11 +63,12 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp int sensorId, boolean isStrongBiometric, int statsClient, @Nullable TaskStackListener taskStackListener, @NonNull LockoutCache lockoutCache, @Nullable IUdfpsOverlayController udfpsOverlayController, - boolean allowBackgroundAuthentication) { + boolean allowBackgroundAuthentication, + @NonNull FingerprintSensorPropertiesInternal sensorProps) { super(context, lazyDaemon, token, listener, targetUserId, operationId, restricted, owner, cookie, requireConfirmation, sensorId, isStrongBiometric, BiometricsProtoEnums.MODALITY_FINGERPRINT, statsClient, taskStackListener, - lockoutCache, allowBackgroundAuthentication); + lockoutCache, allowBackgroundAuthentication, true /* shouldVibrate */); mLockoutCache = lockoutCache; mUdfpsOverlayController = udfpsOverlayController; } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintDetectClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintDetectClient.java index 5e1a245554a65..c5dc44988612e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintDetectClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintDetectClient.java @@ -52,8 +52,8 @@ class FingerprintDetectClient extends AcquisitionClient implements Det @Nullable IUdfpsOverlayController udfpsOverlayController, boolean isStrongBiometric, int statsClient) { super(context, lazyDaemon, token, listener, userId, owner, 0 /* cookie */, sensorId, - BiometricsProtoEnums.MODALITY_FINGERPRINT, BiometricsProtoEnums.ACTION_AUTHENTICATE, - statsClient); + true /* shouldVibrate */, BiometricsProtoEnums.MODALITY_FINGERPRINT, + BiometricsProtoEnums.ACTION_AUTHENTICATE, statsClient); mIsStrongBiometric = isStrongBiometric; mUdfpsOverlayController = udfpsOverlayController; } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintEnrollClient.java index 2859512a9120c..a211bb5e14e38 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintEnrollClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintEnrollClient.java @@ -65,9 +65,10 @@ class FingerprintEnrollClient extends EnrollClient implements Udfps { @Nullable IUdfpsOverlayController udfpsOvelayController, @Nullable ISidefpsController sidefpsController, int maxTemplatesPerUser, @FingerprintManager.EnrollReason int enrollReason) { + // UDFPS haptics occur when an image is acquired (instead of when the result is known) super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, owner, utils, 0 /* timeoutSec */, BiometricsProtoEnums.MODALITY_FINGERPRINT, sensorId, - true /* shouldVibrate */); + !sensorProps.isAnyUdfpsType() /* shouldVibrate */); mSensorProps = sensorProps; mUdfpsOverlayController = udfpsOvelayController; mSidefpsController = sidefpsController; @@ -103,6 +104,7 @@ class FingerprintEnrollClient extends EnrollClient implements Udfps { // See AcquiredInfo#GOOD and AcquiredInfo#RETRYING_CAPTURE if (acquiredInfo == BiometricFingerprintConstants.FINGERPRINT_ACQUIRED_GOOD && mSensorProps.isAnyUdfpsType()) { + vibrateSuccess(); UdfpsHelper.onAcquiredGood(getSensorId(), mUdfpsOverlayController); } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java index 096c3111d35c9..cfc4674303490 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java @@ -395,7 +395,8 @@ public class FingerprintProvider implements IBinder.DeathRecipient, ServiceProvi operationId, restricted, opPackageName, cookie, false /* requireConfirmation */, sensorId, isStrongBiometric, statsClient, mTaskStackListener, mSensors.get(sensorId).getLockoutCache(), - mUdfpsOverlayController, allowBackgroundAuthentication); + mUdfpsOverlayController, allowBackgroundAuthentication, + mSensors.get(sensorId).getSensorProperties()); scheduleForSensor(sensorId, client, fingerprintStateCallback); }); } 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 bf7775730a2cd..40e3bc3a46986 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 @@ -65,7 +65,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient int sensorId, @Nullable IUdfpsOverlayController udfpsOverlayController, boolean isStrongBiometric, int statsClient) { super(context, lazyDaemon, token, listener, userId, owner, 0 /* cookie */, sensorId, - BiometricsProtoEnums.MODALITY_FINGERPRINT, BiometricsProtoEnums.ACTION_AUTHENTICATE, - statsClient); + true /* shouldVibrate */, BiometricsProtoEnums.MODALITY_FINGERPRINT, + BiometricsProtoEnums.ACTION_AUTHENTICATE, statsClient); mUdfpsOverlayController = udfpsOverlayController; mIsStrongBiometric = isStrongBiometric; } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/AcquisitionClientTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/AcquisitionClientTest.java index 46f96364ff9e4..a06a782885359 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/AcquisitionClientTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/AcquisitionClientTest.java @@ -90,7 +90,8 @@ public class AcquisitionClientTest { @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter callback) { super(context, lazyDaemon, token, callback, 0 /* userId */, "Test", 0 /* cookie */, - TEST_SENSOR_ID /* sensorId */, 0 /* statsModality */, 0 /* statsAction */, + TEST_SENSOR_ID /* sensorId */, true /* shouldVibrate */, 0 /* statsModality */, + 0 /* statsAction */, 0 /* statsClient */); } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java index 4d1f241787a75..109fb22520c8e 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java @@ -359,7 +359,7 @@ public class BiometricSchedulerTest { false /* restricted */, TAG, 1 /* cookie */, false /* requireConfirmation */, TEST_SENSOR_ID, true /* isStrongBiometric */, 0 /* statsModality */, 0 /* statsClient */, null /* taskStackListener */, mock(LockoutTracker.class), - false /* isKeyguard */); + false /* isKeyguard */, true /* shouldVibrate */); } @Override @@ -382,7 +382,7 @@ public class BiometricSchedulerTest { false /* restricted */, TAG, 1 /* cookie */, false /* requireConfirmation */, TEST_SENSOR_ID, true /* isStrongBiometric */, 0 /* statsModality */, 0 /* statsClient */, null /* taskStackListener */, mock(LockoutTracker.class), - false /* isKeyguard */); + false /* isKeyguard */, true /* shouldVibrate */); } @Override