Merge "Ensure current operation has not changed on start/stop user operations." into sc-dev am: 92ce6e953e

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15180201

Change-Id: I09ae59306dd99fd85f942cded6c5fbd6e15ec234

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

@@ -62,16 +62,22 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
         @Override
         public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) {
             mHandler.post(() -> {
-                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);
+                if (mOwner != clientMonitor) {
+                    Slog.e(getTag(), "[Wrong client finished], actual: "
+                            + clientMonitor + ", expected: " + mOwner);
+                    return;
                 }
 
-                startNextOperationIfIdle();
+                Slog.d(getTag(), "[Client finished] "
+                        + clientMonitor + ", success: " + success);
+                if (mCurrentOperation != null && mCurrentOperation.mClientMonitor == mOwner) {
+                    mCurrentOperation = null;
+                    startNextOperationIfIdle();
+                } else {
+                    // can usually be ignored (hal died, etc.)
+                    Slog.d(getTag(), "operation is already null or different (reset?): "
+                            + mCurrentOperation);
+                }
             });
         }
     }
@@ -125,9 +131,9 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
                     new ClientFinishedCallback(startClient);
 
             Slog.d(getTag(), "[Starting User] " + startClient);
-            startClient.start(finishedCallback);
             mCurrentOperation = new Operation(
                     startClient, finishedCallback, Operation.STATE_STARTED);
+            startClient.start(finishedCallback);
         } else {
             if (mStopUserClient != null) {
                 Slog.d(getTag(), "[Waiting for StopUser] " + mStopUserClient);
@@ -139,9 +145,9 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
 
                 Slog.d(getTag(), "[Stopping User] current: " + currentUserId
                         + ", next: " + nextUserId + ". " + mStopUserClient);
-                mStopUserClient.start(finishedCallback);
                 mCurrentOperation = new Operation(
                         mStopUserClient, finishedCallback, Operation.STATE_STARTED);
+                mStopUserClient.start(finishedCallback);
             }
         }
     }

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

@@ -17,6 +17,8 @@
 package com.android.server.biometrics.sensors;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -132,6 +134,28 @@ public class UserAwareBiometricSchedulerTest {
         }
     }
 
+    @Test
+    public void testScheduleOperation_whenNoUser_notStarted_andReset() {
+        mCurrentUserId = UserHandle.USER_NULL;
+        mStartOperationsFinish = false;
+
+        final BaseClientMonitor client = mock(BaseClientMonitor.class);
+        when(client.getTargetUserId()).thenReturn(5);
+        mScheduler.scheduleClientMonitor(client);
+        waitForIdle();
+
+        final TestStartUserClient startUserClient =
+                (TestStartUserClient) mScheduler.mCurrentOperation.mClientMonitor;
+        mScheduler.reset();
+        assertNull(mScheduler.mCurrentOperation);
+
+        final BiometricScheduler.Operation fakeOperation = new BiometricScheduler.Operation(
+                mock(BaseClientMonitor.class), new BaseClientMonitor.Callback() {});
+        mScheduler.mCurrentOperation = fakeOperation;
+        startUserClient.mCallback.onClientFinished(startUserClient, true);
+        assertSame(fakeOperation, mScheduler.mCurrentOperation);
+    }
+
     @Test
     public void testScheduleOperation_whenSameUser() {
         mCurrentUserId = 10;
@@ -173,7 +197,6 @@ public class UserAwareBiometricSchedulerTest {
     }
 
     private class TestUserStoppedCallback implements StopUserClient.UserStoppedCallback {
-
         int numInvocations;
 
         @Override
@@ -184,7 +207,6 @@ public class UserAwareBiometricSchedulerTest {
     }
 
     private class TestUserStartedCallback implements StartUserClient.UserStartedCallback<Object> {
-
         int numInvocations;
 
         @Override
@@ -221,6 +243,8 @@ public class UserAwareBiometricSchedulerTest {
     private static class TestStartUserClient extends StartUserClient<Object, Object> {
         private final boolean mShouldFinish;
 
+        Callback mCallback;
+
         public TestStartUserClient(@NonNull Context context,
                 @NonNull LazyDaemon<Object> lazyDaemon, @Nullable IBinder token, int userId,
                 int sensorId, @NonNull UserStartedCallback<Object> callback, boolean shouldFinish) {
@@ -236,6 +260,8 @@ public class UserAwareBiometricSchedulerTest {
         @Override
         public void start(@NonNull Callback callback) {
             super.start(callback);
+
+            mCallback = callback;
             if (mShouldFinish) {
                 mUserStartedCallback.onUserStarted(getTargetUserId(), new Object());
                 callback.onClientFinished(this, true /* success */);