From 446644041bc2f3d9c24f37b40907a6b061bb9920 Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Thu, 13 Jan 2022 17:51:55 +0000 Subject: [PATCH 1/2] Revert "Revert "Fix enrollment cancelation race conditions."" This reverts commit f4dc031e6c25e01de5efad59e6baf482abc9dd91. Reason for revert: Reapplying with additional change to fix the issue that broke HIDL clients. Change-Id: Idd1485f182cf841b96ced1dab4b8330ff3b57c2a --- .../android/hardware/face/FaceManager.java | 49 +- .../android/hardware/face/IFaceService.aidl | 9 +- .../fingerprint/FingerprintManager.java | 30 +- .../fingerprint/IFingerprintService.aidl | 4 +- .../biometrics/sensors/BaseClientMonitor.java | 6 +- .../sensors/BiometricScheduler.java | 423 ++++-------------- .../sensors/BiometricSchedulerOperation.java | 419 +++++++++++++++++ .../biometrics/sensors/Interruptable.java | 5 + .../sensors/UserAwareBiometricScheduler.java | 35 +- .../biometrics/sensors/face/FaceService.java | 15 +- .../sensors/face/ServiceProvider.java | 4 +- .../sensors/face/aidl/FaceEnrollClient.java | 3 +- .../sensors/face/aidl/FaceProvider.java | 11 +- .../biometrics/sensors/face/aidl/Sensor.java | 2 +- .../biometrics/sensors/face/hidl/Face10.java | 23 +- .../sensors/face/hidl/FaceEnrollClient.java | 3 +- .../fingerprint/FingerprintService.java | 12 +- .../sensors/fingerprint/ServiceProvider.java | 4 +- .../aidl/FingerprintEnrollClient.java | 3 +- .../fingerprint/aidl/FingerprintProvider.java | 11 +- .../sensors/fingerprint/aidl/Sensor.java | 2 +- .../fingerprint/hidl/Fingerprint21.java | 30 +- .../hidl/Fingerprint21UdfpsMock.java | 33 +- .../hidl/FingerprintEnrollClient.java | 3 +- .../BiometricSchedulerOperationTest.java | 326 ++++++++++++++ .../sensors/BiometricSchedulerTest.java | 268 ++++++----- .../UserAwareBiometricSchedulerTest.java | 41 +- .../sensors/face/aidl/SensorTest.java | 1 + .../sensors/face/hidl/Face10Test.java | 5 +- .../sensors/fingerprint/aidl/SensorTest.java | 1 + 30 files changed, 1177 insertions(+), 604 deletions(-) create mode 100644 services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java create mode 100644 services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerOperationTest.java diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 56f81423db4ee..b97055976e3ec 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -306,22 +306,21 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan throw new IllegalArgumentException("Must supply an enrollment callback"); } - if (cancel != null) { - if (cancel.isCanceled()) { - Slog.w(TAG, "enrollment already canceled"); - return; - } else { - cancel.setOnCancelListener(new OnEnrollCancelListener()); - } + if (cancel != null && cancel.isCanceled()) { + Slog.w(TAG, "enrollment already canceled"); + return; } if (mService != null) { try { mEnrollmentCallback = callback; Trace.beginSection("FaceManager#enroll"); - mService.enroll(userId, mToken, hardwareAuthToken, mServiceReceiver, - mContext.getOpPackageName(), disabledFeatures, previewSurface, - debugConsent); + final long enrollId = mService.enroll(userId, mToken, hardwareAuthToken, + mServiceReceiver, mContext.getOpPackageName(), disabledFeatures, + previewSurface, debugConsent); + if (cancel != null) { + cancel.setOnCancelListener(new OnEnrollCancelListener(enrollId)); + } } catch (RemoteException e) { Slog.w(TAG, "Remote exception in enroll: ", e); // Though this may not be a hardware issue, it will cause apps to give up or @@ -359,21 +358,20 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan throw new IllegalArgumentException("Must supply an enrollment callback"); } - if (cancel != null) { - if (cancel.isCanceled()) { - Slog.w(TAG, "enrollRemotely is already canceled."); - return; - } else { - cancel.setOnCancelListener(new OnEnrollCancelListener()); - } + if (cancel != null && cancel.isCanceled()) { + Slog.w(TAG, "enrollRemotely is already canceled."); + return; } if (mService != null) { try { mEnrollmentCallback = callback; Trace.beginSection("FaceManager#enrollRemotely"); - mService.enrollRemotely(userId, mToken, hardwareAuthToken, mServiceReceiver, - mContext.getOpPackageName(), disabledFeatures); + final long enrolId = mService.enrollRemotely(userId, mToken, hardwareAuthToken, + mServiceReceiver, mContext.getOpPackageName(), disabledFeatures); + if (cancel != null) { + cancel.setOnCancelListener(new OnEnrollCancelListener(enrolId)); + } } catch (RemoteException e) { Slog.w(TAG, "Remote exception in enrollRemotely: ", e); // Though this may not be a hardware issue, it will cause apps to give up or @@ -713,10 +711,10 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } } - private void cancelEnrollment() { + private void cancelEnrollment(long requestId) { if (mService != null) { try { - mService.cancelEnrollment(mToken); + mService.cancelEnrollment(mToken, requestId); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -1100,9 +1098,16 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } private class OnEnrollCancelListener implements OnCancelListener { + private final long mAuthRequestId; + + private OnEnrollCancelListener(long id) { + mAuthRequestId = id; + } + @Override public void onCancel() { - cancelEnrollment(); + Slog.d(TAG, "Cancel face enrollment requested for: " + mAuthRequestId); + cancelEnrollment(mAuthRequestId); } } diff --git a/core/java/android/hardware/face/IFaceService.aidl b/core/java/android/hardware/face/IFaceService.aidl index e9198246dee3f..989b001ca8bf0 100644 --- a/core/java/android/hardware/face/IFaceService.aidl +++ b/core/java/android/hardware/face/IFaceService.aidl @@ -76,15 +76,16 @@ interface IFaceService { void cancelAuthenticationFromService(int sensorId, IBinder token, String opPackageName, long requestId); // Start face enrollment - void enroll(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver, - String opPackageName, in int [] disabledFeatures, in Surface previewSurface, boolean debugConsent); + long enroll(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver, + String opPackageName, in int [] disabledFeatures, + in Surface previewSurface, boolean debugConsent); // Start remote face enrollment - void enrollRemotely(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver, + long enrollRemotely(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver, String opPackageName, in int [] disabledFeatures); // Cancel enrollment in progress - void cancelEnrollment(IBinder token); + void cancelEnrollment(IBinder token, long requestId); // Removes the specified face enrollment for the specified userId. void remove(IBinder token, int faceId, int userId, IFaceServiceReceiver receiver, diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index fe04e5d35784f..acf9427b12416 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -183,9 +183,16 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } private class OnEnrollCancelListener implements OnCancelListener { + private final long mAuthRequestId; + + private OnEnrollCancelListener(long id) { + mAuthRequestId = id; + } + @Override public void onCancel() { - cancelEnrollment(); + Slog.d(TAG, "Cancel fingerprint enrollment requested for: " + mAuthRequestId); + cancelEnrollment(mAuthRequestId); } } @@ -646,20 +653,19 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing throw new IllegalArgumentException("Must supply an enrollment callback"); } - if (cancel != null) { - if (cancel.isCanceled()) { - Slog.w(TAG, "enrollment already canceled"); - return; - } else { - cancel.setOnCancelListener(new OnEnrollCancelListener()); - } + if (cancel != null && cancel.isCanceled()) { + Slog.w(TAG, "enrollment already canceled"); + return; } if (mService != null) { try { mEnrollmentCallback = callback; - mService.enroll(mToken, hardwareAuthToken, userId, mServiceReceiver, - mContext.getOpPackageName(), enrollReason); + final long enrollId = mService.enroll(mToken, hardwareAuthToken, userId, + mServiceReceiver, mContext.getOpPackageName(), enrollReason); + if (cancel != null) { + cancel.setOnCancelListener(new OnEnrollCancelListener(enrollId)); + } } catch (RemoteException e) { Slog.w(TAG, "Remote exception in enroll: ", e); // Though this may not be a hardware issue, it will cause apps to give up or try @@ -1302,9 +1308,9 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing return allSensors.isEmpty() ? null : allSensors.get(0); } - private void cancelEnrollment() { + private void cancelEnrollment(long requestId) { if (mService != null) try { - mService.cancelEnrollment(mToken); + mService.cancelEnrollment(mToken, requestId); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/hardware/fingerprint/IFingerprintService.aidl b/core/java/android/hardware/fingerprint/IFingerprintService.aidl index ba1dc6da62a64..cbff8b11a72a6 100644 --- a/core/java/android/hardware/fingerprint/IFingerprintService.aidl +++ b/core/java/android/hardware/fingerprint/IFingerprintService.aidl @@ -84,11 +84,11 @@ interface IFingerprintService { void cancelAuthenticationFromService(int sensorId, IBinder token, String opPackageName, long requestId); // Start fingerprint enrollment - void enroll(IBinder token, in byte [] hardwareAuthToken, int userId, IFingerprintServiceReceiver receiver, + long enroll(IBinder token, in byte [] hardwareAuthToken, int userId, IFingerprintServiceReceiver receiver, String opPackageName, int enrollReason); // Cancel enrollment in progress - void cancelEnrollment(IBinder token); + void cancelEnrollment(IBinder token, long requestId); // Any errors resulting from this call will be returned to the listener void remove(IBinder token, int fingerId, int userId, IFingerprintServiceReceiver receiver, diff --git a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java b/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java index b73e91173a432..26bbb403f39f0 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java +++ b/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java @@ -16,6 +16,8 @@ package com.android.server.biometrics.sensors; +import static com.android.internal.annotations.VisibleForTesting.Visibility; + import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; @@ -48,7 +50,6 @@ public abstract class BaseClientMonitor extends LoggableMonitor * Interface that ClientMonitor holders should use to receive callbacks. */ public interface Callback { - /** * Invoked when the ClientMonitor operation has been started (e.g. reached the head of * the queue and becomes the current operation). @@ -203,7 +204,8 @@ public abstract class BaseClientMonitor extends LoggableMonitor } /** Signals this operation has completed its lifecycle and should no longer be used. */ - void destroy() { + @VisibleForTesting(visibility = Visibility.PACKAGE) + public void destroy() { mAlreadyDone = true; if (mToken != null) { try { diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java index a358bc2bad55e..1f91c4d6803e8 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -17,15 +17,14 @@ package com.android.server.biometrics.sensors; import android.annotation.IntDef; +import android.annotation.MainThread; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; -import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.IBiometricService; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; import android.os.Handler; import android.os.IBinder; -import android.os.Looper; import android.os.RemoteException; import android.os.ServiceManager; import android.util.Slog; @@ -55,6 +54,7 @@ import java.util.Locale; * We currently assume (and require) that each biometric sensor have its own instance of a * {@link BiometricScheduler}. See {@link CoexCoordinator}. */ +@MainThread public class BiometricScheduler { private static final String BASE_TAG = "BiometricScheduler"; @@ -110,123 +110,6 @@ public class BiometricScheduler { } } - /** - * Contains all the necessary information for a HAL operation. - */ - @VisibleForTesting - static final class Operation { - - /** - * The operation is added to the list of pending operations and waiting for its turn. - */ - static final int STATE_WAITING_IN_QUEUE = 0; - - /** - * The operation is added to the list of pending operations, but a subsequent operation - * has been added. This state only applies to {@link Interruptable} operations. When this - * operation reaches the head of the queue, it will send ERROR_CANCELED and finish. - */ - static final int STATE_WAITING_IN_QUEUE_CANCELING = 1; - - /** - * The operation has reached the front of the queue and has started. - */ - static final int STATE_STARTED = 2; - - /** - * The operation was started, but is now canceling. Operations should wait for the HAL to - * acknowledge that the operation was canceled, at which point it finishes. - */ - static final int STATE_STARTED_CANCELING = 3; - - /** - * The operation has reached the head of the queue but is waiting for BiometricService - * to acknowledge and start the operation. - */ - static final int STATE_WAITING_FOR_COOKIE = 4; - - /** - * The {@link BaseClientMonitor.Callback} has been invoked and the client is finished. - */ - static final int STATE_FINISHED = 5; - - @IntDef({STATE_WAITING_IN_QUEUE, - STATE_WAITING_IN_QUEUE_CANCELING, - STATE_STARTED, - STATE_STARTED_CANCELING, - STATE_WAITING_FOR_COOKIE, - STATE_FINISHED}) - @Retention(RetentionPolicy.SOURCE) - @interface OperationState {} - - @NonNull final BaseClientMonitor mClientMonitor; - @Nullable final BaseClientMonitor.Callback mClientCallback; - @OperationState int mState; - - Operation( - @NonNull BaseClientMonitor clientMonitor, - @Nullable BaseClientMonitor.Callback callback - ) { - this(clientMonitor, callback, STATE_WAITING_IN_QUEUE); - } - - protected Operation( - @NonNull BaseClientMonitor clientMonitor, - @Nullable BaseClientMonitor.Callback callback, - @OperationState int state - ) { - mClientMonitor = clientMonitor; - mClientCallback = callback; - mState = state; - } - - public boolean isHalOperation() { - return mClientMonitor instanceof HalClientMonitor; - } - - /** - * @return true if the operation requires the HAL, and the HAL is null. - */ - public boolean isUnstartableHalOperation() { - if (isHalOperation()) { - final HalClientMonitor client = (HalClientMonitor) mClientMonitor; - if (client.getFreshDaemon() == null) { - return true; - } - } - return false; - } - - @Override - public String toString() { - return mClientMonitor + ", State: " + mState; - } - } - - /** - * Monitors an operation's cancellation. If cancellation takes too long, the watchdog will - * kill the current operation and forcibly start the next. - */ - private static final class CancellationWatchdog implements Runnable { - static final int DELAY_MS = 3000; - - final String tag; - final Operation operation; - CancellationWatchdog(String tag, Operation operation) { - this.tag = tag; - this.operation = operation; - } - - @Override - public void run() { - if (operation.mState != Operation.STATE_FINISHED) { - Slog.e(tag, "[Watchdog Triggered]: " + operation); - operation.mClientMonitor.mCallback - .onClientFinished(operation.mClientMonitor, false /* success */); - } - } - } - private static final class CrashState { static final int NUM_ENTRIES = 10; final String timestamp; @@ -263,10 +146,9 @@ public class BiometricScheduler { private final @SensorType int mSensorType; @Nullable private final GestureAvailabilityDispatcher mGestureAvailabilityDispatcher; @NonNull private final IBiometricService mBiometricService; - @NonNull protected final Handler mHandler = new Handler(Looper.getMainLooper()); - @NonNull private final InternalCallback mInternalCallback; - @VisibleForTesting @NonNull final Deque mPendingOperations; - @VisibleForTesting @Nullable Operation mCurrentOperation; + @NonNull protected final Handler mHandler; + @VisibleForTesting @NonNull final Deque mPendingOperations; + @VisibleForTesting @Nullable BiometricSchedulerOperation mCurrentOperation; @NonNull private final ArrayDeque mCrashStates; private int mTotalOperationsHandled; @@ -277,7 +159,7 @@ public class BiometricScheduler { // Internal callback, notified when an operation is complete. Notifies the requester // that the operation is complete, before performing internal scheduler work (such as // starting the next client). - public class InternalCallback implements BaseClientMonitor.Callback { + private final BaseClientMonitor.Callback mInternalCallback = new BaseClientMonitor.Callback() { @Override public void onClientStarted(@NonNull BaseClientMonitor clientMonitor) { Slog.d(getTag(), "[Started] " + clientMonitor); @@ -286,16 +168,11 @@ public class BiometricScheduler { mCoexCoordinator.addAuthenticationClient(mSensorType, (AuthenticationClient) clientMonitor); } - - if (mCurrentOperation.mClientCallback != null) { - mCurrentOperation.mClientCallback.onClientStarted(clientMonitor); - } } @Override public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) { mHandler.post(() -> { - clientMonitor.destroy(); if (mCurrentOperation == null) { Slog.e(getTag(), "[Finishing] " + clientMonitor + " but current operation is null, success: " + success @@ -303,9 +180,9 @@ public class BiometricScheduler { return; } - if (clientMonitor != mCurrentOperation.mClientMonitor) { + if (!mCurrentOperation.isFor(clientMonitor)) { Slog.e(getTag(), "[Ignoring Finish] " + clientMonitor + " does not match" - + " current: " + mCurrentOperation.mClientMonitor); + + " current: " + mCurrentOperation); return; } @@ -315,36 +192,33 @@ public class BiometricScheduler { (AuthenticationClient) clientMonitor); } - mCurrentOperation.mState = Operation.STATE_FINISHED; - - if (mCurrentOperation.mClientCallback != null) { - mCurrentOperation.mClientCallback.onClientFinished(clientMonitor, success); - } - if (mGestureAvailabilityDispatcher != null) { mGestureAvailabilityDispatcher.markSensorActive( - mCurrentOperation.mClientMonitor.getSensorId(), false /* active */); + mCurrentOperation.getSensorId(), false /* active */); } if (mRecentOperations.size() >= mRecentOperationsLimit) { mRecentOperations.remove(0); } - mRecentOperations.add(mCurrentOperation.mClientMonitor.getProtoEnum()); + mRecentOperations.add(mCurrentOperation.getProtoEnum()); mCurrentOperation = null; mTotalOperationsHandled++; startNextOperationIfIdle(); }); } - } + }; @VisibleForTesting - BiometricScheduler(@NonNull String tag, @SensorType int sensorType, + BiometricScheduler(@NonNull String tag, + @NonNull Handler handler, + @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, - @NonNull IBiometricService biometricService, int recentOperationsLimit, + @NonNull IBiometricService biometricService, + int recentOperationsLimit, @NonNull CoexCoordinator coexCoordinator) { mBiometricTag = tag; + mHandler = handler; mSensorType = sensorType; - mInternalCallback = new InternalCallback(); mGestureAvailabilityDispatcher = gestureAvailabilityDispatcher; mPendingOperations = new ArrayDeque<>(); mBiometricService = biometricService; @@ -356,24 +230,26 @@ public class BiometricScheduler { /** * Creates a new scheduler. + * * @param tag for the specific instance of the scheduler. Should be unique. + * @param handler handler for callbacks (all methods of this class must be called on the + * thread associated with this handler) * @param sensorType the sensorType that this scheduler is handling. * @param gestureAvailabilityDispatcher may be null if the sensor does not support gestures * (such as fingerprint swipe). */ public BiometricScheduler(@NonNull String tag, + @NonNull Handler handler, @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - this(tag, sensorType, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface( - ServiceManager.getService(Context.BIOMETRIC_SERVICE)), LOG_NUM_RECENT_OPERATIONS, - CoexCoordinator.getInstance()); + this(tag, handler, sensorType, gestureAvailabilityDispatcher, + IBiometricService.Stub.asInterface( + ServiceManager.getService(Context.BIOMETRIC_SERVICE)), + LOG_NUM_RECENT_OPERATIONS, CoexCoordinator.getInstance()); } - /** - * @return A reference to the internal callback that should be invoked whenever the scheduler - * needs to (e.g. client started, client finished). - */ - @NonNull protected InternalCallback getInternalCallback() { + @VisibleForTesting + public BaseClientMonitor.Callback getInternalCallback() { return mInternalCallback; } @@ -392,72 +268,46 @@ public class BiometricScheduler { } mCurrentOperation = mPendingOperations.poll(); - final BaseClientMonitor currentClient = mCurrentOperation.mClientMonitor; Slog.d(getTag(), "[Polled] " + mCurrentOperation); // If the operation at the front of the queue has been marked for cancellation, send // ERROR_CANCELED. No need to start this client. - if (mCurrentOperation.mState == Operation.STATE_WAITING_IN_QUEUE_CANCELING) { + if (mCurrentOperation.isMarkedCanceling()) { Slog.d(getTag(), "[Now Cancelling] " + mCurrentOperation); - if (!(currentClient instanceof Interruptable)) { - throw new IllegalStateException("Mis-implemented client or scheduler, " - + "trying to cancel non-interruptable operation: " + mCurrentOperation); - } - - final Interruptable interruptable = (Interruptable) currentClient; - interruptable.cancelWithoutStarting(getInternalCallback()); + mCurrentOperation.cancel(mHandler, mInternalCallback); // Now we wait for the client to send its FinishCallback, which kicks off the next // operation. return; } - if (mGestureAvailabilityDispatcher != null - && mCurrentOperation.mClientMonitor instanceof AcquisitionClient) { + if (mGestureAvailabilityDispatcher != null && mCurrentOperation.isAcquisitionOperation()) { mGestureAvailabilityDispatcher.markSensorActive( - mCurrentOperation.mClientMonitor.getSensorId(), - true /* active */); + mCurrentOperation.getSensorId(), true /* active */); } // Not all operations start immediately. BiometricPrompt waits for its operation // to arrive at the head of the queue, before pinging it to start. - final boolean shouldStartNow = currentClient.getCookie() == 0; - if (shouldStartNow) { - if (mCurrentOperation.isUnstartableHalOperation()) { - final HalClientMonitor halClientMonitor = - (HalClientMonitor) mCurrentOperation.mClientMonitor; + final int cookie = mCurrentOperation.isReadyToStart(); + if (cookie == 0) { + if (!mCurrentOperation.start(mInternalCallback)) { // Note down current length of queue final int pendingOperationsLength = mPendingOperations.size(); - final Operation lastOperation = mPendingOperations.peekLast(); + final BiometricSchedulerOperation lastOperation = mPendingOperations.peekLast(); Slog.e(getTag(), "[Unable To Start] " + mCurrentOperation + ". Last pending operation: " + lastOperation); - // For current operations, 1) unableToStart, which notifies the caller-side, then - // 2) notify operation's callback, to notify applicable system service that the - // operation failed. - halClientMonitor.unableToStart(); - if (mCurrentOperation.mClientCallback != null) { - mCurrentOperation.mClientCallback.onClientFinished( - mCurrentOperation.mClientMonitor, false /* success */); - } - // Then for each operation currently in the pending queue at the time of this // failure, do the same as above. Otherwise, it's possible that something like // setActiveUser fails, but then authenticate (for the wrong user) is invoked. for (int i = 0; i < pendingOperationsLength; i++) { - final Operation operation = mPendingOperations.pollFirst(); - if (operation == null) { + final BiometricSchedulerOperation operation = mPendingOperations.pollFirst(); + if (operation != null) { + Slog.w(getTag(), "[Aborting Operation] " + operation); + operation.abort(); + } else { Slog.e(getTag(), "Null operation, index: " + i + ", expected length: " + pendingOperationsLength); - break; } - if (operation.isHalOperation()) { - ((HalClientMonitor) operation.mClientMonitor).unableToStart(); - } - if (operation.mClientCallback != null) { - operation.mClientCallback.onClientFinished(operation.mClientMonitor, - false /* success */); - } - Slog.w(getTag(), "[Aborted Operation] " + operation); } // It's possible that during cleanup a new set of operations came in. We can try to @@ -465,25 +315,20 @@ public class BiometricScheduler { // actually be multiple operations (i.e. updateActiveUser + authenticate). mCurrentOperation = null; startNextOperationIfIdle(); - } else { - Slog.d(getTag(), "[Starting] " + mCurrentOperation); - currentClient.start(getInternalCallback()); - mCurrentOperation.mState = Operation.STATE_STARTED; } } else { try { - mBiometricService.onReadyForAuthentication(currentClient.getCookie()); + mBiometricService.onReadyForAuthentication(cookie); } catch (RemoteException e) { Slog.e(getTag(), "Remote exception when contacting BiometricService", e); } Slog.d(getTag(), "Waiting for cookie before starting: " + mCurrentOperation); - mCurrentOperation.mState = Operation.STATE_WAITING_FOR_COOKIE; } } /** * Starts the {@link #mCurrentOperation} if - * 1) its state is {@link Operation#STATE_WAITING_FOR_COOKIE} and + * 1) its state is {@link BiometricSchedulerOperation#STATE_WAITING_FOR_COOKIE} and * 2) its cookie matches this cookie * * This is currently only used by {@link com.android.server.biometrics.BiometricService}, which @@ -499,45 +344,13 @@ public class BiometricScheduler { Slog.e(getTag(), "Current operation is null"); return; } - if (mCurrentOperation.mState != Operation.STATE_WAITING_FOR_COOKIE) { - if (mCurrentOperation.mState == Operation.STATE_WAITING_IN_QUEUE_CANCELING) { - Slog.d(getTag(), "Operation was marked for cancellation, cancelling now: " - + mCurrentOperation); - // This should trigger the internal onClientFinished callback, which clears the - // operation and starts the next one. - final ErrorConsumer errorConsumer = - (ErrorConsumer) mCurrentOperation.mClientMonitor; - errorConsumer.onError(BiometricConstants.BIOMETRIC_ERROR_CANCELED, - 0 /* vendorCode */); - return; - } else { - Slog.e(getTag(), "Operation is in the wrong state: " + mCurrentOperation - + ", expected STATE_WAITING_FOR_COOKIE"); - return; - } - } - if (mCurrentOperation.mClientMonitor.getCookie() != cookie) { - Slog.e(getTag(), "Mismatched cookie for operation: " + mCurrentOperation - + ", received: " + cookie); - return; - } - if (mCurrentOperation.isUnstartableHalOperation()) { + if (mCurrentOperation.startWithCookie(mInternalCallback, cookie)) { + Slog.d(getTag(), "[Started] Prepared client: " + mCurrentOperation); + } else { Slog.e(getTag(), "[Unable To Start] Prepared client: " + mCurrentOperation); - // This is BiometricPrompt trying to auth but something's wrong with the HAL. - final HalClientMonitor halClientMonitor = - (HalClientMonitor) mCurrentOperation.mClientMonitor; - halClientMonitor.unableToStart(); - if (mCurrentOperation.mClientCallback != null) { - mCurrentOperation.mClientCallback.onClientFinished(mCurrentOperation.mClientMonitor, - false /* success */); - } mCurrentOperation = null; startNextOperationIfIdle(); - } else { - Slog.d(getTag(), "[Starting] Prepared client: " + mCurrentOperation); - mCurrentOperation.mState = Operation.STATE_STARTED; - mCurrentOperation.mClientMonitor.start(getInternalCallback()); } } @@ -562,17 +375,13 @@ public class BiometricScheduler { // pending clients as canceling. Once they reach the head of the queue, the scheduler will // send ERROR_CANCELED and skip the operation. if (clientMonitor.interruptsPrecedingClients()) { - for (Operation operation : mPendingOperations) { - if (operation.mClientMonitor instanceof Interruptable - && operation.mState != Operation.STATE_WAITING_IN_QUEUE_CANCELING) { - Slog.d(getTag(), "New client incoming, marking pending client as canceling: " - + operation.mClientMonitor); - operation.mState = Operation.STATE_WAITING_IN_QUEUE_CANCELING; - } + for (BiometricSchedulerOperation operation : mPendingOperations) { + Slog.d(getTag(), "New client, marking pending op as canceling: " + operation); + operation.markCanceling(); } } - mPendingOperations.add(new Operation(clientMonitor, clientCallback)); + mPendingOperations.add(new BiometricSchedulerOperation(clientMonitor, clientCallback)); Slog.d(getTag(), "[Added] " + clientMonitor + ", new queue size: " + mPendingOperations.size()); @@ -580,67 +389,34 @@ public class BiometricScheduler { // cancellable, start the cancellation process. if (clientMonitor.interruptsPrecedingClients() && mCurrentOperation != null - && mCurrentOperation.mClientMonitor instanceof Interruptable - && mCurrentOperation.mState == Operation.STATE_STARTED) { + && mCurrentOperation.isInterruptable() + && mCurrentOperation.isStarted()) { Slog.d(getTag(), "[Cancelling Interruptable]: " + mCurrentOperation); - cancelInternal(mCurrentOperation); - } - - startNextOperationIfIdle(); - } - - private void cancelInternal(Operation operation) { - if (operation != mCurrentOperation) { - Slog.e(getTag(), "cancelInternal invoked on non-current operation: " + operation); - return; - } - if (!(operation.mClientMonitor instanceof Interruptable)) { - Slog.w(getTag(), "Operation not interruptable: " + operation); - return; - } - if (operation.mState == Operation.STATE_STARTED_CANCELING) { - Slog.w(getTag(), "Cancel already invoked for operation: " + operation); - return; - } - if (operation.mState == Operation.STATE_WAITING_FOR_COOKIE) { - Slog.w(getTag(), "Skipping cancellation for non-started operation: " + operation); - // We can set it to null immediately, since the HAL was never notified to start. - if (mCurrentOperation != null) { - mCurrentOperation.mClientMonitor.destroy(); - } - mCurrentOperation = null; + mCurrentOperation.cancel(mHandler, mInternalCallback); + } else { startNextOperationIfIdle(); - return; } - Slog.d(getTag(), "[Cancelling] Current client: " + operation.mClientMonitor); - final Interruptable interruptable = (Interruptable) operation.mClientMonitor; - interruptable.cancel(); - operation.mState = Operation.STATE_STARTED_CANCELING; - - // Add a watchdog. If the HAL does not acknowledge within the timeout, we will - // forcibly finish this client. - mHandler.postDelayed(new CancellationWatchdog(getTag(), operation), - CancellationWatchdog.DELAY_MS); } /** * Requests to cancel enrollment. * @param token from the caller, should match the token passed in when requesting enrollment */ - public void cancelEnrollment(IBinder token) { - if (mCurrentOperation == null) { - Slog.e(getTag(), "Unable to cancel enrollment, null operation"); - return; - } - final boolean isEnrolling = mCurrentOperation.mClientMonitor instanceof EnrollClient; - final boolean tokenMatches = mCurrentOperation.mClientMonitor.getToken() == token; - if (!isEnrolling || !tokenMatches) { - Slog.w(getTag(), "Not cancelling enrollment, isEnrolling: " + isEnrolling - + " tokenMatches: " + tokenMatches); - return; - } + public void cancelEnrollment(IBinder token, long requestId) { + Slog.d(getTag(), "cancelEnrollment, requestId: " + requestId); - cancelInternal(mCurrentOperation); + if (mCurrentOperation != null + && canCancelEnrollOperation(mCurrentOperation, token, requestId)) { + Slog.d(getTag(), "Cancelling enrollment op: " + mCurrentOperation); + mCurrentOperation.cancel(mHandler, mInternalCallback); + } else { + for (BiometricSchedulerOperation operation : mPendingOperations) { + if (canCancelEnrollOperation(operation, token, requestId)) { + Slog.d(getTag(), "Cancelling pending enrollment op: " + operation); + operation.markCanceling(); + } + } + } } /** @@ -649,62 +425,42 @@ public class BiometricScheduler { * @param requestId the id returned when requesting authentication */ public void cancelAuthenticationOrDetection(IBinder token, long requestId) { - Slog.d(getTag(), "cancelAuthenticationOrDetection, requestId: " + requestId - + " current: " + mCurrentOperation - + " stack size: " + mPendingOperations.size()); + Slog.d(getTag(), "cancelAuthenticationOrDetection, requestId: " + requestId); if (mCurrentOperation != null && canCancelAuthOperation(mCurrentOperation, token, requestId)) { - Slog.d(getTag(), "Cancelling: " + mCurrentOperation); - cancelInternal(mCurrentOperation); + Slog.d(getTag(), "Cancelling auth/detect op: " + mCurrentOperation); + mCurrentOperation.cancel(mHandler, mInternalCallback); } else { - // Look through the current queue for all authentication clients for the specified - // token, and mark them as STATE_WAITING_IN_QUEUE_CANCELING. Note that we're marking - // all of them, instead of just the first one, since the API surface currently doesn't - // allow us to distinguish between multiple authentication requests from the same - // process. However, this generally does not happen anyway, and would be a class of - // bugs on its own. - for (Operation operation : mPendingOperations) { + for (BiometricSchedulerOperation operation : mPendingOperations) { if (canCancelAuthOperation(operation, token, requestId)) { - Slog.d(getTag(), "Marking " + operation - + " as STATE_WAITING_IN_QUEUE_CANCELING"); - operation.mState = Operation.STATE_WAITING_IN_QUEUE_CANCELING; + Slog.d(getTag(), "Cancelling pending auth/detect op: " + operation); + operation.markCanceling(); } } } } - private static boolean canCancelAuthOperation(Operation operation, IBinder token, - long requestId) { + private static boolean canCancelEnrollOperation(BiometricSchedulerOperation operation, + IBinder token, long requestId) { + return operation.isEnrollOperation() + && operation.isMatchingToken(token) + && operation.isMatchingRequestId(requestId); + } + + private static boolean canCancelAuthOperation(BiometricSchedulerOperation operation, + IBinder token, long requestId) { // TODO: restrict callers that can cancel without requestId (negative value)? - return isAuthenticationOrDetectionOperation(operation) - && operation.mClientMonitor.getToken() == token - && isMatchingRequestId(operation, requestId); - } - - // By default, monitors are not associated with a request id to retain the original - // behavior (i.e. if no requestId is explicitly set then assume it matches) - private static boolean isMatchingRequestId(Operation operation, long requestId) { - return !operation.mClientMonitor.hasRequestId() - || operation.mClientMonitor.getRequestId() == requestId; - } - - private static boolean isAuthenticationOrDetectionOperation(@NonNull Operation operation) { - final boolean isAuthentication = - operation.mClientMonitor instanceof AuthenticationConsumer; - final boolean isDetection = - operation.mClientMonitor instanceof DetectionConsumer; - return isAuthentication || isDetection; + return operation.isAuthenticationOrDetectionOperation() + && operation.isMatchingToken(token) + && operation.isMatchingRequestId(requestId); } /** * @return the current operation */ public BaseClientMonitor getCurrentClient() { - if (mCurrentOperation == null) { - return null; - } - return mCurrentOperation.mClientMonitor; + return mCurrentOperation != null ? mCurrentOperation.getClientMonitor() : null; } public int getCurrentPendingCount() { @@ -719,7 +475,7 @@ public class BiometricScheduler { new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS", Locale.US); final String timestamp = dateFormat.format(new Date(System.currentTimeMillis())); final List pendingOperations = new ArrayList<>(); - for (Operation operation : mPendingOperations) { + for (BiometricSchedulerOperation operation : mPendingOperations) { pendingOperations.add(operation.toString()); } @@ -735,7 +491,7 @@ public class BiometricScheduler { pw.println("Type: " + mSensorType); pw.println("Current operation: " + mCurrentOperation); pw.println("Pending operations: " + mPendingOperations.size()); - for (Operation operation : mPendingOperations) { + for (BiometricSchedulerOperation operation : mPendingOperations) { pw.println("Pending operation: " + operation); } for (CrashState crashState : mCrashStates) { @@ -746,7 +502,7 @@ public class BiometricScheduler { public byte[] dumpProtoState(boolean clearSchedulerBuffer) { final ProtoOutputStream proto = new ProtoOutputStream(); proto.write(BiometricSchedulerProto.CURRENT_OPERATION, mCurrentOperation != null - ? mCurrentOperation.mClientMonitor.getProtoEnum() : BiometricsProto.CM_NONE); + ? mCurrentOperation.getProtoEnum() : BiometricsProto.CM_NONE); proto.write(BiometricSchedulerProto.TOTAL_OPERATIONS, mTotalOperationsHandled); if (!mRecentOperations.isEmpty()) { @@ -771,6 +527,7 @@ public class BiometricScheduler { * HAL dies. */ public void reset() { + Slog.d(getTag(), "Resetting scheduler"); mPendingOperations.clear(); mCurrentOperation = null; } diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java new file mode 100644 index 0000000000000..a8cce153dc706 --- /dev/null +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java @@ -0,0 +1,419 @@ +/* + * Copyright (C) 2021 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. + */ + +package com.android.server.biometrics.sensors; + +import android.annotation.IntDef; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.hardware.biometrics.BiometricConstants; +import android.os.Handler; +import android.os.IBinder; +import android.util.Slog; + +import com.android.internal.annotations.VisibleForTesting; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Contains all the necessary information for a HAL operation. + */ +public class BiometricSchedulerOperation { + protected static final String TAG = "BiometricSchedulerOperation"; + + /** + * The operation is added to the list of pending operations and waiting for its turn. + */ + protected static final int STATE_WAITING_IN_QUEUE = 0; + + /** + * The operation is added to the list of pending operations, but a subsequent operation + * has been added. This state only applies to {@link Interruptable} operations. When this + * operation reaches the head of the queue, it will send ERROR_CANCELED and finish. + */ + protected static final int STATE_WAITING_IN_QUEUE_CANCELING = 1; + + /** + * The operation has reached the front of the queue and has started. + */ + protected static final int STATE_STARTED = 2; + + /** + * The operation was started, but is now canceling. Operations should wait for the HAL to + * acknowledge that the operation was canceled, at which point it finishes. + */ + protected static final int STATE_STARTED_CANCELING = 3; + + /** + * The operation has reached the head of the queue but is waiting for BiometricService + * to acknowledge and start the operation. + */ + protected static final int STATE_WAITING_FOR_COOKIE = 4; + + /** + * The {@link BaseClientMonitor.Callback} has been invoked and the client is finished. + */ + protected static final int STATE_FINISHED = 5; + + @IntDef({STATE_WAITING_IN_QUEUE, + STATE_WAITING_IN_QUEUE_CANCELING, + STATE_STARTED, + STATE_STARTED_CANCELING, + STATE_WAITING_FOR_COOKIE, + STATE_FINISHED}) + @Retention(RetentionPolicy.SOURCE) + protected @interface OperationState {} + + private static final int CANCEL_WATCHDOG_DELAY_MS = 3000; + + @NonNull + private final BaseClientMonitor mClientMonitor; + @Nullable + private final BaseClientMonitor.Callback mClientCallback; + @OperationState + private int mState; + @VisibleForTesting + @NonNull + final Runnable mCancelWatchdog; + + BiometricSchedulerOperation( + @NonNull BaseClientMonitor clientMonitor, + @Nullable BaseClientMonitor.Callback callback + ) { + this(clientMonitor, callback, STATE_WAITING_IN_QUEUE); + } + + protected BiometricSchedulerOperation( + @NonNull BaseClientMonitor clientMonitor, + @Nullable BaseClientMonitor.Callback callback, + @OperationState int state + ) { + mClientMonitor = clientMonitor; + mClientCallback = callback; + mState = state; + mCancelWatchdog = () -> { + if (!isFinished()) { + Slog.e(TAG, "[Watchdog Triggered]: " + this); + getWrappedCallback().onClientFinished(mClientMonitor, false /* success */); + } + }; + } + + /** + * Zero if this operation is ready to start or has already started. A non-zero cookie + * is returned if the operation has not started and is waiting on + * {@link android.hardware.biometrics.IBiometricService#onReadyForAuthentication(int)}. + * + * @return cookie or 0 if ready/started + */ + public int isReadyToStart() { + if (mState == STATE_WAITING_FOR_COOKIE || mState == STATE_WAITING_IN_QUEUE) { + final int cookie = mClientMonitor.getCookie(); + if (cookie != 0) { + mState = STATE_WAITING_FOR_COOKIE; + } + return cookie; + } + + return 0; + } + + /** + * Start this operation without waiting for a cookie + * (i.e. {@link #isReadyToStart() returns zero} + * + * @param callback lifecycle callback + * @return if this operation started + */ + public boolean start(@NonNull BaseClientMonitor.Callback callback) { + checkInState("start", + STATE_WAITING_IN_QUEUE, + STATE_WAITING_FOR_COOKIE, + STATE_WAITING_IN_QUEUE_CANCELING); + + if (mClientMonitor.getCookie() != 0) { + throw new IllegalStateException("operation requires cookie"); + } + + return doStart(callback); + } + + /** + * Start this operation after receiving the given cookie. + * + * @param callback lifecycle callback + * @param cookie cookie indicting the operation should begin + * @return if this operation started + */ + public boolean startWithCookie(@NonNull BaseClientMonitor.Callback callback, int cookie) { + checkInState("start", + STATE_WAITING_IN_QUEUE, + STATE_WAITING_FOR_COOKIE, + STATE_WAITING_IN_QUEUE_CANCELING); + + if (mClientMonitor.getCookie() != cookie) { + Slog.e(TAG, "Mismatched cookie for operation: " + this + ", received: " + cookie); + return false; + } + + return doStart(callback); + } + + private boolean doStart(@NonNull BaseClientMonitor.Callback callback) { + final BaseClientMonitor.Callback cb = getWrappedCallback(callback); + + if (mState == STATE_WAITING_IN_QUEUE_CANCELING) { + Slog.d(TAG, "Operation marked for cancellation, cancelling now: " + this); + + cb.onClientFinished(mClientMonitor, true /* success */); + if (mClientMonitor instanceof ErrorConsumer) { + final ErrorConsumer errorConsumer = (ErrorConsumer) mClientMonitor; + errorConsumer.onError(BiometricConstants.BIOMETRIC_ERROR_CANCELED, + 0 /* vendorCode */); + } else { + Slog.w(TAG, "monitor cancelled but does not implement ErrorConsumer"); + } + + return false; + } + + if (isUnstartableHalOperation()) { + Slog.v(TAG, "unable to start: " + this); + ((HalClientMonitor) mClientMonitor).unableToStart(); + cb.onClientFinished(mClientMonitor, false /* success */); + return false; + } + + mState = STATE_STARTED; + mClientMonitor.start(cb); + + Slog.v(TAG, "started: " + this); + return true; + } + + /** + * Abort a pending operation. + * + * This is similar to cancel but the operation must not have been started. It will + * immediately abort the operation and notify the client that it has finished unsuccessfully. + */ + public void abort() { + checkInState("cannot abort a non-pending operation", + STATE_WAITING_IN_QUEUE, + STATE_WAITING_FOR_COOKIE, + STATE_WAITING_IN_QUEUE_CANCELING); + + if (isHalOperation()) { + ((HalClientMonitor) mClientMonitor).unableToStart(); + } + getWrappedCallback().onClientFinished(mClientMonitor, false /* success */); + + Slog.v(TAG, "Aborted: " + this); + } + + /** Flags this operation as canceled, but does not cancel it until started. */ + public void markCanceling() { + if (mState == STATE_WAITING_IN_QUEUE && isInterruptable()) { + mState = STATE_WAITING_IN_QUEUE_CANCELING; + Slog.v(TAG, "Marked cancelling: " + this); + } + } + + /** + * Cancel the operation now. + * + * @param handler handler to use for the cancellation watchdog + * @param callback lifecycle callback (only used if this operation hasn't started, otherwise + * the callback used from {@link #start(BaseClientMonitor.Callback)} is used) + */ + public void cancel(@NonNull Handler handler, @NonNull BaseClientMonitor.Callback callback) { + checkNotInState("cancel", STATE_FINISHED); + + final int currentState = mState; + if (!isInterruptable()) { + Slog.w(TAG, "Cannot cancel - operation not interruptable: " + this); + return; + } + if (currentState == STATE_STARTED_CANCELING) { + Slog.w(TAG, "Cannot cancel - already invoked for operation: " + this); + return; + } + + mState = STATE_STARTED_CANCELING; + if (currentState == STATE_WAITING_IN_QUEUE + || currentState == STATE_WAITING_IN_QUEUE_CANCELING + || currentState == STATE_WAITING_FOR_COOKIE) { + Slog.d(TAG, "[Cancelling] Current client (without start): " + mClientMonitor); + ((Interruptable) mClientMonitor).cancelWithoutStarting(getWrappedCallback(callback)); + } else { + Slog.d(TAG, "[Cancelling] Current client: " + mClientMonitor); + ((Interruptable) mClientMonitor).cancel(); + } + + // forcibly finish this client if the HAL does not acknowledge within the timeout + handler.postDelayed(mCancelWatchdog, CANCEL_WATCHDOG_DELAY_MS); + } + + @NonNull + private BaseClientMonitor.Callback getWrappedCallback() { + return getWrappedCallback(null); + } + + @NonNull + private BaseClientMonitor.Callback getWrappedCallback( + @Nullable BaseClientMonitor.Callback callback) { + final BaseClientMonitor.Callback destroyCallback = new BaseClientMonitor.Callback() { + @Override + public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, + boolean success) { + mClientMonitor.destroy(); + mState = STATE_FINISHED; + } + }; + return new BaseClientMonitor.CompositeCallback(destroyCallback, callback, mClientCallback); + } + + /** {@link BaseClientMonitor#getSensorId()}. */ + public int getSensorId() { + return mClientMonitor.getSensorId(); + } + + /** {@link BaseClientMonitor#getProtoEnum()}. */ + public int getProtoEnum() { + return mClientMonitor.getProtoEnum(); + } + + /** {@link BaseClientMonitor#getTargetUserId()}. */ + public int getTargetUserId() { + return mClientMonitor.getTargetUserId(); + } + + /** If the given clientMonitor is the same as the one in the constructor. */ + public boolean isFor(@NonNull BaseClientMonitor clientMonitor) { + return mClientMonitor == clientMonitor; + } + + /** If this operation is {@link Interruptable}. */ + public boolean isInterruptable() { + return mClientMonitor instanceof Interruptable; + } + + private boolean isHalOperation() { + return mClientMonitor instanceof HalClientMonitor; + } + + private boolean isUnstartableHalOperation() { + if (isHalOperation()) { + final HalClientMonitor client = (HalClientMonitor) mClientMonitor; + if (client.getFreshDaemon() == null) { + return true; + } + } + return false; + } + + /** If this operation is an enrollment. */ + public boolean isEnrollOperation() { + return mClientMonitor instanceof EnrollClient; + } + + /** If this operation is authentication. */ + public boolean isAuthenticateOperation() { + return mClientMonitor instanceof AuthenticationClient; + } + + /** If this operation is authentication or detection. */ + public boolean isAuthenticationOrDetectionOperation() { + final boolean isAuthentication = mClientMonitor instanceof AuthenticationConsumer; + final boolean isDetection = mClientMonitor instanceof DetectionConsumer; + return isAuthentication || isDetection; + } + + /** If this operation performs acquisition {@link AcquisitionClient}. */ + public boolean isAcquisitionOperation() { + return mClientMonitor instanceof AcquisitionClient; + } + + /** + * If this operation matches the original requestId. + * + * By default, monitors are not associated with a request id to retain the original + * behavior (i.e. if no requestId is explicitly set then assume it matches) + * + * @param requestId a unique id {@link BaseClientMonitor#setRequestId(long)}. + */ + public boolean isMatchingRequestId(long requestId) { + return !mClientMonitor.hasRequestId() + || mClientMonitor.getRequestId() == requestId; + } + + /** If the token matches */ + public boolean isMatchingToken(@Nullable IBinder token) { + return mClientMonitor.getToken() == token; + } + + /** If this operation has started. */ + public boolean isStarted() { + return mState == STATE_STARTED; + } + + /** If this operation is cancelling but has not yet completed. */ + public boolean isCanceling() { + return mState == STATE_STARTED_CANCELING; + } + + /** If this operation has finished and completed its lifecycle. */ + public boolean isFinished() { + return mState == STATE_FINISHED; + } + + /** If {@link #markCanceling()} was called but the operation hasn't been canceled. */ + public boolean isMarkedCanceling() { + return mState == STATE_WAITING_IN_QUEUE_CANCELING; + } + + /** + * The monitor passed to the constructor. + * @deprecated avoid using and move to encapsulate within the operation + */ + @Deprecated + public BaseClientMonitor getClientMonitor() { + return mClientMonitor; + } + + private void checkNotInState(String message, @OperationState int... states) { + for (int state : states) { + if (mState == state) { + throw new IllegalStateException(message + ": illegal state= " + state); + } + } + } + + private void checkInState(String message, @OperationState int... states) { + for (int state : states) { + if (mState == state) { + return; + } + } + throw new IllegalStateException(message + ": illegal state= " + mState); + } + + @Override + public String toString() { + return mClientMonitor + ", State: " + mState; + } +} diff --git a/services/core/java/com/android/server/biometrics/sensors/Interruptable.java b/services/core/java/com/android/server/biometrics/sensors/Interruptable.java index fab98b6581a3e..d5093c7564154 100644 --- a/services/core/java/com/android/server/biometrics/sensors/Interruptable.java +++ b/services/core/java/com/android/server/biometrics/sensors/Interruptable.java @@ -32,6 +32,11 @@ public interface Interruptable { * {@link BaseClientMonitor#start(BaseClientMonitor.Callback)} was invoked. This usually happens * if the client is still waiting in the pending queue and got notified that a subsequent * operation is preempting it. + * + * This method must invoke + * {@link BaseClientMonitor.Callback#onClientFinished(BaseClientMonitor, boolean)} on the + * given callback (with success). + * * @param callback invoked when the operation is completed. */ void cancelWithoutStarting(@NonNull BaseClientMonitor.Callback callback); diff --git a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java index b056bf897b5c5..19eaa178c7c9f 100644 --- a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java @@ -16,10 +16,13 @@ package com.android.server.biometrics.sensors; +import static com.android.server.biometrics.sensors.BiometricSchedulerOperation.STATE_STARTED; + import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; import android.hardware.biometrics.IBiometricService; +import android.os.Handler; import android.os.ServiceManager; import android.os.UserHandle; import android.util.Slog; @@ -68,9 +71,8 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { return; } - Slog.d(getTag(), "[Client finished] " - + clientMonitor + ", success: " + success); - if (mCurrentOperation != null && mCurrentOperation.mClientMonitor == mOwner) { + Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success); + if (mCurrentOperation != null && mCurrentOperation.isFor(mOwner)) { mCurrentOperation = null; startNextOperationIfIdle(); } else { @@ -83,26 +85,31 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { } @VisibleForTesting - UserAwareBiometricScheduler(@NonNull String tag, @SensorType int sensorType, + UserAwareBiometricScheduler(@NonNull String tag, + @NonNull Handler handler, + @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, @NonNull IBiometricService biometricService, @NonNull CurrentUserRetriever currentUserRetriever, @NonNull UserSwitchCallback userSwitchCallback, @NonNull CoexCoordinator coexCoordinator) { - super(tag, sensorType, gestureAvailabilityDispatcher, biometricService, + super(tag, handler, sensorType, gestureAvailabilityDispatcher, biometricService, LOG_NUM_RECENT_OPERATIONS, coexCoordinator); mCurrentUserRetriever = currentUserRetriever; mUserSwitchCallback = userSwitchCallback; } - public UserAwareBiometricScheduler(@NonNull String tag, @SensorType int sensorType, + public UserAwareBiometricScheduler(@NonNull String tag, + @NonNull Handler handler, + @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, @NonNull CurrentUserRetriever currentUserRetriever, @NonNull UserSwitchCallback userSwitchCallback) { - this(tag, sensorType, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface( - ServiceManager.getService(Context.BIOMETRIC_SERVICE)), currentUserRetriever, - userSwitchCallback, CoexCoordinator.getInstance()); + this(tag, handler, sensorType, gestureAvailabilityDispatcher, + IBiometricService.Stub.asInterface( + ServiceManager.getService(Context.BIOMETRIC_SERVICE)), + currentUserRetriever, userSwitchCallback, CoexCoordinator.getInstance()); } @Override @@ -122,7 +129,7 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { } final int currentUserId = mCurrentUserRetriever.getCurrentUserId(); - final int nextUserId = mPendingOperations.getFirst().mClientMonitor.getTargetUserId(); + final int nextUserId = mPendingOperations.getFirst().getTargetUserId(); if (nextUserId == currentUserId) { super.startNextOperationIfIdle(); @@ -133,8 +140,8 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { new ClientFinishedCallback(startClient); Slog.d(getTag(), "[Starting User] " + startClient); - mCurrentOperation = new Operation( - startClient, finishedCallback, Operation.STATE_STARTED); + mCurrentOperation = new BiometricSchedulerOperation( + startClient, finishedCallback, STATE_STARTED); startClient.start(finishedCallback); } else { if (mStopUserClient != null) { @@ -147,8 +154,8 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { Slog.d(getTag(), "[Stopping User] current: " + currentUserId + ", next: " + nextUserId + ". " + mStopUserClient); - mCurrentOperation = new Operation( - mStopUserClient, finishedCallback, Operation.STATE_STARTED); + mCurrentOperation = new BiometricSchedulerOperation( + mStopUserClient, finishedCallback, STATE_STARTED); mStopUserClient.start(finishedCallback); } } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java index 675ee545a14f5..039b08e805c16 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java @@ -213,7 +213,7 @@ public class FaceService extends SystemService { } @Override // Binder call - public void enroll(int userId, final IBinder token, final byte[] hardwareAuthToken, + public long enroll(int userId, final IBinder token, final byte[] hardwareAuthToken, final IFaceServiceReceiver receiver, final String opPackageName, final int[] disabledFeatures, Surface previewSurface, boolean debugConsent) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); @@ -221,23 +221,24 @@ public class FaceService extends SystemService { final Pair provider = getSingleProvider(); if (provider == null) { Slog.w(TAG, "Null provider for enroll"); - return; + return -1; } - provider.second.scheduleEnroll(provider.first, token, hardwareAuthToken, userId, + return provider.second.scheduleEnroll(provider.first, token, hardwareAuthToken, userId, receiver, opPackageName, disabledFeatures, previewSurface, debugConsent); } @Override // Binder call - public void enrollRemotely(int userId, final IBinder token, final byte[] hardwareAuthToken, + public long enrollRemotely(int userId, final IBinder token, final byte[] hardwareAuthToken, final IFaceServiceReceiver receiver, final String opPackageName, final int[] disabledFeatures) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); // TODO(b/145027036): Implement this. + return -1; } @Override // Binder call - public void cancelEnrollment(final IBinder token) { + public void cancelEnrollment(final IBinder token, long requestId) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); final Pair provider = getSingleProvider(); @@ -246,7 +247,7 @@ public class FaceService extends SystemService { return; } - provider.second.cancelEnrollment(provider.first, token); + provider.second.cancelEnrollment(provider.first, token, requestId); } @Override // Binder call @@ -624,7 +625,7 @@ public class FaceService extends SystemService { private void addHidlProviders(@NonNull List hidlSensors) { for (FaceSensorPropertiesInternal hidlSensor : hidlSensors) { mServiceProviders.add( - new Face10(getContext(), hidlSensor, mLockoutResetDispatcher)); + Face10.newInstance(getContext(), hidlSensor, mLockoutResetDispatcher)); } } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java b/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java index e099ba372b058..77e431c811923 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java @@ -94,12 +94,12 @@ public interface ServiceProvider { void scheduleRevokeChallenge(int sensorId, int userId, @NonNull IBinder token, @NonNull String opPackageName, long challenge); - void scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, + long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName, @NonNull int[] disabledFeatures, @Nullable Surface previewSurface, boolean debugConsent); - void cancelEnrollment(int sensorId, @NonNull IBinder token); + void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId); long scheduleFaceDetect(int sensorId, @NonNull IBinder token, int userId, @NonNull ClientMonitorCallbackConverter callback, @NonNull String opPackageName, diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java index a806277ed45e2..aae4fbe9b0d73 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java @@ -82,13 +82,14 @@ public class FaceEnrollClient extends EnrollClient { FaceEnrollClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId, - @NonNull byte[] hardwareAuthToken, @NonNull String opPackageName, + @NonNull byte[] hardwareAuthToken, @NonNull String opPackageName, long requestId, @NonNull BiometricUtils utils, @NonNull int[] disabledFeatures, int timeoutSec, @Nullable Surface previewSurface, int sensorId, int maxTemplatesPerUser, boolean debugConsent) { super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, opPackageName, utils, timeoutSec, BiometricsProtoEnums.MODALITY_FACE, sensorId, false /* shouldVibrate */); + setRequestId(requestId); mEnrollIgnoreList = getContext().getResources() .getIntArray(R.array.config_face_acquire_enroll_ignorelist); mEnrollIgnoreListVendor = getContext().getResources() diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java index 4bae7756abe00..ae507abea537d 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java @@ -327,17 +327,18 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { } @Override - public void scheduleEnroll(int sensorId, @NonNull IBinder token, + public long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName, @NonNull int[] disabledFeatures, @Nullable Surface previewSurface, boolean debugConsent) { + final long id = mRequestCounter.incrementAndGet(); mHandler.post(() -> { final int maxTemplatesPerUser = mSensors.get( sensorId).getSensorProperties().maxEnrollmentsPerUser; final FaceEnrollClient client = new FaceEnrollClient(mContext, mSensors.get(sensorId).getLazySession(), token, new ClientMonitorCallbackConverter(receiver), userId, hardwareAuthToken, - opPackageName, FaceUtils.getInstance(sensorId), disabledFeatures, + opPackageName, id, FaceUtils.getInstance(sensorId), disabledFeatures, ENROLL_TIMEOUT_SEC, previewSurface, sensorId, maxTemplatesPerUser, debugConsent); scheduleForSensor(sensorId, client, new BaseClientMonitor.Callback() { @@ -351,11 +352,13 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { } }); }); + return id; } @Override - public void cancelEnrollment(int sensorId, @NonNull IBinder token) { - mHandler.post(() -> mSensors.get(sensorId).getScheduler().cancelEnrollment(token)); + public void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId) { + mHandler.post(() -> + mSensors.get(sensorId).getScheduler().cancelEnrollment(token, requestId)); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java index 206b8f0779e8e..39270430c21d4 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java @@ -494,7 +494,7 @@ public class Sensor { mToken = new Binder(); mHandler = handler; mSensorProperties = sensorProperties; - mScheduler = new UserAwareBiometricScheduler(tag, + mScheduler = new UserAwareBiometricScheduler(tag, mHandler, BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityDispatcher */, () -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL, new UserAwareBiometricScheduler.UserSwitchCallback() { diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java index f4dcbbba21d73..493c0a05e3795 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java @@ -333,12 +333,13 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { Face10(@NonNull Context context, @NonNull FaceSensorPropertiesInternal sensorProps, @NonNull LockoutResetDispatcher lockoutResetDispatcher, + @NonNull Handler handler, @NonNull BiometricScheduler scheduler) { mSensorProperties = sensorProps; mContext = context; mSensorId = sensorProps.sensorId; mScheduler = scheduler; - mHandler = new Handler(Looper.getMainLooper()); + mHandler = handler; mUsageStats = new UsageStats(context); mAuthenticatorIds = new HashMap<>(); mLazyDaemon = Face10.this::getDaemon; @@ -357,10 +358,12 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { } } - public Face10(@NonNull Context context, @NonNull FaceSensorPropertiesInternal sensorProps, + public static Face10 newInstance(@NonNull Context context, + @NonNull FaceSensorPropertiesInternal sensorProps, @NonNull LockoutResetDispatcher lockoutResetDispatcher) { - this(context, sensorProps, lockoutResetDispatcher, - new BiometricScheduler(TAG, BiometricScheduler.SENSOR_TYPE_FACE, + final Handler handler = new Handler(Looper.getMainLooper()); + return new Face10(context, sensorProps, lockoutResetDispatcher, handler, + new BiometricScheduler(TAG, handler, BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityTracker */)); } @@ -573,10 +576,11 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { } @Override - public void scheduleEnroll(int sensorId, @NonNull IBinder token, + public long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName, @NonNull int[] disabledFeatures, @Nullable Surface previewSurface, boolean debugConsent) { + final long id = mRequestCounter.incrementAndGet(); mHandler.post(() -> { scheduleUpdateActiveUserWithoutHandler(userId); @@ -584,7 +588,7 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { final FaceEnrollClient client = new FaceEnrollClient(mContext, mLazyDaemon, token, new ClientMonitorCallbackConverter(receiver), userId, hardwareAuthToken, - opPackageName, FaceUtils.getLegacyInstance(mSensorId), disabledFeatures, + opPackageName, id, FaceUtils.getLegacyInstance(mSensorId), disabledFeatures, ENROLL_TIMEOUT_SEC, previewSurface, mSensorId); mScheduler.scheduleClientMonitor(client, new BaseClientMonitor.Callback() { @@ -598,13 +602,12 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { } }); }); + return id; } @Override - public void cancelEnrollment(int sensorId, @NonNull IBinder token) { - mHandler.post(() -> { - mScheduler.cancelEnrollment(token); - }); + public void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId) { + mHandler.post(() -> mScheduler.cancelEnrollment(token, requestId)); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java index 80828cced4e89..31e5c86103fbe 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java @@ -53,12 +53,13 @@ public class FaceEnrollClient extends EnrollClient { FaceEnrollClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId, - @NonNull byte[] hardwareAuthToken, @NonNull String owner, + @NonNull byte[] hardwareAuthToken, @NonNull String owner, long requestId, @NonNull BiometricUtils utils, @NonNull int[] disabledFeatures, int timeoutSec, @Nullable Surface previewSurface, int sensorId) { super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, owner, utils, timeoutSec, BiometricsProtoEnums.MODALITY_FACE, sensorId, false /* shouldVibrate */); + setRequestId(requestId); mDisabledFeatures = Arrays.copyOf(disabledFeatures, disabledFeatures.length); mEnrollIgnoreList = getContext().getResources() .getIntArray(R.array.config_face_acquire_enroll_ignorelist); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java index 3e70ee52ff1b5..6366e19ef1917 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java @@ -249,7 +249,7 @@ public class FingerprintService extends SystemService { } @Override // Binder call - public void enroll(final IBinder token, @NonNull final byte[] hardwareAuthToken, + public long enroll(final IBinder token, @NonNull final byte[] hardwareAuthToken, final int userId, final IFingerprintServiceReceiver receiver, final String opPackageName, @FingerprintManager.EnrollReason int enrollReason) { Utils.checkPermission(getContext(), MANAGE_FINGERPRINT); @@ -257,15 +257,15 @@ public class FingerprintService extends SystemService { final Pair provider = getSingleProvider(); if (provider == null) { Slog.w(TAG, "Null provider for enroll"); - return; + return -1; } - provider.second.scheduleEnroll(provider.first, token, hardwareAuthToken, userId, + return provider.second.scheduleEnroll(provider.first, token, hardwareAuthToken, userId, receiver, opPackageName, enrollReason); } @Override // Binder call - public void cancelEnrollment(final IBinder token) { + public void cancelEnrollment(final IBinder token, long requestId) { Utils.checkPermission(getContext(), MANAGE_FINGERPRINT); final Pair provider = getSingleProvider(); @@ -274,7 +274,7 @@ public class FingerprintService extends SystemService { return; } - provider.second.cancelEnrollment(provider.first, token); + provider.second.cancelEnrollment(provider.first, token, requestId); } @SuppressWarnings("deprecation") @@ -818,7 +818,7 @@ public class FingerprintService extends SystemService { mLockoutResetDispatcher, mGestureAvailabilityDispatcher); } else { fingerprint21 = Fingerprint21.newInstance(getContext(), - mFingerprintStateCallback, hidlSensor, + mFingerprintStateCallback, hidlSensor, mHandler, mLockoutResetDispatcher, mGestureAvailabilityDispatcher); } mServiceProviders.add(fingerprint21); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/ServiceProvider.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/ServiceProvider.java index 1772f814dd102..535705c63cab0 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/ServiceProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/ServiceProvider.java @@ -88,11 +88,11 @@ public interface ServiceProvider { /** * Schedules fingerprint enrollment. */ - void scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, + long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFingerprintServiceReceiver receiver, @NonNull String opPackageName, @FingerprintManager.EnrollReason int enrollReason); - void cancelEnrollment(int sensorId, @NonNull IBinder token); + void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId); long scheduleFingerDetect(int sensorId, @NonNull IBinder token, int userId, @NonNull ClientMonitorCallbackConverter callback, @NonNull String opPackageName, 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 ccb34aad3198d..67507ccbbbfef 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 @@ -57,7 +57,7 @@ class FingerprintEnrollClient extends EnrollClient implements Udfps { private boolean mIsPointerDown; FingerprintEnrollClient(@NonNull Context context, - @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, + @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, long requestId, @NonNull ClientMonitorCallbackConverter listener, int userId, @NonNull byte[] hardwareAuthToken, @NonNull String owner, @NonNull BiometricUtils utils, int sensorId, @@ -69,6 +69,7 @@ class FingerprintEnrollClient extends EnrollClient implements Udfps { super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, owner, utils, 0 /* timeoutSec */, BiometricsProtoEnums.MODALITY_FINGERPRINT, sensorId, !sensorProps.isAnyUdfpsType() /* shouldVibrate */); + setRequestId(requestId); mSensorProps = sensorProps; mSensorOverlays = new SensorOverlays(udfpsOverlayController, sidefpsController); mMaxTemplatesPerUser = maxTemplatesPerUser; 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 734b1737dfbcb..eb16c763dea6d 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 @@ -347,15 +347,16 @@ public class FingerprintProvider implements IBinder.DeathRecipient, ServiceProvi } @Override - public void scheduleEnroll(int sensorId, @NonNull IBinder token, + public long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFingerprintServiceReceiver receiver, @NonNull String opPackageName, @FingerprintManager.EnrollReason int enrollReason) { + final long id = mRequestCounter.incrementAndGet(); mHandler.post(() -> { final int maxTemplatesPerUser = mSensors.get(sensorId).getSensorProperties() .maxEnrollmentsPerUser; final FingerprintEnrollClient client = new FingerprintEnrollClient(mContext, - mSensors.get(sensorId).getLazySession(), token, + mSensors.get(sensorId).getLazySession(), token, id, new ClientMonitorCallbackConverter(receiver), userId, hardwareAuthToken, opPackageName, FingerprintUtils.getInstance(sensorId), sensorId, mSensors.get(sensorId).getSensorProperties(), @@ -378,11 +379,13 @@ public class FingerprintProvider implements IBinder.DeathRecipient, ServiceProvi } }); }); + return id; } @Override - public void cancelEnrollment(int sensorId, @NonNull IBinder token) { - mHandler.post(() -> mSensors.get(sensorId).getScheduler().cancelEnrollment(token)); + public void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId) { + mHandler.post(() -> + mSensors.get(sensorId).getScheduler().cancelEnrollment(token, requestId)); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java index 59e4b582ca84e..256761a61a72a 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java @@ -449,7 +449,7 @@ class Sensor { mHandler = handler; mSensorProperties = sensorProperties; mLockoutCache = new LockoutCache(); - mScheduler = new UserAwareBiometricScheduler(tag, + mScheduler = new UserAwareBiometricScheduler(tag, handler, BiometricScheduler.sensorTypeFromFingerprintProperties(mSensorProperties), gestureAvailabilityDispatcher, () -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL, diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java index 5f2f4cf6ef3c0..d352cda609e3d 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java @@ -42,7 +42,6 @@ import android.hardware.fingerprint.IUdfpsOverlayController; import android.os.Handler; import android.os.IBinder; import android.os.IHwBinder; -import android.os.Looper; import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; @@ -320,7 +319,8 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider Fingerprint21(@NonNull Context context, @NonNull FingerprintStateCallback fingerprintStateCallback, @NonNull FingerprintSensorPropertiesInternal sensorProps, - @NonNull BiometricScheduler scheduler, @NonNull Handler handler, + @NonNull BiometricScheduler scheduler, + @NonNull Handler handler, @NonNull LockoutResetDispatcher lockoutResetDispatcher, @NonNull HalResultController controller) { mContext = context; @@ -356,16 +356,15 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider public static Fingerprint21 newInstance(@NonNull Context context, @NonNull FingerprintStateCallback fingerprintStateCallback, @NonNull FingerprintSensorPropertiesInternal sensorProps, + @NonNull Handler handler, @NonNull LockoutResetDispatcher lockoutResetDispatcher, @NonNull GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - final Handler handler = new Handler(Looper.getMainLooper()); final BiometricScheduler scheduler = - new BiometricScheduler(TAG, + new BiometricScheduler(TAG, handler, BiometricScheduler.sensorTypeFromFingerprintProperties(sensorProps), gestureAvailabilityDispatcher); final HalResultController controller = new HalResultController(sensorProps.sensorId, - context, handler, - scheduler); + context, handler, scheduler); return new Fingerprint21(context, fingerprintStateCallback, sensorProps, scheduler, handler, lockoutResetDispatcher, controller); } @@ -558,18 +557,20 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider } @Override - public void scheduleEnroll(int sensorId, @NonNull IBinder token, + public long scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFingerprintServiceReceiver receiver, @NonNull String opPackageName, @FingerprintManager.EnrollReason int enrollReason) { + final long id = mRequestCounter.incrementAndGet(); mHandler.post(() -> { scheduleUpdateActiveUserWithoutHandler(userId); final FingerprintEnrollClient client = new FingerprintEnrollClient(mContext, - mLazyDaemon, token, new ClientMonitorCallbackConverter(receiver), userId, - hardwareAuthToken, opPackageName, FingerprintUtils.getLegacyInstance(mSensorId), - ENROLL_TIMEOUT_SEC, mSensorProperties.sensorId, mUdfpsOverlayController, - mSidefpsController, enrollReason); + mLazyDaemon, token, id, new ClientMonitorCallbackConverter(receiver), + userId, hardwareAuthToken, opPackageName, + FingerprintUtils.getLegacyInstance(mSensorId), ENROLL_TIMEOUT_SEC, + mSensorProperties.sensorId, mUdfpsOverlayController, mSidefpsController, + enrollReason); mScheduler.scheduleClientMonitor(client, new BaseClientMonitor.Callback() { @Override public void onClientStarted(@NonNull BaseClientMonitor clientMonitor) { @@ -588,13 +589,12 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider } }); }); + return id; } @Override - public void cancelEnrollment(int sensorId, @NonNull IBinder token) { - mHandler.post(() -> { - mScheduler.cancelEnrollment(token); - }); + public void cancelEnrollment(int sensorId, @NonNull IBinder token, long requestId) { + mHandler.post(() -> mScheduler.cancelEnrollment(token, requestId)); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java index dd68b4d37e2a4..20dab5552df98 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java @@ -26,7 +26,6 @@ import android.hardware.fingerprint.FingerprintManager.AuthenticationCallback; import android.hardware.fingerprint.FingerprintManager.AuthenticationResult; import android.hardware.fingerprint.FingerprintSensorProperties; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; -import android.hardware.fingerprint.FingerprintStateListener; import android.hardware.fingerprint.IUdfpsOverlayController; import android.os.Handler; import android.os.IBinder; @@ -135,43 +134,17 @@ public class Fingerprint21UdfpsMock extends Fingerprint21 implements TrustManage @NonNull private final RestartAuthRunnable mRestartAuthRunnable; private static class TestableBiometricScheduler extends BiometricScheduler { - @NonNull private final TestableInternalCallback mInternalCallback; @NonNull private Fingerprint21UdfpsMock mFingerprint21; - TestableBiometricScheduler(@NonNull String tag, + TestableBiometricScheduler(@NonNull String tag, @NonNull Handler handler, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - super(tag, BiometricScheduler.SENSOR_TYPE_FP_OTHER, + super(tag, handler, BiometricScheduler.SENSOR_TYPE_FP_OTHER, gestureAvailabilityDispatcher); - mInternalCallback = new TestableInternalCallback(); - } - - class TestableInternalCallback extends InternalCallback { - @Override - public void onClientStarted(BaseClientMonitor clientMonitor) { - super.onClientStarted(clientMonitor); - Slog.d(TAG, "Client started: " + clientMonitor); - mFingerprint21.setDebugMessage("Started: " + clientMonitor); - } - - @Override - public void onClientFinished(BaseClientMonitor clientMonitor, boolean success) { - super.onClientFinished(clientMonitor, success); - Slog.d(TAG, "Client finished: " + clientMonitor); - mFingerprint21.setDebugMessage("Finished: " + clientMonitor); - } } void init(@NonNull Fingerprint21UdfpsMock fingerprint21) { mFingerprint21 = fingerprint21; } - - /** - * Expose the internal finish callback so it can be used for testing - */ - @Override - @NonNull protected InternalCallback getInternalCallback() { - return mInternalCallback; - } } /** @@ -280,7 +253,7 @@ public class Fingerprint21UdfpsMock extends Fingerprint21 implements TrustManage final Handler handler = new Handler(Looper.getMainLooper()); final TestableBiometricScheduler scheduler = - new TestableBiometricScheduler(TAG, gestureAvailabilityDispatcher); + new TestableBiometricScheduler(TAG, handler, gestureAvailabilityDispatcher); final MockHalResultController controller = new MockHalResultController(sensorProps.sensorId, context, handler, scheduler); return new Fingerprint21UdfpsMock(context, fingerprintStateCallback, sensorProps, scheduler, diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintEnrollClient.java index 1ebf44ca707f9..cc50bdfb59aec 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintEnrollClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintEnrollClient.java @@ -55,7 +55,7 @@ public class FingerprintEnrollClient extends EnrollClient lazyDaemon, @NonNull IBinder token, - @NonNull ClientMonitorCallbackConverter listener, int userId, + long requestId, @NonNull ClientMonitorCallbackConverter listener, int userId, @NonNull byte[] hardwareAuthToken, @NonNull String owner, @NonNull BiometricUtils utils, int timeoutSec, int sensorId, @Nullable IUdfpsOverlayController udfpsOverlayController, @@ -64,6 +64,7 @@ public class FingerprintEnrollClient extends EnrollClient + extends HalClientMonitor implements Interruptable { + public InterruptableMonitor() { + super(null, null, null, null, 0, null, 0, 0, 0, 0, 0); + } + } + + @Mock + private InterruptableMonitor mClientMonitor; + @Mock + private BaseClientMonitor.Callback mClientCallback; + @Mock + private FakeHal mHal; + @Captor + ArgumentCaptor mStartCallback; + + private Handler mHandler; + private BiometricSchedulerOperation mOperation; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mHandler = new Handler(TestableLooper.get(this).getLooper()); + mOperation = new BiometricSchedulerOperation(mClientMonitor, mClientCallback); + } + + @Test + public void testStartWithCookie() { + final int cookie = 200; + when(mClientMonitor.getCookie()).thenReturn(cookie); + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + assertThat(mOperation.isReadyToStart()).isEqualTo(cookie); + assertThat(mOperation.isStarted()).isFalse(); + assertThat(mOperation.isCanceling()).isFalse(); + assertThat(mOperation.isFinished()).isFalse(); + + final boolean started = mOperation.startWithCookie( + mock(BaseClientMonitor.Callback.class), cookie); + + assertThat(started).isTrue(); + verify(mClientMonitor).start(mStartCallback.capture()); + mStartCallback.getValue().onClientStarted(mClientMonitor); + assertThat(mOperation.isStarted()).isTrue(); + } + + @Test + public void testNoStartWithoutCookie() { + final int goodCookie = 20; + final int badCookie = 22; + when(mClientMonitor.getCookie()).thenReturn(goodCookie); + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + assertThat(mOperation.isReadyToStart()).isEqualTo(goodCookie); + final boolean started = mOperation.startWithCookie( + mock(BaseClientMonitor.Callback.class), badCookie); + + assertThat(started).isFalse(); + assertThat(mOperation.isStarted()).isFalse(); + assertThat(mOperation.isCanceling()).isFalse(); + assertThat(mOperation.isFinished()).isFalse(); + } + + @Test + public void startsWhenReadyAndHalAvailable() { + when(mClientMonitor.getCookie()).thenReturn(0); + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + final BaseClientMonitor.Callback cb = mock(BaseClientMonitor.Callback.class); + mOperation.start(cb); + verify(mClientMonitor).start(mStartCallback.capture()); + mStartCallback.getValue().onClientStarted(mClientMonitor); + + assertThat(mOperation.isStarted()).isTrue(); + assertThat(mOperation.isCanceling()).isFalse(); + assertThat(mOperation.isFinished()).isFalse(); + + verify(mClientCallback).onClientStarted(eq(mClientMonitor)); + verify(cb).onClientStarted(eq(mClientMonitor)); + verify(mClientCallback, never()).onClientFinished(any(), anyBoolean()); + verify(cb, never()).onClientFinished(any(), anyBoolean()); + + mStartCallback.getValue().onClientFinished(mClientMonitor, true); + + assertThat(mOperation.isFinished()).isTrue(); + assertThat(mOperation.isCanceling()).isFalse(); + verify(mClientMonitor).destroy(); + verify(cb).onClientFinished(eq(mClientMonitor), eq(true)); + } + + @Test + public void startFailsWhenReadyButHalNotAvailable() { + when(mClientMonitor.getCookie()).thenReturn(0); + when(mClientMonitor.getFreshDaemon()).thenReturn(null); + + final BaseClientMonitor.Callback cb = mock(BaseClientMonitor.Callback.class); + mOperation.start(cb); + verify(mClientMonitor, never()).start(any()); + + assertThat(mOperation.isStarted()).isFalse(); + assertThat(mOperation.isCanceling()).isFalse(); + assertThat(mOperation.isFinished()).isTrue(); + + verify(mClientCallback, never()).onClientStarted(eq(mClientMonitor)); + verify(cb, never()).onClientStarted(eq(mClientMonitor)); + verify(mClientCallback).onClientFinished(eq(mClientMonitor), eq(false)); + verify(cb).onClientFinished(eq(mClientMonitor), eq(false)); + } + + @Test + public void doesNotStartWithCookie() { + when(mClientMonitor.getCookie()).thenReturn(9); + assertThrows(IllegalStateException.class, + () -> mOperation.start(mock(BaseClientMonitor.Callback.class))); + } + + @Test + public void cannotRestart() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + mOperation.start(mock(BaseClientMonitor.Callback.class)); + + assertThrows(IllegalStateException.class, + () -> mOperation.start(mock(BaseClientMonitor.Callback.class))); + } + + @Test + public void abortsNotRunning() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + mOperation.abort(); + + assertThat(mOperation.isFinished()).isTrue(); + verify(mClientMonitor).unableToStart(); + verify(mClientMonitor).destroy(); + assertThrows(IllegalStateException.class, + () -> mOperation.start(mock(BaseClientMonitor.Callback.class))); + } + + @Test + public void cannotAbortRunning() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + mOperation.start(mock(BaseClientMonitor.Callback.class)); + + assertThrows(IllegalStateException.class, () -> mOperation.abort()); + } + + @Test + public void cancel() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + final BaseClientMonitor.Callback startCb = mock(BaseClientMonitor.Callback.class); + final BaseClientMonitor.Callback cancelCb = mock(BaseClientMonitor.Callback.class); + mOperation.start(startCb); + verify(mClientMonitor).start(mStartCallback.capture()); + mStartCallback.getValue().onClientStarted(mClientMonitor); + mOperation.cancel(mHandler, cancelCb); + + assertThat(mOperation.isCanceling()).isTrue(); + verify(mClientMonitor).cancel(); + verify(mClientMonitor, never()).cancelWithoutStarting(any()); + verify(mClientMonitor, never()).destroy(); + + mStartCallback.getValue().onClientFinished(mClientMonitor, true); + + assertThat(mOperation.isFinished()).isTrue(); + assertThat(mOperation.isCanceling()).isFalse(); + verify(mClientMonitor).destroy(); + + // should be unused since the operation was started + verify(cancelCb, never()).onClientStarted(any()); + verify(cancelCb, never()).onClientFinished(any(), anyBoolean()); + } + + @Test + public void cancelWithoutStarting() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + final BaseClientMonitor.Callback cancelCb = mock(BaseClientMonitor.Callback.class); + mOperation.cancel(mHandler, cancelCb); + + assertThat(mOperation.isCanceling()).isTrue(); + ArgumentCaptor cbCaptor = + ArgumentCaptor.forClass(BaseClientMonitor.Callback.class); + verify(mClientMonitor).cancelWithoutStarting(cbCaptor.capture()); + + cbCaptor.getValue().onClientFinished(mClientMonitor, true); + verify(cancelCb).onClientFinished(eq(mClientMonitor), eq(true)); + verify(mClientMonitor, never()).start(any()); + verify(mClientMonitor, never()).cancel(); + verify(mClientMonitor).destroy(); + } + + @Test + public void markCanceling() { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + mOperation.markCanceling(); + + assertThat(mOperation.isMarkedCanceling()).isTrue(); + assertThat(mOperation.isCanceling()).isFalse(); + assertThat(mOperation.isFinished()).isFalse(); + verify(mClientMonitor, never()).start(any()); + verify(mClientMonitor, never()).cancel(); + verify(mClientMonitor, never()).cancelWithoutStarting(any()); + verify(mClientMonitor, never()).unableToStart(); + verify(mClientMonitor, never()).destroy(); + } + + @Test + public void cancelPendingWithCookie() { + markCancellingAndStart(2); + } + + @Test + public void cancelPendingWithoutCookie() { + markCancellingAndStart(null); + } + + private void markCancellingAndStart(Integer withCookie) { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + if (withCookie != null) { + when(mClientMonitor.getCookie()).thenReturn(withCookie); + } + + mOperation.markCanceling(); + final BaseClientMonitor.Callback cb = mock(BaseClientMonitor.Callback.class); + if (withCookie != null) { + mOperation.startWithCookie(cb, withCookie); + } else { + mOperation.start(cb); + } + + assertThat(mOperation.isFinished()).isTrue(); + verify(cb).onClientFinished(eq(mClientMonitor), eq(true)); + verify(mClientMonitor, never()).start(any()); + verify(mClientMonitor, never()).cancel(); + verify(mClientMonitor, never()).cancelWithoutStarting(any()); + verify(mClientMonitor, never()).unableToStart(); + verify(mClientMonitor).destroy(); + } + + @Test + public void cancelWatchdogWhenStarted() { + cancelWatchdog(true); + } + + @Test + public void cancelWatchdogWithoutStarting() { + cancelWatchdog(false); + } + + private void cancelWatchdog(boolean start) { + when(mClientMonitor.getFreshDaemon()).thenReturn(mHal); + + mOperation.start(mock(BaseClientMonitor.Callback.class)); + if (start) { + verify(mClientMonitor).start(mStartCallback.capture()); + mStartCallback.getValue().onClientStarted(mClientMonitor); + } + mOperation.cancel(mHandler, mock(BaseClientMonitor.Callback.class)); + + assertThat(mOperation.isCanceling()).isTrue(); + + // omit call to onClientFinished and trigger watchdog + mOperation.mCancelWatchdog.run(); + + assertThat(mOperation.isFinished()).isTrue(); + assertThat(mOperation.isCanceling()).isFalse(); + verify(mClientMonitor).destroy(); + } +} 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 d192697827f6b..ac0831983262c 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 @@ -16,10 +16,14 @@ package com.android.server.biometrics.sensors; +import static android.testing.TestableLooper.RunWithLooper; + import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.fail; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; @@ -34,10 +38,13 @@ import android.content.Context; import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.IBiometricService; import android.os.Binder; +import android.os.Handler; import android.os.IBinder; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; +import android.testing.AndroidTestingRunner; import android.testing.TestableContext; +import android.testing.TestableLooper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -46,16 +53,18 @@ import androidx.test.filters.SmallTest; import com.android.server.biometrics.nano.BiometricSchedulerProto; import com.android.server.biometrics.nano.BiometricsProto; -import com.android.server.biometrics.sensors.BiometricScheduler.Operation; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @Presubmit @SmallTest +@RunWith(AndroidTestingRunner.class) +@RunWithLooper(setAsMainLooper = true) public class BiometricSchedulerTest { private static final String TAG = "BiometricSchedulerTest"; @@ -76,8 +85,9 @@ public class BiometricSchedulerTest { public void setUp() { MockitoAnnotations.initMocks(this); mToken = new Binder(); - mScheduler = new BiometricScheduler(TAG, BiometricScheduler.SENSOR_TYPE_UNKNOWN, - null /* gestureAvailabilityTracker */, mBiometricService, LOG_NUM_RECENT_OPERATIONS, + mScheduler = new BiometricScheduler(TAG, new Handler(TestableLooper.get(this).getLooper()), + BiometricScheduler.SENSOR_TYPE_UNKNOWN, null /* gestureAvailabilityTracker */, + mBiometricService, LOG_NUM_RECENT_OPERATIONS, CoexCoordinator.getInstance()); } @@ -86,9 +96,9 @@ public class BiometricSchedulerTest { final HalClientMonitor.LazyDaemon nonNullDaemon = () -> mock(Object.class); final HalClientMonitor client1 = - new TestClientMonitor(mContext, mToken, nonNullDaemon); + new TestHalClientMonitor(mContext, mToken, nonNullDaemon); final HalClientMonitor client2 = - new TestClientMonitor(mContext, mToken, nonNullDaemon); + new TestHalClientMonitor(mContext, mToken, nonNullDaemon); mScheduler.scheduleClientMonitor(client1); mScheduler.scheduleClientMonitor(client2); @@ -99,20 +109,17 @@ public class BiometricSchedulerTest { @Test public void testRemovesPendingOperations_whenNullHal_andNotBiometricPrompt() { // Even if second client has a non-null daemon, it needs to be canceled. - Object daemon2 = mock(Object.class); - - final HalClientMonitor.LazyDaemon lazyDaemon1 = () -> null; - final HalClientMonitor.LazyDaemon lazyDaemon2 = () -> daemon2; - - final TestClientMonitor client1 = new TestClientMonitor(mContext, mToken, lazyDaemon1); - final TestClientMonitor client2 = new TestClientMonitor(mContext, mToken, lazyDaemon2); + final TestHalClientMonitor client1 = new TestHalClientMonitor( + mContext, mToken, () -> null); + final TestHalClientMonitor client2 = new TestHalClientMonitor( + mContext, mToken, () -> mock(Object.class)); final BaseClientMonitor.Callback callback1 = mock(BaseClientMonitor.Callback.class); final BaseClientMonitor.Callback callback2 = mock(BaseClientMonitor.Callback.class); // Pretend the scheduler is busy so the first operation doesn't start right away. We want // to pretend like there are two operations in the queue before kicking things off - mScheduler.mCurrentOperation = new BiometricScheduler.Operation( + mScheduler.mCurrentOperation = new BiometricSchedulerOperation( mock(BaseClientMonitor.class), mock(BaseClientMonitor.Callback.class)); mScheduler.scheduleClientMonitor(client1, callback1); @@ -122,11 +129,11 @@ public class BiometricSchedulerTest { mScheduler.scheduleClientMonitor(client2, callback2); waitForIdle(); - assertTrue(client1.wasUnableToStart()); + assertTrue(client1.mUnableToStart); verify(callback1).onClientFinished(eq(client1), eq(false) /* success */); verify(callback1, never()).onClientStarted(any()); - assertTrue(client2.wasUnableToStart()); + assertTrue(client2.mUnableToStart); verify(callback2).onClientFinished(eq(client2), eq(false) /* success */); verify(callback2, never()).onClientStarted(any()); @@ -138,21 +145,19 @@ public class BiometricSchedulerTest { // Second non-BiometricPrompt client has a valid daemon final Object daemon2 = mock(Object.class); - final HalClientMonitor.LazyDaemon lazyDaemon1 = () -> null; - final HalClientMonitor.LazyDaemon lazyDaemon2 = () -> daemon2; - final ClientMonitorCallbackConverter listener1 = mock(ClientMonitorCallbackConverter.class); final TestAuthenticationClient client1 = - new TestAuthenticationClient(mContext, lazyDaemon1, mToken, listener1); - final TestClientMonitor client2 = new TestClientMonitor(mContext, mToken, lazyDaemon2); + new TestAuthenticationClient(mContext, () -> null, mToken, listener1); + final TestHalClientMonitor client2 = + new TestHalClientMonitor(mContext, mToken, () -> daemon2); final BaseClientMonitor.Callback callback1 = mock(BaseClientMonitor.Callback.class); final BaseClientMonitor.Callback callback2 = mock(BaseClientMonitor.Callback.class); // Pretend the scheduler is busy so the first operation doesn't start right away. We want // to pretend like there are two operations in the queue before kicking things off - mScheduler.mCurrentOperation = new BiometricScheduler.Operation( + mScheduler.mCurrentOperation = new BiometricSchedulerOperation( mock(BaseClientMonitor.class), mock(BaseClientMonitor.Callback.class)); mScheduler.scheduleClientMonitor(client1, callback1); @@ -172,8 +177,8 @@ public class BiometricSchedulerTest { verify(callback1, never()).onClientStarted(any()); // Client 2 was able to start - assertFalse(client2.wasUnableToStart()); - assertTrue(client2.hasStarted()); + assertFalse(client2.mUnableToStart); + assertTrue(client2.mStarted); verify(callback2).onClientStarted(eq(client2)); } @@ -187,16 +192,18 @@ public class BiometricSchedulerTest { // Schedule a BiometricPrompt authentication request mScheduler.scheduleClientMonitor(client1, callback1); - assertEquals(Operation.STATE_WAITING_FOR_COOKIE, mScheduler.mCurrentOperation.mState); - assertEquals(client1, mScheduler.mCurrentOperation.mClientMonitor); + assertNotEquals(0, mScheduler.mCurrentOperation.isReadyToStart()); + assertEquals(client1, mScheduler.mCurrentOperation.getClientMonitor()); assertEquals(0, mScheduler.mPendingOperations.size()); // Request it to be canceled. The operation can be canceled immediately, and the scheduler // should go back to idle, since in this case the framework has not even requested the HAL // to authenticate yet. mScheduler.cancelAuthenticationOrDetection(mToken, 1 /* requestId */); + waitForIdle(); assertTrue(client1.isAlreadyDone()); assertTrue(client1.mDestroyed); + assertFalse(client1.mStartedHal); assertNull(mScheduler.mCurrentOperation); } @@ -210,8 +217,8 @@ public class BiometricSchedulerTest { // assertEquals(0, bsp.recentOperations.length); // Pretend the scheduler is busy enrolling, and check the proto dump again. - final TestClientMonitor2 client = new TestClientMonitor2(mContext, mToken, - () -> mock(Object.class), BiometricsProto.CM_ENROLL); + final TestHalClientMonitor client = new TestHalClientMonitor(mContext, mToken, + () -> mock(Object.class), 0, BiometricsProto.CM_ENROLL); mScheduler.scheduleClientMonitor(client); waitForIdle(); bsp = getDump(true /* clearSchedulerBuffer */); @@ -230,8 +237,8 @@ public class BiometricSchedulerTest { @Test public void testProtoDump_fifo() throws Exception { // Add the first operation - final TestClientMonitor2 client = new TestClientMonitor2(mContext, mToken, - () -> mock(Object.class), BiometricsProto.CM_ENROLL); + final TestHalClientMonitor client = new TestHalClientMonitor(mContext, mToken, + () -> mock(Object.class), 0, BiometricsProto.CM_ENROLL); mScheduler.scheduleClientMonitor(client); waitForIdle(); BiometricSchedulerProto bsp = getDump(false /* clearSchedulerBuffer */); @@ -244,8 +251,8 @@ public class BiometricSchedulerTest { client.getCallback().onClientFinished(client, true); // Add another operation - final TestClientMonitor2 client2 = new TestClientMonitor2(mContext, mToken, - () -> mock(Object.class), BiometricsProto.CM_REMOVE); + final TestHalClientMonitor client2 = new TestHalClientMonitor(mContext, mToken, + () -> mock(Object.class), 0, BiometricsProto.CM_REMOVE); mScheduler.scheduleClientMonitor(client2); waitForIdle(); bsp = getDump(false /* clearSchedulerBuffer */); @@ -256,8 +263,8 @@ public class BiometricSchedulerTest { client2.getCallback().onClientFinished(client2, true); // And another operation - final TestClientMonitor2 client3 = new TestClientMonitor2(mContext, mToken, - () -> mock(Object.class), BiometricsProto.CM_AUTHENTICATE); + final TestHalClientMonitor client3 = new TestHalClientMonitor(mContext, mToken, + () -> mock(Object.class), 0, BiometricsProto.CM_AUTHENTICATE); mScheduler.scheduleClientMonitor(client3); waitForIdle(); bsp = getDump(false /* clearSchedulerBuffer */); @@ -290,8 +297,7 @@ public class BiometricSchedulerTest { @Test public void testCancelPendingAuth() throws RemoteException { final HalClientMonitor.LazyDaemon lazyDaemon = () -> mock(Object.class); - - final TestClientMonitor client1 = new TestClientMonitor(mContext, mToken, lazyDaemon); + final TestHalClientMonitor client1 = new TestHalClientMonitor(mContext, mToken, lazyDaemon); final ClientMonitorCallbackConverter callback = mock(ClientMonitorCallbackConverter.class); final TestAuthenticationClient client2 = new TestAuthenticationClient(mContext, lazyDaemon, mToken, callback); @@ -302,14 +308,12 @@ public class BiometricSchedulerTest { waitForIdle(); assertEquals(mScheduler.getCurrentClient(), client1); - assertEquals(Operation.STATE_WAITING_IN_QUEUE, - mScheduler.mPendingOperations.getFirst().mState); + assertFalse(mScheduler.mPendingOperations.getFirst().isStarted()); // Request cancel before the authentication client has started mScheduler.cancelAuthenticationOrDetection(mToken, 1 /* requestId */); waitForIdle(); - assertEquals(Operation.STATE_WAITING_IN_QUEUE_CANCELING, - mScheduler.mPendingOperations.getFirst().mState); + assertTrue(mScheduler.mPendingOperations.getFirst().isMarkedCanceling()); // Finish the blocking client. The authentication client should send ERROR_CANCELED client1.getCallback().onClientFinished(client1, true /* success */); @@ -326,67 +330,109 @@ public class BiometricSchedulerTest { @Test public void testCancels_whenAuthRequestIdNotSet() { - testCancelsWhenRequestId(null /* requestId */, 2, true /* started */); + testCancelsAuthDetectWhenRequestId(null /* requestId */, 2, true /* started */); } @Test public void testCancels_whenAuthRequestIdNotSet_notStarted() { - testCancelsWhenRequestId(null /* requestId */, 2, false /* started */); + testCancelsAuthDetectWhenRequestId(null /* requestId */, 2, false /* started */); } @Test public void testCancels_whenAuthRequestIdMatches() { - testCancelsWhenRequestId(200L, 200, true /* started */); + testCancelsAuthDetectWhenRequestId(200L, 200, true /* started */); } @Test public void testCancels_whenAuthRequestIdMatches_noStarted() { - testCancelsWhenRequestId(200L, 200, false /* started */); + testCancelsAuthDetectWhenRequestId(200L, 200, false /* started */); } @Test public void testDoesNotCancel_whenAuthRequestIdMismatched() { - testCancelsWhenRequestId(10L, 20, true /* started */); + testCancelsAuthDetectWhenRequestId(10L, 20, true /* started */); } @Test public void testDoesNotCancel_whenAuthRequestIdMismatched_notStarted() { - testCancelsWhenRequestId(10L, 20, false /* started */); + testCancelsAuthDetectWhenRequestId(10L, 20, false /* started */); + } + + private void testCancelsAuthDetectWhenRequestId(@Nullable Long requestId, long cancelRequestId, + boolean started) { + final HalClientMonitor.LazyDaemon lazyDaemon = () -> mock(Object.class); + final ClientMonitorCallbackConverter callback = mock(ClientMonitorCallbackConverter.class); + testCancelsWhenRequestId(requestId, cancelRequestId, started, + new TestAuthenticationClient(mContext, lazyDaemon, mToken, callback)); + } + + @Test + public void testCancels_whenEnrollRequestIdNotSet() { + testCancelsEnrollWhenRequestId(null /* requestId */, 2, false /* started */); + } + + @Test + public void testCancels_whenEnrollRequestIdMatches() { + testCancelsEnrollWhenRequestId(200L, 200, false /* started */); + } + + @Test + public void testDoesNotCancel_whenEnrollRequestIdMismatched() { + testCancelsEnrollWhenRequestId(10L, 20, false /* started */); + } + + private void testCancelsEnrollWhenRequestId(@Nullable Long requestId, long cancelRequestId, + boolean started) { + final HalClientMonitor.LazyDaemon lazyDaemon = () -> mock(Object.class); + final ClientMonitorCallbackConverter callback = mock(ClientMonitorCallbackConverter.class); + testCancelsWhenRequestId(requestId, cancelRequestId, started, + new TestEnrollClient(mContext, lazyDaemon, mToken, callback)); } private void testCancelsWhenRequestId(@Nullable Long requestId, long cancelRequestId, - boolean started) { + boolean started, HalClientMonitor client) { final boolean matches = requestId == null || requestId == cancelRequestId; - final HalClientMonitor.LazyDaemon lazyDaemon = () -> mock(Object.class); - final ClientMonitorCallbackConverter callback = mock(ClientMonitorCallbackConverter.class); - final TestAuthenticationClient client = new TestAuthenticationClient( - mContext, lazyDaemon, mToken, callback); if (requestId != null) { client.setRequestId(requestId); } + final boolean isAuth = client instanceof TestAuthenticationClient; + final boolean isEnroll = client instanceof TestEnrollClient; + mScheduler.scheduleClientMonitor(client); if (started) { mScheduler.startPreparedClient(client.getCookie()); } waitForIdle(); - mScheduler.cancelAuthenticationOrDetection(mToken, cancelRequestId); + if (isAuth) { + mScheduler.cancelAuthenticationOrDetection(mToken, cancelRequestId); + } else if (isEnroll) { + mScheduler.cancelEnrollment(mToken, cancelRequestId); + } else { + fail("unexpected operation type"); + } waitForIdle(); - assertEquals(matches && started ? 1 : 0, client.mNumCancels); + if (isAuth) { + // auth clients that were waiting for cookie when canceled should never invoke the hal + final TestAuthenticationClient authClient = (TestAuthenticationClient) client; + assertEquals(matches && started ? 1 : 0, authClient.mNumCancels); + assertEquals(started, authClient.mStartedHal); + } else if (isEnroll) { + final TestEnrollClient enrollClient = (TestEnrollClient) client; + assertEquals(matches ? 1 : 0, enrollClient.mNumCancels); + assertTrue(enrollClient.mStartedHal); + } if (matches) { - if (started) { - assertEquals(Operation.STATE_STARTED_CANCELING, - mScheduler.mCurrentOperation.mState); + if (started || isEnroll) { // prep'd auth clients and enroll clients + assertTrue(mScheduler.mCurrentOperation.isCanceling()); } } else { - if (started) { - assertEquals(Operation.STATE_STARTED, - mScheduler.mCurrentOperation.mState); + if (started || isEnroll) { // prep'd auth clients and enroll clients + assertTrue(mScheduler.mCurrentOperation.isStarted()); } else { - assertEquals(Operation.STATE_WAITING_FOR_COOKIE, - mScheduler.mCurrentOperation.mState); + assertNotEquals(0, mScheduler.mCurrentOperation.isReadyToStart()); } } } @@ -411,18 +457,14 @@ public class BiometricSchedulerTest { mScheduler.cancelAuthenticationOrDetection(mToken, 9999); waitForIdle(); - assertEquals(Operation.STATE_STARTED, - mScheduler.mCurrentOperation.mState); - assertEquals(Operation.STATE_WAITING_IN_QUEUE, - mScheduler.mPendingOperations.getFirst().mState); + assertTrue(mScheduler.mCurrentOperation.isStarted()); + assertFalse(mScheduler.mPendingOperations.getFirst().isStarted()); mScheduler.cancelAuthenticationOrDetection(mToken, requestId2); waitForIdle(); - assertEquals(Operation.STATE_STARTED, - mScheduler.mCurrentOperation.mState); - assertEquals(Operation.STATE_WAITING_IN_QUEUE_CANCELING, - mScheduler.mPendingOperations.getFirst().mState); + assertTrue(mScheduler.mCurrentOperation.isStarted()); + assertTrue(mScheduler.mPendingOperations.getFirst().isMarkedCanceling()); } @Test @@ -459,12 +501,12 @@ public class BiometricSchedulerTest { @Test public void testClientDestroyed_afterFinish() { final HalClientMonitor.LazyDaemon nonNullDaemon = () -> mock(Object.class); - final TestClientMonitor client = - new TestClientMonitor(mContext, mToken, nonNullDaemon); + final TestHalClientMonitor client = + new TestHalClientMonitor(mContext, mToken, nonNullDaemon); mScheduler.scheduleClientMonitor(client); client.mCallback.onClientFinished(client, true /* success */); waitForIdle(); - assertTrue(client.wasDestroyed()); + assertTrue(client.mDestroyed); } private BiometricSchedulerProto getDump(boolean clearSchedulerBuffer) throws Exception { @@ -472,8 +514,10 @@ public class BiometricSchedulerTest { } private static class TestAuthenticationClient extends AuthenticationClient { - int mNumCancels = 0; + boolean mStartedHal = false; + boolean mStoppedHal = false; boolean mDestroyed = false; + int mNumCancels = 0; public TestAuthenticationClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @@ -488,18 +532,16 @@ public class BiometricSchedulerTest { @Override protected void stopHalOperation() { - + mStoppedHal = true; } @Override protected void startHalOperation() { - + mStartedHal = true; } @Override - protected void handleLifecycleAfterAuth(boolean authenticated) { - - } + protected void handleLifecycleAfterAuth(boolean authenticated) {} @Override public boolean wasUserDetected() { @@ -519,36 +561,59 @@ public class BiometricSchedulerTest { } } - private static class TestClientMonitor2 extends TestClientMonitor { - private final int mProtoEnum; + private static class TestEnrollClient extends EnrollClient { + boolean mStartedHal = false; + boolean mStoppedHal = false; + int mNumCancels = 0; - public TestClientMonitor2(@NonNull Context context, @NonNull IBinder token, - @NonNull LazyDaemon lazyDaemon, int protoEnum) { - super(context, token, lazyDaemon); - mProtoEnum = protoEnum; + TestEnrollClient(@NonNull Context context, + @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, + @NonNull ClientMonitorCallbackConverter listener) { + super(context, lazyDaemon, token, listener, 0 /* userId */, new byte[69], + "test" /* owner */, mock(BiometricUtils.class), + 5 /* timeoutSec */, 0 /* statsModality */, TEST_SENSOR_ID, + true /* shouldVibrate */); } @Override - public int getProtoEnum() { - return mProtoEnum; + protected void stopHalOperation() { + mStoppedHal = true; + } + + @Override + protected void startHalOperation() { + mStartedHal = true; + } + + @Override + protected boolean hasReachedEnrollmentLimit() { + return false; + } + + @Override + public void cancel() { + mNumCancels++; + super.cancel(); } } - private static class TestClientMonitor extends HalClientMonitor { + private static class TestHalClientMonitor extends HalClientMonitor { + private final int mProtoEnum; private boolean mUnableToStart; private boolean mStarted; private boolean mDestroyed; - public TestClientMonitor(@NonNull Context context, @NonNull IBinder token, + TestHalClientMonitor(@NonNull Context context, @NonNull IBinder token, @NonNull LazyDaemon lazyDaemon) { - this(context, token, lazyDaemon, 0 /* cookie */); + this(context, token, lazyDaemon, 0 /* cookie */, BiometricsProto.CM_UPDATE_ACTIVE_USER); } - public TestClientMonitor(@NonNull Context context, @NonNull IBinder token, - @NonNull LazyDaemon lazyDaemon, int cookie) { + TestHalClientMonitor(@NonNull Context context, @NonNull IBinder token, + @NonNull LazyDaemon lazyDaemon, int cookie, int protoEnum) { super(context, lazyDaemon, token /* token */, null /* listener */, 0 /* userId */, TAG, cookie, TEST_SENSOR_ID, 0 /* statsModality */, 0 /* statsAction */, 0 /* statsClient */); + mProtoEnum = protoEnum; } @Override @@ -559,9 +624,7 @@ public class BiometricSchedulerTest { @Override public int getProtoEnum() { - // Anything other than CM_NONE, which is used to represent "idle". Tests that need - // real proto enums should use TestClientMonitor2 - return BiometricsProto.CM_UPDATE_ACTIVE_USER; + return mProtoEnum; } @Override @@ -573,7 +636,7 @@ public class BiometricSchedulerTest { @Override protected void startHalOperation() { - + mStarted = true; } @Override @@ -581,22 +644,9 @@ public class BiometricSchedulerTest { super.destroy(); mDestroyed = true; } - - public boolean wasUnableToStart() { - return mUnableToStart; - } - - public boolean hasStarted() { - return mStarted; - } - - public boolean wasDestroyed() { - return mDestroyed; - } - } - private static void waitForIdle() { - InstrumentationRegistry.getInstrumentation().waitForIdleSync(); + private void waitForIdle() { + TestableLooper.get(this).processAllMessages(); } } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java index 7fccd49db04b1..407f5fb04adf7 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java @@ -16,6 +16,8 @@ package com.android.server.biometrics.sensors; +import static android.testing.TestableLooper.RunWithLooper; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -28,52 +30,53 @@ import static org.mockito.Mockito.when; import android.content.Context; import android.hardware.biometrics.IBiometricService; import android.os.Binder; +import android.os.Handler; import android.os.IBinder; import android.os.UserHandle; import android.platform.test.annotations.Presubmit; +import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @Presubmit +@RunWith(AndroidTestingRunner.class) +@RunWithLooper @SmallTest public class UserAwareBiometricSchedulerTest { - private static final String TAG = "BiometricSchedulerTest"; + private static final String TAG = "UserAwareBiometricSchedulerTest"; private static final int TEST_SENSOR_ID = 0; + private Handler mHandler; private UserAwareBiometricScheduler mScheduler; - private IBinder mToken; + private IBinder mToken = new Binder(); @Mock private Context mContext; @Mock private IBiometricService mBiometricService; - private TestUserStartedCallback mUserStartedCallback; - private TestUserStoppedCallback mUserStoppedCallback; + private TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback(); + private TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback(); private int mCurrentUserId = UserHandle.USER_NULL; - private boolean mStartOperationsFinish; - private int mStartUserClientCount; + private boolean mStartOperationsFinish = true; + private int mStartUserClientCount = 0; @Before public void setUp() { MockitoAnnotations.initMocks(this); - - mToken = new Binder(); - mStartOperationsFinish = true; - mStartUserClientCount = 0; - mUserStartedCallback = new TestUserStartedCallback(); - mUserStoppedCallback = new TestUserStoppedCallback(); - + mHandler = new Handler(TestableLooper.get(this).getLooper()); mScheduler = new UserAwareBiometricScheduler(TAG, + mHandler, BiometricScheduler.SENSOR_TYPE_UNKNOWN, null /* gestureAvailabilityDispatcher */, mBiometricService, @@ -117,7 +120,7 @@ public class UserAwareBiometricSchedulerTest { mCurrentUserId = UserHandle.USER_NULL; mStartOperationsFinish = false; - final BaseClientMonitor[] nextClients = new BaseClientMonitor[] { + final BaseClientMonitor[] nextClients = new BaseClientMonitor[]{ mock(BaseClientMonitor.class), mock(BaseClientMonitor.class), mock(BaseClientMonitor.class) @@ -147,11 +150,11 @@ public class UserAwareBiometricSchedulerTest { waitForIdle(); final TestStartUserClient startUserClient = - (TestStartUserClient) mScheduler.mCurrentOperation.mClientMonitor; + (TestStartUserClient) mScheduler.mCurrentOperation.getClientMonitor(); mScheduler.reset(); assertNull(mScheduler.mCurrentOperation); - final BiometricScheduler.Operation fakeOperation = new BiometricScheduler.Operation( + final BiometricSchedulerOperation fakeOperation = new BiometricSchedulerOperation( mock(BaseClientMonitor.class), new BaseClientMonitor.Callback() {}); mScheduler.mCurrentOperation = fakeOperation; startUserClient.mCallback.onClientFinished(startUserClient, true); @@ -194,8 +197,8 @@ public class UserAwareBiometricSchedulerTest { verify(nextClient).start(any()); } - private static void waitForIdle() { - InstrumentationRegistry.getInstrumentation().waitForIdleSync(); + private void waitForIdle() { + TestableLooper.get(this).processAllMessages(); } private class TestUserStoppedCallback implements StopUserClient.UserStoppedCallback { diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java index a13dff21439d6..0891eca9f61c0 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java @@ -79,6 +79,7 @@ public class SensorTest { when(mContext.getSystemService(Context.BIOMETRIC_SERVICE)).thenReturn(mBiometricService); mScheduler = new UserAwareBiometricScheduler(TAG, + new Handler(mLooper.getLooper()), BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityDispatcher */, () -> USER_ID, diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/Face10Test.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/Face10Test.java index 39c51d5f5e5e1..21a7a8ae65b97 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/Face10Test.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/Face10Test.java @@ -32,7 +32,9 @@ import android.hardware.face.FaceSensorProperties; import android.hardware.face.FaceSensorPropertiesInternal; import android.hardware.face.IFaceServiceReceiver; import android.os.Binder; +import android.os.Handler; import android.os.IBinder; +import android.os.Looper; import android.os.UserManager; import android.platform.test.annotations.Presubmit; @@ -69,6 +71,7 @@ public class Face10Test { @Mock private BiometricScheduler mScheduler; + private final Handler mHandler = new Handler(Looper.getMainLooper()); private LockoutResetDispatcher mLockoutResetDispatcher; private com.android.server.biometrics.sensors.face.hidl.Face10 mFace10; private IBinder mBinder; @@ -97,7 +100,7 @@ public class Face10Test { resetLockoutRequiresChallenge); Face10.sSystemClock = Clock.fixed(Instant.ofEpochMilli(100), ZoneId.of("PST")); - mFace10 = new Face10(mContext, sensorProps, mLockoutResetDispatcher, mScheduler); + mFace10 = new Face10(mContext, sensorProps, mLockoutResetDispatcher, mHandler, mScheduler); mBinder = new Binder(); } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java index 0d520ca9a4e41..a012b8b06c7f5 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java @@ -79,6 +79,7 @@ public class SensorTest { when(mContext.getSystemService(Context.BIOMETRIC_SERVICE)).thenReturn(mBiometricService); mScheduler = new UserAwareBiometricScheduler(TAG, + new Handler(mLooper.getLooper()), BiometricScheduler.SENSOR_TYPE_FP_OTHER, null /* gestureAvailabilityDispatcher */, () -> USER_ID, From bd4cd38772f2881b2bda97e74d6435de202acab2 Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Thu, 13 Jan 2022 12:57:50 -0800 Subject: [PATCH 2/2] Check current user when the operations runs for fingerprint hidl. The current and target user are both provided to the switch client constructor which is brittle unless operations are scheduled and run synchronously. Commit 9a99503870074d40d794245d9e3ac7a076f5b2f2 changed the handlers and somehow caused a bug where the cached current user was out of sync. The switch client tries to optimize for this and can skip switching when that occurs. This also includes two additonal changes 1) a few updated logs from the original change and 2) restores each scheduling having its own handler for clarity. Bug: 213962104 Test: atest UserAwareBiometricSchedulerTest BiometricSchedulerTest BiometricSchedulerOperationTest SensorTest Face10Test Test: manual (flash, enroll fingerprint, add work profile account, reboot, verify fingerprint still works) Change-Id: Ifb73b0145aeb8afb62d1f55d2f881347b0d2ef8a --- .../biometrics/sensors/BiometricScheduler.java | 11 +++++------ .../sensors/BiometricSchedulerOperation.java | 8 +++++--- .../sensors/UserAwareBiometricScheduler.java | 6 +++--- .../biometrics/sensors/face/aidl/Sensor.java | 2 +- .../biometrics/sensors/face/hidl/Face10.java | 4 +++- .../sensors/fingerprint/aidl/Sensor.java | 2 +- .../sensors/fingerprint/hidl/Fingerprint21.java | 12 +++++++++--- .../fingerprint/hidl/Fingerprint21UdfpsMock.java | 3 +-- .../hidl/FingerprintUpdateActiveUserClient.java | 16 ++++++++++------ .../biometrics/sensors/face/aidl/SensorTest.java | 5 ++++- .../sensors/fingerprint/aidl/SensorTest.java | 5 ++++- 11 files changed, 46 insertions(+), 28 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java index 1f91c4d6803e8..39c5944d65c73 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -25,6 +25,7 @@ import android.hardware.biometrics.IBiometricService; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; import android.os.Handler; import android.os.IBinder; +import android.os.Looper; import android.os.RemoteException; import android.os.ServiceManager; import android.util.Slog; @@ -232,17 +233,14 @@ public class BiometricScheduler { * Creates a new scheduler. * * @param tag for the specific instance of the scheduler. Should be unique. - * @param handler handler for callbacks (all methods of this class must be called on the - * thread associated with this handler) * @param sensorType the sensorType that this scheduler is handling. * @param gestureAvailabilityDispatcher may be null if the sensor does not support gestures * (such as fingerprint swipe). */ public BiometricScheduler(@NonNull String tag, - @NonNull Handler handler, @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - this(tag, handler, sensorType, gestureAvailabilityDispatcher, + this(tag, new Handler(Looper.getMainLooper()), sensorType, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface( ServiceManager.getService(Context.BIOMETRIC_SERVICE)), LOG_NUM_RECENT_OPERATIONS, CoexCoordinator.getInstance()); @@ -376,8 +374,9 @@ public class BiometricScheduler { // send ERROR_CANCELED and skip the operation. if (clientMonitor.interruptsPrecedingClients()) { for (BiometricSchedulerOperation operation : mPendingOperations) { - Slog.d(getTag(), "New client, marking pending op as canceling: " + operation); - operation.markCanceling(); + if (operation.markCanceling()) { + Slog.d(getTag(), "New client, marking pending op as canceling: " + operation); + } } } diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java index a8cce153dc706..e8b50d90b5865 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java @@ -225,12 +225,13 @@ public class BiometricSchedulerOperation { Slog.v(TAG, "Aborted: " + this); } - /** Flags this operation as canceled, but does not cancel it until started. */ - public void markCanceling() { + /** Flags this operation as canceled, if possible, but does not cancel it until started. */ + public boolean markCanceling() { if (mState == STATE_WAITING_IN_QUEUE && isInterruptable()) { mState = STATE_WAITING_IN_QUEUE_CANCELING; - Slog.v(TAG, "Marked cancelling: " + this); + return true; } + return false; } /** @@ -280,6 +281,7 @@ public class BiometricSchedulerOperation { @Override public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) { + Slog.d(TAG, "[Finished / destroy]: " + clientMonitor); mClientMonitor.destroy(); mState = STATE_FINISHED; } diff --git a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java index 19eaa178c7c9f..603cc22968a99 100644 --- a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java @@ -23,6 +23,7 @@ import android.annotation.Nullable; import android.content.Context; import android.hardware.biometrics.IBiometricService; import android.os.Handler; +import android.os.Looper; import android.os.ServiceManager; import android.os.UserHandle; import android.util.Slog; @@ -85,7 +86,7 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { } @VisibleForTesting - UserAwareBiometricScheduler(@NonNull String tag, + public UserAwareBiometricScheduler(@NonNull String tag, @NonNull Handler handler, @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, @@ -101,12 +102,11 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { } public UserAwareBiometricScheduler(@NonNull String tag, - @NonNull Handler handler, @SensorType int sensorType, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, @NonNull CurrentUserRetriever currentUserRetriever, @NonNull UserSwitchCallback userSwitchCallback) { - this(tag, handler, sensorType, gestureAvailabilityDispatcher, + this(tag, new Handler(Looper.getMainLooper()), sensorType, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface( ServiceManager.getService(Context.BIOMETRIC_SERVICE)), currentUserRetriever, userSwitchCallback, CoexCoordinator.getInstance()); diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java index 39270430c21d4..206b8f0779e8e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java @@ -494,7 +494,7 @@ public class Sensor { mToken = new Binder(); mHandler = handler; mSensorProperties = sensorProperties; - mScheduler = new UserAwareBiometricScheduler(tag, mHandler, + mScheduler = new UserAwareBiometricScheduler(tag, BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityDispatcher */, () -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL, new UserAwareBiometricScheduler.UserSwitchCallback() { diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java index 493c0a05e3795..e957794372aac 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java @@ -363,7 +363,7 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { @NonNull LockoutResetDispatcher lockoutResetDispatcher) { final Handler handler = new Handler(Looper.getMainLooper()); return new Face10(context, sensorProps, lockoutResetDispatcher, handler, - new BiometricScheduler(TAG, handler, BiometricScheduler.SENSOR_TYPE_FACE, + new BiometricScheduler(TAG, BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityTracker */)); } @@ -896,6 +896,8 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { boolean success) { if (success) { mCurrentUserId = targetUserId; + } else { + Slog.w(TAG, "Failed to change user, still: " + mCurrentUserId); } } }); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java index 256761a61a72a..59e4b582ca84e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java @@ -449,7 +449,7 @@ class Sensor { mHandler = handler; mSensorProperties = sensorProperties; mLockoutCache = new LockoutCache(); - mScheduler = new UserAwareBiometricScheduler(tag, handler, + mScheduler = new UserAwareBiometricScheduler(tag, BiometricScheduler.sensorTypeFromFingerprintProperties(mSensorProperties), gestureAvailabilityDispatcher, () -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL, diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java index d352cda609e3d..6feb5fa418bbc 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java @@ -360,7 +360,7 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider @NonNull LockoutResetDispatcher lockoutResetDispatcher, @NonNull GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { final BiometricScheduler scheduler = - new BiometricScheduler(TAG, handler, + new BiometricScheduler(TAG, BiometricScheduler.sensorTypeFromFingerprintProperties(sensorProps), gestureAvailabilityDispatcher); final HalResultController controller = new HalResultController(sensorProps.sensorId, @@ -490,19 +490,25 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider !getEnrolledFingerprints(mSensorProperties.sensorId, targetUserId).isEmpty(); final FingerprintUpdateActiveUserClient client = new FingerprintUpdateActiveUserClient(mContext, mLazyDaemon, targetUserId, - mContext.getOpPackageName(), mSensorProperties.sensorId, mCurrentUserId, - hasEnrolled, mAuthenticatorIds, force); + mContext.getOpPackageName(), mSensorProperties.sensorId, + this::getCurrentUser, hasEnrolled, mAuthenticatorIds, force); mScheduler.scheduleClientMonitor(client, new BaseClientMonitor.Callback() { @Override public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) { if (success) { mCurrentUserId = targetUserId; + } else { + Slog.w(TAG, "Failed to change user, still: " + mCurrentUserId); } } }); } + private int getCurrentUser() { + return mCurrentUserId; + } + @Override public boolean containsSensor(int sensorId) { return mSensorProperties.sensorId == sensorId; diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java index 20dab5552df98..273f8a545db55 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java @@ -138,8 +138,7 @@ public class Fingerprint21UdfpsMock extends Fingerprint21 implements TrustManage TestableBiometricScheduler(@NonNull String tag, @NonNull Handler handler, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - super(tag, handler, BiometricScheduler.SENSOR_TYPE_FP_OTHER, - gestureAvailabilityDispatcher); + super(tag, BiometricScheduler.SENSOR_TYPE_FP_OTHER, gestureAvailabilityDispatcher); } void init(@NonNull Fingerprint21UdfpsMock fingerprint21) { diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java index fd38bdd1201eb..a2c18923c00e8 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java @@ -31,6 +31,7 @@ import com.android.server.biometrics.sensors.HalClientMonitor; import java.io.File; import java.util.Map; +import java.util.function.Supplier; /** * Sets the HAL's current active user, and updates the framework's authenticatorId cache. @@ -40,7 +41,7 @@ public class FingerprintUpdateActiveUserClient extends HalClientMonitor mCurrentUserId; private final boolean mForceUpdateAuthenticatorId; private final boolean mHasEnrolledBiometrics; private final Map mAuthenticatorIds; @@ -48,8 +49,9 @@ public class FingerprintUpdateActiveUserClient extends HalClientMonitor lazyDaemon, int userId, - @NonNull String owner, int sensorId, int currentUserId, boolean hasEnrolledBiometrics, - @NonNull Map authenticatorIds, boolean forceUpdateAuthenticatorId) { + @NonNull String owner, int sensorId, Supplier currentUserId, + boolean hasEnrolledBiometrics, @NonNull Map authenticatorIds, + boolean forceUpdateAuthenticatorId) { super(context, lazyDaemon, null /* token */, null /* listener */, userId, owner, 0 /* cookie */, sensorId, BiometricsProtoEnums.MODALITY_UNKNOWN, BiometricsProtoEnums.ACTION_UNKNOWN, BiometricsProtoEnums.CLIENT_UNKNOWN); @@ -63,7 +65,7 @@ public class FingerprintUpdateActiveUserClient extends HalClientMonitor USER_ID, - mUserSwitchCallback); + mUserSwitchCallback, + CoexCoordinator.getInstance()); mHalCallback = new Sensor.HalSessionCallback(mContext, new Handler(mLooper.getLooper()), TAG, mScheduler, SENSOR_ID, USER_ID, mLockoutCache, mLockoutResetDispatcher, mHalSessionCallback); diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java index a012b8b06c7f5..d4609b55afba7 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java @@ -33,6 +33,7 @@ import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; import com.android.server.biometrics.sensors.BiometricScheduler; +import com.android.server.biometrics.sensors.CoexCoordinator; import com.android.server.biometrics.sensors.LockoutCache; import com.android.server.biometrics.sensors.LockoutResetDispatcher; import com.android.server.biometrics.sensors.LockoutTracker; @@ -82,8 +83,10 @@ public class SensorTest { new Handler(mLooper.getLooper()), BiometricScheduler.SENSOR_TYPE_FP_OTHER, null /* gestureAvailabilityDispatcher */, + mBiometricService, () -> USER_ID, - mUserSwitchCallback); + mUserSwitchCallback, + CoexCoordinator.getInstance()); mHalCallback = new Sensor.HalSessionCallback(mContext, new Handler(mLooper.getLooper()), TAG, mScheduler, SENSOR_ID, USER_ID, mLockoutCache, mLockoutResetDispatcher, mHalSessionCallback);