Add BaseClientMonitor#interruptsPrecedingClients

Only a select few operations (such as auth or enroll related stuff)
should interrupt preceeding clients. Book-keeping operations (such
as generateChallenge, invalidation, etc) generally should not interrupt
preceding clients.

Fixes bug in existing test case, and adds new testcases for the newly
added functionality.

Fixes: 179646305
Test: atest CtsBiometricsTestCases
Test: atest com.android.server.biometrics

Change-Id: Ie51b121ba5f0cf3f51477c12807a8be4c80c7c61

services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java

@@ -311,4 +311,9 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T>
     public int getProtoEnum() {
         return BiometricsProto.CM_AUTHENTICATE;
     }
+
+    @Override
+    public boolean interruptsPrecedingClients() {
+        return true;
+    }
 }

services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java

@@ -82,11 +82,17 @@ public abstract class BaseClientMonitor extends LoggableMonitor
     @NonNull protected Callback mCallback;
 
     /**
-     * Returns a ClientMonitorEnum constant defined in biometrics.proto
-     * @return
+     * @return A ClientMonitorEnum constant defined in biometrics.proto
      */
     public abstract int getProtoEnum();
 
+    /**
+     * @return True if the ClientMonitor should cancel any current and pending interruptable clients
+     */
+    public boolean interruptsPrecedingClients() {
+        return false;
+    }
+
     /**
      * @param context    system_server context
      * @param token      a unique token for the client

services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java

@@ -475,14 +475,17 @@ public class BiometricScheduler {
      */
     public void scheduleClientMonitor(@NonNull BaseClientMonitor clientMonitor,
             @Nullable BaseClientMonitor.Callback clientCallback) {
-        // Mark any interruptable pending clients as canceling. Once they reach the head of the
-        // queue, the scheduler will send ERROR_CANCELED and skip the operation.
-        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;
+        // If the incoming operation should interrupt preceding clients, mark any interruptable
+        // 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;
+                }
             }
         }
 
@@ -490,8 +493,11 @@ public class BiometricScheduler {
         Slog.d(getTag(), "[Added] " + clientMonitor
                 + ", new queue size: " + mPendingOperations.size());
 
-        // If the current operation is cancellable, start the cancellation process.
-        if (mCurrentOperation != null && mCurrentOperation.mClientMonitor instanceof Interruptable
+        // If the new operation should interrupt preceding clients, and if the current operation is
+        // cancellable, start the cancellation process.
+        if (clientMonitor.interruptsPrecedingClients()
+                && mCurrentOperation != null
+                && mCurrentOperation.mClientMonitor instanceof Interruptable
                 && mCurrentOperation.mState == Operation.STATE_STARTED) {
             Slog.d(getTag(), "[Cancelling Interruptable]: " + mCurrentOperation);
             cancelInternal(mCurrentOperation);

services/core/java/com/android/server/biometrics/sensors/EnrollClient.java

@@ -113,4 +113,9 @@ public abstract class EnrollClient<T> extends AcquisitionClient<T> {
     public int getProtoEnum() {
         return BiometricsProto.CM_ENROLL;
     }
+
+    @Override
+    public boolean interruptsPrecedingClients() {
+        return true;
+    }
 }

services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintDetectClient.java

@@ -98,4 +98,9 @@ class FingerprintDetectClient extends AcquisitionClient<ISession> {
     public int getProtoEnum() {
         return BiometricsProto.CM_DETECT_INTERACTION;
     }
+
+    @Override
+    public boolean interruptsPrecedingClients() {
+        return true;
+    }
 }

services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintDetectClient.java

@@ -130,4 +130,9 @@ class FingerprintDetectClient extends AcquisitionClient<IBiometricsFingerprint>
     public int getProtoEnum() {
         return BiometricsProto.CM_DETECT_INTERACTION;
     }
+
+    @Override
+    public boolean interruptsPrecedingClients() {
+        return true;
+    }
 }

services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java

@@ -27,6 +27,8 @@ 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 static org.mockito.Mockito.when;
+import static org.mockito.Mockito.withSettings;
 
 import android.content.Context;
 import android.hardware.biometrics.BiometricConstants;
@@ -159,8 +161,8 @@ public class BiometricSchedulerTest {
 
         // Client 1 cleans up properly
         verify(listener1).onError(eq(TEST_SENSOR_ID), anyInt(),
-                eq(BiometricConstants.BIOMETRIC_ERROR_CANCELED), eq(0));
-        verify(callback1).onClientFinished(eq(client1), eq(true) /* success */);
+                eq(BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE), eq(0));
+        verify(callback1).onClientFinished(eq(client1), eq(false) /* success */);
         verify(callback1, never()).onClientStarted(any());
 
         // Client 2 was able to start
@@ -310,6 +312,37 @@ public class BiometricSchedulerTest {
         assertNull(mScheduler.getCurrentClient());
     }
 
+    @Test
+    public void testInterruptPrecedingClients_whenExpected() {
+        final BaseClientMonitor interruptableMonitor = mock(BaseClientMonitor.class,
+                withSettings().extraInterfaces(Interruptable.class));
+
+        final BaseClientMonitor interrupter = mock(BaseClientMonitor.class);
+        when(interrupter.interruptsPrecedingClients()).thenReturn(true);
+
+        mScheduler.scheduleClientMonitor(interruptableMonitor);
+        mScheduler.scheduleClientMonitor(interrupter);
+        waitForIdle();
+
+        verify((Interruptable) interruptableMonitor).cancel();
+        mScheduler.getInternalCallback().onClientFinished(interruptableMonitor, true /* success */);
+    }
+
+    @Test
+    public void testDoesNotInterruptPrecedingClients_whenNotExpected() {
+        final BaseClientMonitor interruptableMonitor = mock(BaseClientMonitor.class,
+                withSettings().extraInterfaces(Interruptable.class));
+
+        final BaseClientMonitor interrupter = mock(BaseClientMonitor.class);
+        when(interrupter.interruptsPrecedingClients()).thenReturn(false);
+
+        mScheduler.scheduleClientMonitor(interruptableMonitor);
+        mScheduler.scheduleClientMonitor(interrupter);
+        waitForIdle();
+
+        verify((Interruptable) interruptableMonitor, never()).cancel();
+    }
+
     private BiometricSchedulerProto getDump(boolean clearSchedulerBuffer) throws Exception {
         return BiometricSchedulerProto.parseFrom(mScheduler.dumpProtoState(clearSchedulerBuffer));
     }