Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0aa23c28 authored by Ilya Matyukhin's avatar Ilya Matyukhin
Browse files

Fix mismatched state between SystemUI and FingerprintService 

This CL fixes issues that lead to occasional fingerprint enrollment
state inconsistencies between SystemUI and FingerprintService:

1. Unordered processing of "onAllAuthenticatorsRegistered" and
   "onEnrollmentsChanged" in AuthController.
2. Data races in AuthController caused by unguarded access between the
   main and incoming Binder threads.

Bug: 207726663
Test: atest AuthControllerTest
Test: add artificial 10 sec delay to fingerprint HAL start up
Change-Id: I9dcb80443996fcdefcba976b0f60cc6f2d036afd
parent 8b988e72

packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java

+79 −56
Original line number Diff line number Diff line
@@ -49,7 +49,6 @@ import android.hardware.fingerprint.IFingerprintAuthenticatorsRegisteredCallback 
import android.hardware.fingerprint.IUdfpsHbmListener;

import android.os.Bundle;

import android.os.Handler;

import android.os.Looper;

import android.os.RemoteException;

import android.util.Log;

import android.util.SparseBooleanArray;
@@ -65,6 +64,7 @@ import com.android.systemui.dagger.SysUISingleton; 
import com.android.systemui.dagger.qualifiers.Main;

import com.android.systemui.doze.DozeReceiver;

import com.android.systemui.statusbar.CommandQueue;

import com.android.systemui.util.concurrency.Execution;



import java.util.ArrayList;

import java.util.Arrays;
@@ -92,15 +92,20 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
    private static final boolean DEBUG = true;

    private static final int SENSOR_PRIVACY_DELAY = 500;



    private final Handler mHandler = new Handler(Looper.getMainLooper());

    private final Handler mHandler;

    private final Execution mExecution;

    private final CommandQueue mCommandQueue;

    private final ActivityTaskManager mActivityTaskManager;

    @Nullable private final FingerprintManager mFingerprintManager;

    @Nullable private final FaceManager mFaceManager;

    @Nullable

    private final FingerprintManager mFingerprintManager;

    @Nullable

    private final FaceManager mFaceManager;

    private final Provider<UdfpsController> mUdfpsControllerFactory;

    private final Provider<SidefpsController> mSidefpsControllerFactory;

    @Nullable private final PointF mFaceAuthSensorLocation;

    @Nullable private PointF mFingerprintLocation;

    @Nullable

    private final PointF mFaceAuthSensorLocation;

    @Nullable

    private PointF mFingerprintLocation;

    private final Set<Callback> mCallbacks = new HashSet<>();



    // TODO: These should just be saved from onSaveState
