Handle case where ClientMonitors should not be started
Currently, the only case is when the HAL is null. For this, there are two cases: 1) Non-BiometricPrompt operation For these, clean up the current client, and do the same for any operation that's currently pending. See code comments for more details 2) BiometricPrompt operation For these, just clean up the current BiometricPrompt operation Fixes: 165636620 Test: atest BiometricSchedulerTest Test: smoke test: enroll, auth on existing devices Change-Id: Iff64d5dba8dc6ebd73f0f9fba233390e76ca8a89
This commit is contained in:
@@ -28,6 +28,7 @@ import android.os.RemoteException;
|
||||
import android.os.ServiceManager;
|
||||
import android.util.Slog;
|
||||
|
||||
import com.android.internal.annotations.VisibleForTesting;
|
||||
import com.android.server.biometrics.sensors.fingerprint.GestureAvailabilityDispatcher;
|
||||
|
||||
import java.io.PrintWriter;
|
||||
@@ -37,9 +38,9 @@ import java.text.SimpleDateFormat;
|
||||
import java.util.ArrayDeque;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Date;
|
||||
import java.util.Deque;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Queue;
|
||||
|
||||
/**
|
||||
* A scheduler for biometric HAL operations. Maintains a queue of {@link ClientMonitor} operations,
|
||||
@@ -53,7 +54,8 @@ public class BiometricScheduler {
|
||||
/**
|
||||
* Contains all the necessary information for a HAL operation.
|
||||
*/
|
||||
private static final class Operation {
|
||||
@VisibleForTesting
|
||||
static final class Operation {
|
||||
|
||||
/**
|
||||
* The operation is added to the list of pending operations and waiting for its turn.
|
||||
@@ -176,8 +178,8 @@ public class BiometricScheduler {
|
||||
@NonNull private final IBiometricService mBiometricService;
|
||||
@NonNull private final Handler mHandler = new Handler(Looper.getMainLooper());
|
||||
@NonNull private final InternalCallback mInternalCallback;
|
||||
@NonNull private final Queue<Operation> mPendingOperations;
|
||||
@Nullable private Operation mCurrentOperation;
|
||||
@VisibleForTesting @NonNull final Deque<Operation> mPendingOperations;
|
||||
@VisibleForTesting @Nullable Operation mCurrentOperation;
|
||||
@NonNull private final ArrayDeque<CrashState> mCrashStates;
|
||||
|
||||
// Internal callback, notified when an operation is complete. Notifies the requester
|
||||
@@ -226,6 +228,18 @@ public class BiometricScheduler {
|
||||
}
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
BiometricScheduler(@NonNull String tag,
|
||||
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher,
|
||||
@NonNull IBiometricService biometricService) {
|
||||
mBiometricTag = tag;
|
||||
mInternalCallback = new InternalCallback();
|
||||
mGestureAvailabilityDispatcher = gestureAvailabilityDispatcher;
|
||||
mPendingOperations = new ArrayDeque<>();
|
||||
mBiometricService = biometricService;
|
||||
mCrashStates = new ArrayDeque<>();
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new scheduler.
|
||||
* @param tag for the specific instance of the scheduler. Should be unique.
|
||||
@@ -234,13 +248,8 @@ public class BiometricScheduler {
|
||||
*/
|
||||
public BiometricScheduler(@NonNull String tag,
|
||||
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) {
|
||||
mBiometricTag = tag;
|
||||
mInternalCallback = new InternalCallback();
|
||||
mGestureAvailabilityDispatcher = gestureAvailabilityDispatcher;
|
||||
mPendingOperations = new ArrayDeque<>();
|
||||
mBiometricService = IBiometricService.Stub.asInterface(
|
||||
ServiceManager.getService(Context.BIOMETRIC_SERVICE));
|
||||
mCrashStates = new ArrayDeque<>();
|
||||
this(tag, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface(
|
||||
ServiceManager.getService(Context.BIOMETRIC_SERVICE)));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -295,9 +304,50 @@ public class BiometricScheduler {
|
||||
// to arrive at the head of the queue, before pinging it to start.
|
||||
final boolean shouldStartNow = currentClient.getCookie() == 0;
|
||||
if (shouldStartNow) {
|
||||
Slog.d(getTag(), "[Starting] " + mCurrentOperation);
|
||||
currentClient.start(getInternalCallback());
|
||||
mCurrentOperation.state = Operation.STATE_STARTED;
|
||||
if (mCurrentOperation.clientMonitor.getFreshDaemon() == null) {
|
||||
// Note down current length of queue
|
||||
final int pendingOperationsLength = mPendingOperations.size();
|
||||
final Operation 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.
|
||||
mCurrentOperation.clientMonitor.unableToStart();
|
||||
if (mCurrentOperation.mClientCallback != null) {
|
||||
mCurrentOperation.mClientCallback
|
||||
.onClientFinished(mCurrentOperation.clientMonitor, 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) {
|
||||
Slog.e(getTag(), "Null operation, index: " + i
|
||||
+ ", expected length: " + pendingOperationsLength);
|
||||
break;
|
||||
}
|
||||
operation.clientMonitor.unableToStart();
|
||||
if (operation.mClientCallback != null) {
|
||||
operation.mClientCallback.onClientFinished(operation.clientMonitor,
|
||||
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
|
||||
// run these. A single request from the manager layer to the service layer may
|
||||
// actually be multiple operations (i.e. updateActiveUser + authenticate).
|
||||
mCurrentOperation = null;
|
||||
startNextOperationIfIdle();
|
||||
} else {
|
||||
Slog.d(getTag(), "[Starting] " + mCurrentOperation);
|
||||
currentClient.start(getInternalCallback());
|
||||
mCurrentOperation.state = Operation.STATE_STARTED;
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
mBiometricService.onReadyForAuthentication(currentClient.getCookie());
|
||||
@@ -338,9 +388,21 @@ public class BiometricScheduler {
|
||||
return;
|
||||
}
|
||||
|
||||
Slog.d(getTag(), "[Starting] Prepared client: " + mCurrentOperation);
|
||||
mCurrentOperation.state = Operation.STATE_STARTED;
|
||||
mCurrentOperation.clientMonitor.start(getInternalCallback());
|
||||
if (mCurrentOperation.clientMonitor.getFreshDaemon() == null) {
|
||||
Slog.e(getTag(), "[Unable To Start] Prepared client: " + mCurrentOperation);
|
||||
// This is BiometricPrompt trying to auth but something's wrong with the HAL.
|
||||
mCurrentOperation.clientMonitor.unableToStart();
|
||||
if (mCurrentOperation.mClientCallback != null) {
|
||||
mCurrentOperation.mClientCallback.onClientFinished(mCurrentOperation.clientMonitor,
|
||||
false /* success */);
|
||||
}
|
||||
mCurrentOperation = null;
|
||||
startNextOperationIfIdle();
|
||||
} else {
|
||||
Slog.d(getTag(), "[Starting] Prepared client: " + mCurrentOperation);
|
||||
mCurrentOperation.state = Operation.STATE_STARTED;
|
||||
mCurrentOperation.clientMonitor.start(getInternalCallback());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -16,11 +16,21 @@
|
||||
|
||||
package com.android.server.biometrics.sensors;
|
||||
|
||||
import static junit.framework.Assert.assertTrue;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.verify;
|
||||
|
||||
import android.content.Context;
|
||||
import android.hardware.biometrics.IBiometricService;
|
||||
import android.platform.test.annotations.Presubmit;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
import androidx.annotation.Nullable;
|
||||
import androidx.test.InstrumentationRegistry;
|
||||
import androidx.test.filters.SmallTest;
|
||||
|
||||
@@ -40,18 +50,21 @@ public class BiometricSchedulerTest {
|
||||
@Mock
|
||||
private Context mContext;
|
||||
@Mock
|
||||
private ClientMonitor.LazyDaemon<Object> mLazyDaemon;
|
||||
private IBiometricService mBiometricService;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
MockitoAnnotations.initMocks(this);
|
||||
mScheduler = new BiometricScheduler(TAG, null /* gestureAvailabilityTracker */);
|
||||
mScheduler = new BiometricScheduler(TAG, null /* gestureAvailabilityTracker */,
|
||||
mBiometricService);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testClientDuplicateFinish_ignoredBySchedulerAndDoesNotCrash() {
|
||||
final ClientMonitor<Object> client1 = new TestClientMonitor(mContext, mLazyDaemon);
|
||||
final ClientMonitor<Object> client2 = new TestClientMonitor(mContext, mLazyDaemon);
|
||||
final ClientMonitor.LazyDaemon<Object> nonNullDaemon = () -> mock(Object.class);
|
||||
|
||||
final ClientMonitor<Object> client1 = new TestClientMonitor(mContext, nonNullDaemon);
|
||||
final ClientMonitor<Object> client2 = new TestClientMonitor(mContext, nonNullDaemon);
|
||||
mScheduler.scheduleClientMonitor(client1);
|
||||
mScheduler.scheduleClientMonitor(client2);
|
||||
|
||||
@@ -59,7 +72,93 @@ public class BiometricSchedulerTest {
|
||||
client1.mCallback.onClientFinished(client1, true /* success */);
|
||||
}
|
||||
|
||||
@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 ClientMonitor.LazyDaemon<Object> lazyDaemon1 = () -> null;
|
||||
final ClientMonitor.LazyDaemon<Object> lazyDaemon2 = () -> daemon2;
|
||||
|
||||
final TestClientMonitor client1 = new TestClientMonitor(mContext, lazyDaemon1);
|
||||
final TestClientMonitor client2 = new TestClientMonitor(mContext, lazyDaemon2);
|
||||
|
||||
final ClientMonitor.Callback callback1 = mock(ClientMonitor.Callback.class);
|
||||
final ClientMonitor.Callback callback2 = mock(ClientMonitor.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(
|
||||
mock(ClientMonitor.class), mock(ClientMonitor.Callback.class));
|
||||
|
||||
mScheduler.scheduleClientMonitor(client1, callback1);
|
||||
assertEquals(1, mScheduler.mPendingOperations.size());
|
||||
// client1 is pending. Allow the scheduler to start once second client is added.
|
||||
mScheduler.mCurrentOperation = null;
|
||||
mScheduler.scheduleClientMonitor(client2, callback2);
|
||||
waitForIdle();
|
||||
|
||||
assertTrue(client1.wasUnableToStart());
|
||||
verify(callback1).onClientFinished(eq(client1), eq(false) /* success */);
|
||||
verify(callback1, never()).onClientStarted(any());
|
||||
|
||||
assertTrue(client2.wasUnableToStart());
|
||||
verify(callback2).onClientFinished(eq(client2), eq(false) /* success */);
|
||||
verify(callback2, never()).onClientStarted(any());
|
||||
|
||||
assertTrue(mScheduler.mPendingOperations.isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRemovesOnlyBiometricPromptOperation_whenNullHal() {
|
||||
// Second non-BiometricPrompt client has a valid daemon
|
||||
final Object daemon2 = mock(Object.class);
|
||||
|
||||
final ClientMonitor.LazyDaemon<Object> lazyDaemon1 = () -> null;
|
||||
final ClientMonitor.LazyDaemon<Object> lazyDaemon2 = () -> daemon2;
|
||||
|
||||
final TestClientMonitor client1 =
|
||||
new TestBiometricPromptClientMonitor(mContext, lazyDaemon1);
|
||||
final TestClientMonitor client2 = new TestClientMonitor(mContext, lazyDaemon2);
|
||||
|
||||
final ClientMonitor.Callback callback1 = mock(ClientMonitor.Callback.class);
|
||||
final ClientMonitor.Callback callback2 = mock(ClientMonitor.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(
|
||||
mock(ClientMonitor.class), mock(ClientMonitor.Callback.class));
|
||||
|
||||
mScheduler.scheduleClientMonitor(client1, callback1);
|
||||
assertEquals(1, mScheduler.mPendingOperations.size());
|
||||
// client1 is pending. Allow the scheduler to start once second client is added.
|
||||
mScheduler.mCurrentOperation = null;
|
||||
mScheduler.scheduleClientMonitor(client2, callback2);
|
||||
waitForIdle();
|
||||
|
||||
// Simulate that the BiometricPrompt client's sensor is ready
|
||||
mScheduler.startPreparedClient(client1.getCookie());
|
||||
|
||||
assertTrue(client1.wasUnableToStart());
|
||||
verify(callback1).onClientFinished(eq(client1), eq(false) /* success */);
|
||||
verify(callback1, never()).onClientStarted(any());
|
||||
|
||||
// Client 2 was able to start
|
||||
assertFalse(client2.wasUnableToStart());
|
||||
assertTrue(client2.hasStarted());
|
||||
verify(callback2).onClientStarted(eq(client2));
|
||||
}
|
||||
|
||||
private static class TestBiometricPromptClientMonitor extends TestClientMonitor {
|
||||
public TestBiometricPromptClientMonitor(@NonNull Context context,
|
||||
@NonNull LazyDaemon<Object> lazyDaemon) {
|
||||
super(context, lazyDaemon, 1 /* cookie */);
|
||||
}
|
||||
}
|
||||
|
||||
private static class TestClientMonitor extends ClientMonitor<Object> {
|
||||
private boolean mUnableToStart;
|
||||
private boolean mStarted;
|
||||
|
||||
public TestClientMonitor(@NonNull Context context, @NonNull LazyDaemon<Object> lazyDaemon) {
|
||||
super(context, lazyDaemon, null /* token */, null /* listener */, 0 /* userId */,
|
||||
@@ -67,14 +166,42 @@ public class BiometricSchedulerTest {
|
||||
0 /* statsAction */, 0 /* statsClient */);
|
||||
}
|
||||
|
||||
public TestClientMonitor(@NonNull Context context, @NonNull LazyDaemon<Object> lazyDaemon,
|
||||
int cookie) {
|
||||
super(context, lazyDaemon, null /* token */, null /* listener */, 0 /* userId */,
|
||||
TAG, cookie, 0 /* sensorId */, 0 /* statsModality */,
|
||||
0 /* statsAction */, 0 /* statsClient */);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public void unableToStart() {
|
||||
assertFalse(mUnableToStart);
|
||||
mUnableToStart = true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void start(@NonNull Callback callback) {
|
||||
super.start(callback);
|
||||
assertFalse(mStarted);
|
||||
mStarted = true;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void startHalOperation() {
|
||||
|
||||
}
|
||||
|
||||
public boolean wasUnableToStart() {
|
||||
return mUnableToStart;
|
||||
}
|
||||
|
||||
public boolean hasStarted() {
|
||||
return mStarted;
|
||||
}
|
||||
}
|
||||
|
||||
private static void waitForIdle() {
|
||||
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user