Merge "Always invoke start next operation from start & stop user clients." into tm-dev am: 41eda41580

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

Change-Id: I05f6c483e26a8e96bec106782b9339020cb712c7

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

@@ -57,30 +57,25 @@ public class UserAwareBiometricScheduler extends BiometricScheduler {
     @Nullable private StopUserClient<?> mStopUserClient;
 
     private class ClientFinishedCallback implements ClientMonitorCallback {
-        private final BaseClientMonitor mOwner;
+        @NonNull private final BaseClientMonitor mOwner;
 
-        ClientFinishedCallback(BaseClientMonitor owner) {
+        ClientFinishedCallback(@NonNull BaseClientMonitor owner) {
             mOwner = owner;
         }
 
         @Override
         public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) {
             mHandler.post(() -> {
-                if (mOwner != clientMonitor) {
-                    Slog.e(getTag(), "[Wrong client finished], actual: "
-                            + clientMonitor + ", expected: " + mOwner);
-                    return;
-                }
-
                 Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success);
                 if (mCurrentOperation != null && mCurrentOperation.isFor(mOwner)) {
                     mCurrentOperation = null;
-                    startNextOperationIfIdle();
                 } else {
-                    // can usually be ignored (hal died, etc.)
+                    // can happen if the hal dies and is usually okay
-                    Slog.d(getTag(), "operation is already null or different (reset?): "
+                    // do not unset the current operation that may be newer
+                    Slog.w(getTag(), "operation is already null or different (reset?): "
                             + mCurrentOperation);
                 }
+                startNextOperationIfIdle();
             });
         }
     }

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

@@ -18,9 +18,8 @@ package com.android.server.biometrics.sensors;
 
 import static android.testing.TestableLooper.RunWithLooper;
 
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.Truth.assertThat;
-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;
@@ -45,11 +44,15 @@ import com.android.server.biometrics.log.BiometricContext;
 import com.android.server.biometrics.log.BiometricLogger;
 
 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;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.function.Supplier;
 
 @Presubmit
@@ -61,9 +64,12 @@ public class UserAwareBiometricSchedulerTest {
     private static final String TAG = "UserAwareBiometricSchedulerTest";
     private static final int TEST_SENSOR_ID = 0;
 
+    @Rule
+    public final MockitoRule mockito = MockitoJUnit.rule();
+
     private Handler mHandler;
     private UserAwareBiometricScheduler mScheduler;
-    private IBinder mToken = new Binder();
+    private final IBinder mToken = new Binder();
 
     @Mock
     private Context mContext;
@@ -74,15 +80,14 @@ public class UserAwareBiometricSchedulerTest {
     @Mock
     private BiometricContext mBiometricContext;
 
-    private TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback();
+    private final TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback();
-    private TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback();
+    private final TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback();
     private int mCurrentUserId = UserHandle.USER_NULL;
     private boolean mStartOperationsFinish = true;
     private int mStartUserClientCount = 0;
 
     @Before
     public void setUp() {
-        MockitoAnnotations.initMocks(this);
         mHandler = new Handler(TestableLooper.get(this).getLooper());
         mScheduler = new UserAwareBiometricScheduler(TAG,
                 mHandler,
@@ -121,8 +126,8 @@ public class UserAwareBiometricSchedulerTest {
         mScheduler.scheduleClientMonitor(nextClient);
         waitForIdle();
 
-        assertEquals(0, mUserStoppedCallback.numInvocations);
+        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);
-        assertEquals(1, mUserStartedCallback.numInvocations);
+        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(0);
         verify(nextClient).start(any());
     }
 
@@ -142,9 +147,9 @@ public class UserAwareBiometricSchedulerTest {
             waitForIdle();
         }
 
-        assertEquals(0, mUserStoppedCallback.numInvocations);
+        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);
-        assertEquals(0, mUserStartedCallback.numInvocations);
+        assertThat(mUserStartedCallback.mStartedUsers).isEmpty();
-        assertEquals(1, mStartUserClientCount);
+        assertThat(mStartUserClientCount).isEqualTo(1);
         for (BaseClientMonitor client : nextClients) {
             verify(client, never()).start(any());
         }
@@ -163,13 +168,13 @@ public class UserAwareBiometricSchedulerTest {
         final TestStartUserClient startUserClient =
                 (TestStartUserClient) mScheduler.mCurrentOperation.getClientMonitor();
         mScheduler.reset();
-        assertNull(mScheduler.mCurrentOperation);
+        assertThat(mScheduler.mCurrentOperation).isNull();
 
         final BiometricSchedulerOperation fakeOperation = new BiometricSchedulerOperation(
                 mock(BaseClientMonitor.class), new ClientMonitorCallback() {});
         mScheduler.mCurrentOperation = fakeOperation;
         startUserClient.mCallback.onClientFinished(startUserClient, true);
-        assertSame(fakeOperation, mScheduler.mCurrentOperation);
+        assertThat(fakeOperation).isSameInstanceAs(mScheduler.mCurrentOperation);
     }
 
     @Test
@@ -184,8 +189,8 @@ public class UserAwareBiometricSchedulerTest {
         waitForIdle();
 
         verify(nextClient).start(any());
-        assertEquals(0, mUserStoppedCallback.numInvocations);
+        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);
-        assertEquals(0, mUserStartedCallback.numInvocations);
+        assertThat(mUserStartedCallback.mStartedUsers).isEmpty();
     }
 
     @Test
@@ -199,36 +204,67 @@ public class UserAwareBiometricSchedulerTest {
         mScheduler.scheduleClientMonitor(nextClient);
 
         waitForIdle();
-        assertEquals(1, mUserStoppedCallback.numInvocations);
+        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1);
 
         waitForIdle();
-        assertEquals(1, mUserStartedCallback.numInvocations);
+        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(nextUserId);
 
         waitForIdle();
         verify(nextClient).start(any());
     }
 
+    @Test
+    public void testStartUser_alwaysStartsNextOperation() {
+        BaseClientMonitor nextClient = mock(BaseClientMonitor.class);
+        when(nextClient.getTargetUserId()).thenReturn(10);
+
+        mScheduler.scheduleClientMonitor(nextClient);
+
+        waitForIdle();
+        verify(nextClient).start(any());
+
+        // finish first operation
+        mScheduler.getInternalCallback().onClientFinished(nextClient, true /* success */);
+        waitForIdle();
+
+        // schedule second operation but swap out the current operation
+        // before it runs so that it's not current when it's completion callback runs
+        nextClient = mock(BaseClientMonitor.class);
+        when(nextClient.getTargetUserId()).thenReturn(11);
+        mUserStartedCallback.mAfterStart = () -> mScheduler.mCurrentOperation = null;
+        mScheduler.scheduleClientMonitor(nextClient);
+
+        waitForIdle();
+        verify(nextClient).start(any());
+        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(10, 11).inOrder();
+        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1);
+    }
+
     private void waitForIdle() {
         TestableLooper.get(this).processAllMessages();
     }
 
     private class TestUserStoppedCallback implements StopUserClient.UserStoppedCallback {
-        int numInvocations;
+        int mNumInvocations;
 
         @Override
         public void onUserStopped() {
-            numInvocations++;
+            mNumInvocations++;
             mCurrentUserId = UserHandle.USER_NULL;
         }
     }
 
     private class TestUserStartedCallback implements StartUserClient.UserStartedCallback<Object> {
-        int numInvocations;
+        final List<Integer> mStartedUsers = new ArrayList<>();
+        Runnable mAfterStart = null;
 
         @Override
         public void onUserStarted(int newUserId, Object newObject, int halInterfaceVersion) {
-            numInvocations++;
+            mStartedUsers.add(newUserId);
             mCurrentUserId = newUserId;
+            if (mAfterStart != null) {
+                mAfterStart.run();
+            }
         }
     }