@@ -133,62 +138,27 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
        }

    }



    private final FingerprintStateListener mFingerprintStateListener =

            new FingerprintStateListener() {

        @Override

        public void onEnrollmentsChanged(int userId, int sensorId, boolean hasEnrollments) {

            Log.d(TAG, "onEnrollmentsChanged, userId: " + userId

                    + ", sensorId: " + sensorId

                    + ", hasEnrollments: " + hasEnrollments);

            for (FingerprintSensorPropertiesInternal prop : mUdfpsProps) {

                if (prop.sensorId == sensorId) {

                    mUdfpsEnrolledForUser.put(userId, hasEnrollments);

                }

            }



            for (Callback cb : mCallbacks) {

                cb.onEnrollmentsChanged();

            }

        }

    };



    @NonNull

    private final IFingerprintAuthenticatorsRegisteredCallback

            mFingerprintAuthenticatorsRegisteredCallback =

            new IFingerprintAuthenticatorsRegisteredCallback.Stub() {

                @Override public void onAllAuthenticatorsRegistered(

                @Override

                public void onAllAuthenticatorsRegistered(

                        List<FingerprintSensorPropertiesInternal> sensors) {

                    if (DEBUG) {

                        Log.d(TAG, "onFingerprintProvidersAvailable | sensors: " + Arrays.toString(

                                sensors.toArray()));

                    }

                    mFpProps = sensors;

                    List<FingerprintSensorPropertiesInternal> udfpsProps = new ArrayList<>();

                    List<FingerprintSensorPropertiesInternal> sidefpsProps = new ArrayList<>();

                    for (FingerprintSensorPropertiesInternal props : mFpProps) {

                        if (props.isAnyUdfpsType()) {

                            udfpsProps.add(props);

                        }

                        if (props.isAnySidefpsType()) {

                            sidefpsProps.add(props);

                        }

                    }

                    mUdfpsProps = !udfpsProps.isEmpty() ? udfpsProps : null;

                    if (mUdfpsProps != null) {

                        mUdfpsController = mUdfpsControllerFactory.get();

                    }

                    mSidefpsProps = !sidefpsProps.isEmpty() ? sidefpsProps : null;

                    if (mSidefpsProps != null) {

                        mSidefpsController = mSidefpsControllerFactory.get();

                    mHandler.post(() -> handleAllAuthenticatorsRegistered(sensors));

                }

            };



                    for (Callback cb : mCallbacks) {

                        cb.onAllAuthenticatorsRegistered();

                    }

    private final FingerprintStateListener mFingerprintStateListener =

            new FingerprintStateListener() {

                @Override

                public void onEnrollmentsChanged(int userId, int sensorId, boolean hasEnrollments) {

                    mHandler.post(

                            () -> handleEnrollmentsChanged(userId, sensorId, hasEnrollments));

                }

            };



    @VisibleForTesting final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {

    @VisibleForTesting

    final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {

        @Override

        public void onReceive(Context context, Intent intent) {

            if (mCurrentDialog != null
@@ -212,6 +182,7 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
    };



    private void handleTaskStackChanged() {

        mExecution.assertIsMainThread();

        if (mCurrentDialog != null) {

            try {

                final String clientPackage = mCurrentDialog.getOpPackageName();
@@ -241,6 +212,56 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
        }

    }



    private void handleAllAuthenticatorsRegistered(

            List<FingerprintSensorPropertiesInternal> sensors) {

        mExecution.assertIsMainThread();

        if (DEBUG) {

            Log.d(TAG, "handleAllAuthenticatorsRegistered | sensors: " + Arrays.toString(

                    sensors.toArray()));

        }

        mFpProps = sensors;

        List<FingerprintSensorPropertiesInternal> udfpsProps = new ArrayList<>();

        List<FingerprintSensorPropertiesInternal> sidefpsProps = new ArrayList<>();

        for (FingerprintSensorPropertiesInternal props : mFpProps) {

            if (props.isAnyUdfpsType()) {

                udfpsProps.add(props);

            }

            if (props.isAnySidefpsType()) {

                sidefpsProps.add(props);

            }

        }

        mUdfpsProps = !udfpsProps.isEmpty() ? udfpsProps : null;

        if (mUdfpsProps != null) {

            mUdfpsController = mUdfpsControllerFactory.get();

        }

        mSidefpsProps = !sidefpsProps.isEmpty() ? sidefpsProps : null;

        if (mSidefpsProps != null) {

            mSidefpsController = mSidefpsControllerFactory.get();

        }

        for (Callback cb : mCallbacks) {

            cb.onAllAuthenticatorsRegistered();

        }

        mFingerprintManager.registerFingerprintStateListener(mFingerprintStateListener);

    }



    private void handleEnrollmentsChanged(int userId, int sensorId, boolean hasEnrollments) {

        mExecution.assertIsMainThread();

        Log.d(TAG, "handleEnrollmentsChanged, userId: " + userId + ", sensorId: " + sensorId

                + ", hasEnrollments: " + hasEnrollments);

        if (mUdfpsProps == null) {

            Log.d(TAG, "handleEnrollmentsChanged, mUdfpsProps is null");

        } else {

            for (FingerprintSensorPropertiesInternal prop : mUdfpsProps) {

                if (prop.sensorId == sensorId) {

                    mUdfpsEnrolledForUser.put(userId, hasEnrollments);

                }

            }

        }

        for (Callback cb : mCallbacks) {

            cb.onEnrollmentsChanged();

        }

    }



    /**

     * Adds a callback. See {@link Callback}.

     */
@@ -449,6 +470,7 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 


    @Inject

    public AuthController(Context context,

            Execution execution,

            CommandQueue commandQueue,

            ActivityTaskManager activityTaskManager,

            @NonNull WindowManager windowManager,
@@ -459,6 +481,8 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
            @NonNull DisplayManager displayManager,

            @Main Handler handler) {

        super(context);

        mExecution = execution;

        mHandler = handler;

        mCommandQueue = commandQueue;

        mActivityTaskManager = activityTaskManager;

        mFingerprintManager = fingerprintManager;
@@ -470,7 +494,7 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
        mOrientationListener = new BiometricDisplayListener(

                context,

                displayManager,

                handler,

                mHandler,

                BiometricDisplayListener.SensorType.Generic.INSTANCE,

                () -> {

                    onOrientationChanged();
@@ -521,7 +545,6 @@ public class AuthController extends SystemUI implements CommandQueue.Callbacks, 
        if (mFingerprintManager != null) {

            mFingerprintManager.addAuthenticatorsRegisteredCallback(

                    mFingerprintAuthenticatorsRegisteredCallback);

            mFingerprintManager.registerFingerprintStateListener(mFingerprintStateListener);

        }



        mTaskStackListener = new BiometricTaskStackListener();

packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthControllerTest.java

+79 −9
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import static org.mockito.ArgumentMatchers.eq; 
import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.reset;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;
@@ -53,12 +54,14 @@ import android.hardware.face.FaceManager; 
import android.hardware.fingerprint.FingerprintManager;

import android.hardware.fingerprint.FingerprintSensorProperties;

import android.hardware.fingerprint.FingerprintSensorPropertiesInternal;

import android.hardware.fingerprint.FingerprintStateListener;

import android.hardware.fingerprint.IFingerprintAuthenticatorsRegisteredCallback;

import android.os.Bundle;

import android.os.Handler;

import android.os.RemoteException;

import android.testing.AndroidTestingRunner;

import android.testing.TestableContext;

import android.testing.TestableLooper;

import android.testing.TestableLooper.RunWithLooper;

import android.view.WindowManager;
@@ -67,6 +70,8 @@ import androidx.test.filters.SmallTest; 
import com.android.internal.R;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.statusbar.CommandQueue;

import com.android.systemui.util.concurrency.Execution;

import com.android.systemui.util.concurrency.FakeExecution;



import org.junit.Before;

import org.junit.Test;
@@ -112,20 +117,27 @@ public class AuthControllerTest extends SysuiTestCase { 
    private SidefpsController mSidefpsController;

    @Mock

    private DisplayManager mDisplayManager;

    @Mock

    private Handler mHandler;

    @Captor

    ArgumentCaptor<IFingerprintAuthenticatorsRegisteredCallback> mAuthenticatorsRegisteredCaptor;

    @Captor

    ArgumentCaptor<FingerprintStateListener> mFingerprintStateCaptor;



    private TestableContext mContextSpy;

    private Execution mExecution;

    private TestableLooper mTestableLooper;

    private Handler mHandler;

    private TestableAuthController mAuthController;



    @Before

    public void setup() throws RemoteException {

        MockitoAnnotations.initMocks(this);



        TestableContext context = spy(mContext);

        mContextSpy = spy(mContext);

        mExecution = new FakeExecution();

        mTestableLooper = TestableLooper.get(this);

        mHandler = new Handler(mTestableLooper.getLooper());



        when(context.getPackageManager()).thenReturn(mPackageManager);

        when(mContextSpy.getPackageManager()).thenReturn(mPackageManager);

        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_FACE))

                .thenReturn(true);

        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_FINGERPRINT))
@@ -158,18 +170,75 @@ public class AuthControllerTest extends SysuiTestCase { 
        props.add(prop);

        when(mFingerprintManager.getSensorPropertiesInternal()).thenReturn(props);



        mAuthController = new TestableAuthController(context, mCommandQueue,

        mAuthController = new TestableAuthController(mContextSpy, mExecution, mCommandQueue,

                mActivityTaskManager, mWindowManager, mFingerprintManager, mFaceManager,

                () -> mUdfpsController, () -> mSidefpsController);



        mAuthController.start();

        verify(mFingerprintManager).addAuthenticatorsRegisteredCallback(

                mAuthenticatorsRegisteredCaptor.capture());



        mAuthenticatorsRegisteredCaptor.getValue().onAllAuthenticatorsRegistered(props);



        // Ensures that the operations posted on the handler get executed.

        mTestableLooper.processAllMessages();

    }



    // Callback tests



    @Test

    public void testRegistersFingerprintStateListener_afterAllAuthenticatorsAreRegistered()

            throws RemoteException {

        // This test is sensitive to prior FingerprintManager interactions.

        reset(mFingerprintManager);



        // This test requires an uninitialized AuthController.

        AuthController authController = new TestableAuthController(mContextSpy, mExecution,

                mCommandQueue, mActivityTaskManager, mWindowManager, mFingerprintManager,

                mFaceManager, () -> mUdfpsController, () -> mSidefpsController);

        authController.start();



        verify(mFingerprintManager).addAuthenticatorsRegisteredCallback(

                mAuthenticatorsRegisteredCaptor.capture());

        mTestableLooper.processAllMessages();



        verify(mFingerprintManager, never()).registerFingerprintStateListener(any());



        mAuthenticatorsRegisteredCaptor.getValue().onAllAuthenticatorsRegistered(new ArrayList<>());

        mTestableLooper.processAllMessages();



        verify(mFingerprintManager).registerFingerprintStateListener(any());

    }



    @Test

    public void testDoesNotCrash_afterEnrollmentsChangedForUnknownSensor() throws RemoteException {

        // This test is sensitive to prior FingerprintManager interactions.

        reset(mFingerprintManager);



        // This test requires an uninitialized AuthController.

        AuthController authController = new TestableAuthController(mContextSpy, mExecution,

                mCommandQueue, mActivityTaskManager, mWindowManager, mFingerprintManager,

                mFaceManager, () -> mUdfpsController, () -> mSidefpsController);

        authController.start();



        verify(mFingerprintManager).addAuthenticatorsRegisteredCallback(

                mAuthenticatorsRegisteredCaptor.capture());



        // Emulates a device with no authenticators (empty list).

        mAuthenticatorsRegisteredCaptor.getValue().onAllAuthenticatorsRegistered(new ArrayList<>());

        mTestableLooper.processAllMessages();



        verify(mFingerprintManager).registerFingerprintStateListener(

                mFingerprintStateCaptor.capture());



        // Enrollments changed for an unknown sensor.

        mFingerprintStateCaptor.getValue().onEnrollmentsChanged(0 /* userId */,

                0xbeef /* sensorId */, true /* hasEnrollments */);

        mTestableLooper.processAllMessages();



        // Nothing should crash.

    }



    @Test

    public void testSendsReasonUserCanceled_whenDismissedByUserCancel() throws Exception {

        showDialog(new int[]{1} /* sensorIds */, false /* credentialAllowed */);
@@ -497,7 +566,7 @@ public class AuthControllerTest extends SysuiTestCase { 
        when(mActivityTaskManager.getTasks(anyInt())).thenReturn(tasks);



        mAuthController.mTaskStackListener.onTaskStackChanged();

        waitForIdleSync();

        mTestableLooper.processAllMessages();



        assertNull(mAuthController.mCurrentDialog);

        assertNull(mAuthController.mReceiver);
@@ -528,7 +597,7 @@ public class AuthControllerTest extends SysuiTestCase { 
        showDialog(new int[] {1} /* sensorIds */, false /* credentialAllowed */);

        Intent intent = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS);

        mAuthController.mBroadcastReceiver.onReceive(mContext, intent);

        waitForIdleSync();

        mTestableLooper.processAllMessages();



        assertNull(mAuthController.mCurrentDialog);

        assertNull(mAuthController.mReceiver);
@@ -598,6 +667,7 @@ public class AuthControllerTest extends SysuiTestCase { 
        private PromptInfo mLastBiometricPromptInfo;



        TestableAuthController(Context context,

                Execution execution,

                CommandQueue commandQueue,

                ActivityTaskManager activityTaskManager,

                WindowManager windowManager,
@@ -605,7 +675,7 @@ public class AuthControllerTest extends SysuiTestCase { 
                FaceManager faceManager,

                Provider<UdfpsController> udfpsControllerFactory,

                Provider<SidefpsController> sidefpsControllerFactory) {

            super(context, commandQueue, activityTaskManager, windowManager,

            super(context, execution, commandQueue, activityTaskManager, windowManager,

                    fingerprintManager, faceManager, udfpsControllerFactory,

                    sidefpsControllerFactory, mDisplayManager, mHandler);

        }