Merge "Wrap start and stop user clients in an operation." into sc-dev

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

@@ -110,11 +110,21 @@ public class BiometricScheduler {
         @Nullable final BaseClientMonitor.Callback mClientCallback;
         @OperationState int mState;
 
-        Operation(@NonNull BaseClientMonitor clientMonitor,
+        Operation(
-                @Nullable BaseClientMonitor.Callback callback) {
+                @NonNull BaseClientMonitor clientMonitor,
-            this.mClientMonitor = clientMonitor;
+                @Nullable BaseClientMonitor.Callback callback
-            this.mClientCallback = callback;
+        ) {
-            mState = STATE_WAITING_IN_QUEUE;
+            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() {

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

@@ -50,16 +50,26 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
 
     @NonNull private final CurrentUserRetriever mCurrentUserRetriever;
     @NonNull private final UserSwitchCallback mUserSwitchCallback;
-    @NonNull @VisibleForTesting final ClientFinishedCallback mClientFinishedCallback;
-
     @Nullable private StopUserClient<?> mStopUserClient;
 
-    @VisibleForTesting
+    private class ClientFinishedCallback implements BaseClientMonitor.Callback {
-    class ClientFinishedCallback implements BaseClientMonitor.Callback {
+        private final BaseClientMonitor mOwner;
+
+        ClientFinishedCallback(BaseClientMonitor owner) {
+            mOwner = owner;
+        }
+
         @Override
         public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) {
             mHandler.post(() -> {
-                Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success);
+                if (mOwner == clientMonitor && mOwner == mCurrentOperation.mClientMonitor) {
+                    Slog.d(getTag(), "[Client finished] "
+                            + clientMonitor + ", success: " + success);
+                    mCurrentOperation = null;
+                } else {
+                    Slog.e(getTag(), "[Client finished, but not current operation], actual: "
+                            + mCurrentOperation + ", expected: " + mOwner);
+                }
 
                 startNextOperationIfIdle();
             });
@@ -76,7 +86,6 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
 
         mCurrentUserRetriever = currentUserRetriever;
         mUserSwitchCallback = userSwitchCallback;
-        mClientFinishedCallback = new ClientFinishedCallback();
     }
 
     public UserAwareBiometricScheduler(@NonNull String tag,
@@ -112,17 +121,27 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
         } else if (currentUserId == UserHandle.USER_NULL) {
             final BaseClientMonitor startClient =
                     mUserSwitchCallback.getStartUserClient(nextUserId);
+            final ClientFinishedCallback finishedCallback =
+                    new ClientFinishedCallback(startClient);
+
             Slog.d(getTag(), "[Starting User] " + startClient);
-            startClient.start(mClientFinishedCallback);
+            startClient.start(finishedCallback);
+            mCurrentOperation = new Operation(
+                    startClient, finishedCallback, Operation.STATE_STARTED);
         } else {
             if (mStopUserClient != null) {
                 Slog.d(getTag(), "[Waiting for StopUser] " + mStopUserClient);
             } else {
                 mStopUserClient = mUserSwitchCallback
                         .getStopUserClient(currentUserId);
+                final ClientFinishedCallback finishedCallback =
+                        new ClientFinishedCallback(mStopUserClient);
+
                 Slog.d(getTag(), "[Stopping User] current: " + currentUserId
                         + ", next: " + nextUserId + ". " + mStopUserClient);
-                mStopUserClient.start(mClientFinishedCallback);
+                mStopUserClient.start(finishedCallback);
+                mCurrentOperation = new Operation(
+                        mStopUserClient, finishedCallback, Operation.STATE_STARTED);
             }
         }
     }

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

@@ -19,6 +19,7 @@ package com.android.server.biometrics.sensors;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -57,12 +58,16 @@ public class UserAwareBiometricSchedulerTest {
     private TestUserStartedCallback mUserStartedCallback;
     private TestUserStoppedCallback mUserStoppedCallback;
     private int mCurrentUserId = UserHandle.USER_NULL;
+    private boolean mStartOperationsFinish;
+    private int mStartUserClientCount;
 
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
         mToken = new Binder();
+        mStartOperationsFinish = true;
+        mStartUserClientCount = 0;
         mUserStartedCallback = new TestUserStartedCallback();
         mUserStoppedCallback = new TestUserStoppedCallback();
 
@@ -81,8 +86,9 @@ public class UserAwareBiometricSchedulerTest {
                     @NonNull
                     @Override
                     public StartUserClient<?, ?> getStartUserClient(int newUserId) {
+                        mStartUserClientCount++;
                         return new TestStartUserClient(mContext, Object::new, mToken, newUserId,
-                                TEST_SENSOR_ID, mUserStartedCallback);
+                                TEST_SENSOR_ID, mUserStartedCallback, mStartOperationsFinish);
                     }
                 });
     }
@@ -91,20 +97,41 @@ public class UserAwareBiometricSchedulerTest {
     public void testScheduleOperation_whenNoUser() {
         mCurrentUserId = UserHandle.USER_NULL;
 
-        final int nextUserId = 0;
+        final BaseClientMonitor nextClient = mock(BaseClientMonitor.class);
-
+        when(nextClient.getTargetUserId()).thenReturn(0);
-        BaseClientMonitor nextClient = mock(BaseClientMonitor.class);
-        when(nextClient.getTargetUserId()).thenReturn(nextUserId);
 
         mScheduler.scheduleClientMonitor(nextClient);
+        waitForIdle();
 
         assertEquals(0, mUserStoppedCallback.numInvocations);
         assertEquals(1, mUserStartedCallback.numInvocations);
-
-        waitForIdle();
         verify(nextClient).start(any());
     }
 
+    @Test
+    public void testScheduleOperation_whenNoUser_notStarted() {
+        mCurrentUserId = UserHandle.USER_NULL;
+        mStartOperationsFinish = false;
+
+        final BaseClientMonitor[] nextClients = new BaseClientMonitor[] {
+                mock(BaseClientMonitor.class),
+                mock(BaseClientMonitor.class),
+                mock(BaseClientMonitor.class)
+        };
+        for (BaseClientMonitor client : nextClients) {
+            when(client.getTargetUserId()).thenReturn(5);
+            mScheduler.scheduleClientMonitor(client);
+            waitForIdle();
+        }
+
+        assertEquals(0, mUserStoppedCallback.numInvocations);
+        assertEquals(0, mUserStartedCallback.numInvocations);
+        assertEquals(1, mStartUserClientCount);
+        for (BaseClientMonitor client : nextClients) {
+            verify(client, never()).start(any());
+        }
+    }
+
     @Test
     public void testScheduleOperation_whenSameUser() {
         mCurrentUserId = 10;
@@ -192,10 +219,13 @@ public class UserAwareBiometricSchedulerTest {
     }
 
     private static class TestStartUserClient extends StartUserClient<Object, Object> {
+        private final boolean mShouldFinish;
+
         public TestStartUserClient(@NonNull Context context,
                 @NonNull LazyDaemon<Object> lazyDaemon, @Nullable IBinder token, int userId,
-                int sensorId, @NonNull UserStartedCallback<Object> callback) {
+                int sensorId, @NonNull UserStartedCallback<Object> callback, boolean shouldFinish) {
             super(context, lazyDaemon, token, userId, sensorId, callback);
+            mShouldFinish = shouldFinish;
         }
 
         @Override
@@ -206,8 +236,10 @@ public class UserAwareBiometricSchedulerTest {
         @Override
         public void start(@NonNull Callback callback) {
             super.start(callback);
-            mUserStartedCallback.onUserStarted(getTargetUserId(), new Object());
+            if (mShouldFinish) {
-            callback.onClientFinished(this, true /* success */);
+                mUserStartedCallback.onUserStarted(getTargetUserId(), new Object());
+                callback.onClientFinished(this, true /* success */);
+            }
         }
 
         @Override