From 0b434e653e49d14574fe411c4cba8b4e9d2a646d Mon Sep 17 00:00:00 2001 From: Diya Bera Date: Thu, 26 Jan 2023 00:44:57 +0000 Subject: [PATCH] Reduce binder calls Caches SensorProperties and uses that to determine if hardware is detected in KeyguardUpdateMonitor Remove binder calls for getSensorPropertiesInternal Bug: 263281864 Bug: 263282558 Test: atest FingerprintManagerTest, atest FaceManagerTest Change-Id: Ib7a12e2c6eb39e8ca18b64cf0354d7e98578c116 --- .../android/hardware/face/FaceManager.java | 18 +++- .../fingerprint/FingerprintManager.java | 16 +++- .../keyguard/KeyguardUpdateMonitor.java | 16 ++-- .../keyguard/KeyguardUpdateMonitorTest.java | 2 - .../hardware/face/FaceManagerTest.java | 86 ++++++++++++++++++ .../src/android/hardware/face/OWNERS | 1 + .../fingerprint/FingerprintManagerTest.java | 88 +++++++++++++++++++ .../src/android/hardware/fingerprint/OWNERS | 1 + 8 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 services/tests/mockingservicestests/src/android/hardware/face/FaceManagerTest.java create mode 100644 services/tests/mockingservicestests/src/android/hardware/face/OWNERS create mode 100644 services/tests/mockingservicestests/src/android/hardware/fingerprint/FingerprintManagerTest.java create mode 100644 services/tests/mockingservicestests/src/android/hardware/fingerprint/OWNERS diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 197739b6a067c..96098f8535526 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -26,6 +26,7 @@ import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.annotation.SystemService; import android.content.Context; +import android.content.pm.PackageManager; import android.hardware.biometrics.BiometricAuthenticator; import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricFaceConstants; @@ -87,6 +88,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan private CryptoObject mCryptoObject; private Face mRemovalFace; private Handler mHandler; + private List mProps = new ArrayList<>(); private final IFaceServiceReceiver mServiceReceiver = new IFaceServiceReceiver.Stub() { @@ -168,6 +170,16 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan Slog.v(TAG, "FaceAuthenticationManagerService was null"); } mHandler = new MyHandler(context); + if (context.checkCallingOrSelfPermission(USE_BIOMETRIC_INTERNAL) + == PackageManager.PERMISSION_GRANTED) { + addAuthenticatorsRegisteredCallback(new IFaceAuthenticatorsRegisteredCallback.Stub() { + @Override + public void onAllAuthenticatorsRegistered( + @NonNull List sensors) { + mProps = sensors; + } + }); + } } /** @@ -664,14 +676,14 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan @NonNull public List getSensorPropertiesInternal() { try { - if (mService == null) { - return new ArrayList<>(); + if (!mProps.isEmpty() || mService == null) { + return mProps; } return mService.getSensorPropertiesInternal(mContext.getOpPackageName()); } catch (RemoteException e) { e.rethrowFromSystemServer(); } - return new ArrayList<>(); + return mProps; } /** diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index 04a204a09f5e9..edbe6eaee68ab 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -153,6 +153,7 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing @Nullable private RemoveTracker mRemoveTracker; private Handler mHandler; @Nullable private float[] mEnrollStageThresholds; + private List mProps = new ArrayList<>(); /** * Retrieves a list of properties for all fingerprint sensors on the device. @@ -1175,8 +1176,8 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing @NonNull public List getSensorPropertiesInternal() { try { - if (mService == null) { - return new ArrayList<>(); + if (!mProps.isEmpty() || mService == null) { + return mProps; } return mService.getSensorPropertiesInternal(mContext.getOpPackageName()); } catch (RemoteException e) { @@ -1485,6 +1486,17 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing Slog.v(TAG, "FingerprintService was null"); } mHandler = new MyHandler(context); + if (context.checkCallingOrSelfPermission(USE_BIOMETRIC_INTERNAL) + == PackageManager.PERMISSION_GRANTED) { + addAuthenticatorsRegisteredCallback( + new IFingerprintAuthenticatorsRegisteredCallback.Stub() { + @Override + public void onAllAuthenticatorsRegistered( + @NonNull List sensors) { + mProps = sensors; + } + }); + } } private int getCurrentUserId() { diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 9d6bb087288b6..836c1c4b69015 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -937,7 +937,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab @Override public void run() { mLogger.logRetryAfterFpHwUnavailable(mHardwareFingerprintUnavailableRetryCount); - if (mFpm.isHardwareDetected()) { + if (!mFingerprintSensorProperties.isEmpty()) { updateFingerprintListeningState(BIOMETRIC_ACTION_UPDATE); } else if (mHardwareFingerprintUnavailableRetryCount < HAL_ERROR_RETRY_MAX) { mHardwareFingerprintUnavailableRetryCount++; @@ -2352,14 +2352,13 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab } private void updateFaceEnrolled(int userId) { - mIsFaceEnrolled = whitelistIpcs( - () -> mFaceManager != null && mFaceManager.isHardwareDetected() - && mBiometricEnabledForUser.get(userId)) + mIsFaceEnrolled = mFaceManager != null && !mFaceSensorProperties.isEmpty() + && mBiometricEnabledForUser.get(userId) && mAuthController.isFaceAuthEnrolled(userId); } public boolean isFaceSupported() { - return mFaceManager != null && mFaceManager.isHardwareDetected(); + return mFaceManager != null && !mFaceSensorProperties.isEmpty(); } /** @@ -2975,7 +2974,8 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab @VisibleForTesting boolean isUnlockWithFingerprintPossible(int userId) { // TODO (b/242022358), make this rely on onEnrollmentChanged event and update it only once. - mIsUnlockWithFingerprintPossible.put(userId, mFpm != null && mFpm.isHardwareDetected() + mIsUnlockWithFingerprintPossible.put(userId, mFpm != null + && !mFingerprintSensorProperties.isEmpty() && !isFingerprintDisabled(userId) && mFpm.hasEnrolledTemplates(userId)); return mIsUnlockWithFingerprintPossible.get(userId); } @@ -3895,7 +3895,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab for (int subId : mServiceStates.keySet()) { pw.println(" " + subId + "=" + mServiceStates.get(subId)); } - if (mFpm != null && mFpm.isHardwareDetected()) { + if (mFpm != null && !mFingerprintSensorProperties.isEmpty()) { final int userId = mUserTracker.getUserId(); final int strongAuthFlags = mStrongAuthTracker.getStrongAuthForUser(userId); BiometricAuthenticated fingerprint = mUserFingerprintAuthenticated.get(userId); @@ -3944,7 +3944,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab mFingerprintListenBuffer.toList() ).printTableData(pw); } - if (mFaceManager != null && mFaceManager.isHardwareDetected()) { + if (mFaceManager != null && !mFaceSensorProperties.isEmpty()) { final int userId = mUserTracker.getUserId(); final int strongAuthFlags = mStrongAuthTracker.getStrongAuthForUser(userId); BiometricAuthenticated face = mUserFaceAuthenticated.get(userId); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java index 87dd6a4cfa5e8..3485d37dc34c0 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java @@ -96,7 +96,6 @@ import android.os.PowerManager; import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; -import android.provider.Settings; import android.service.dreams.IDreamManager; import android.service.trust.TrustAgentService; import android.telephony.ServiceState; @@ -700,7 +699,6 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { setKeyguardBouncerVisibility(true); verify(mFaceManager).authenticate(any(), any(), any(), any(), anyInt(), anyBoolean()); - verify(mFaceManager).isHardwareDetected(); verify(mFaceManager, never()).hasEnrolledTemplates(anyInt()); } diff --git a/services/tests/mockingservicestests/src/android/hardware/face/FaceManagerTest.java b/services/tests/mockingservicestests/src/android/hardware/face/FaceManagerTest.java new file mode 100644 index 0000000000000..f3feb02695985 --- /dev/null +++ b/services/tests/mockingservicestests/src/android/hardware/face/FaceManagerTest.java @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.face; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.os.Looper; +import android.os.RemoteException; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.MockitoRule; + +import java.util.ArrayList; +import java.util.List; + +@RunWith(MockitoJUnitRunner.class) +public class FaceManagerTest { + @Rule + public final MockitoRule mockito = MockitoJUnit.rule(); + + @Mock + Context mContext; + @Mock + IFaceService mService; + + @Captor + ArgumentCaptor mCaptor; + + List mProps; + FaceManager mFaceManager; + + @Before + public void setUp() throws Exception { + when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper()); + mFaceManager = new FaceManager(mContext, mService); + mProps = new ArrayList<>(); + mProps.add(new FaceSensorPropertiesInternal( + 0 /* id */, + FaceSensorProperties.STRENGTH_STRONG, + 1 /* maxTemplatesAllowed */, + new ArrayList<>() /* conponentInfo */, + FaceSensorProperties.TYPE_UNKNOWN, + true /* supportsFaceDetection */, + true /* supportsSelfIllumination */, + false /* resetLockoutRequiresChallenge */)); + } + + @Test + public void getSensorPropertiesInternal_noBinderCalls() throws RemoteException { + verify(mService).addAuthenticatorsRegisteredCallback(mCaptor.capture()); + + mCaptor.getValue().onAllAuthenticatorsRegistered(mProps); + List actual = mFaceManager.getSensorPropertiesInternal(); + + assertThat(actual).isEqualTo(mProps); + verify(mService, never()).getSensorPropertiesInternal(any()); + } +} diff --git a/services/tests/mockingservicestests/src/android/hardware/face/OWNERS b/services/tests/mockingservicestests/src/android/hardware/face/OWNERS new file mode 100644 index 0000000000000..6a2192a2c7fbc --- /dev/null +++ b/services/tests/mockingservicestests/src/android/hardware/face/OWNERS @@ -0,0 +1 @@ +include /services/core/java/com/android/server/biometrics/OWNERS diff --git a/services/tests/mockingservicestests/src/android/hardware/fingerprint/FingerprintManagerTest.java b/services/tests/mockingservicestests/src/android/hardware/fingerprint/FingerprintManagerTest.java new file mode 100644 index 0000000000000..558202d51b27f --- /dev/null +++ b/services/tests/mockingservicestests/src/android/hardware/fingerprint/FingerprintManagerTest.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.fingerprint; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.os.Looper; +import android.os.RemoteException; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.MockitoRule; + +import java.util.ArrayList; +import java.util.List; + + +@RunWith(MockitoJUnitRunner.class) +public class FingerprintManagerTest { + @Rule + public final MockitoRule mockito = MockitoJUnit.rule(); + + @Mock + Context mContext; + @Mock + IFingerprintService mService; + + @Captor + ArgumentCaptor mCaptor; + + List mProps; + FingerprintManager mFingerprintManager; + + @Before + public void setUp() throws Exception { + when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper()); + mFingerprintManager = new FingerprintManager(mContext, mService); + mProps = new ArrayList<>(); + mProps.add(new FingerprintSensorPropertiesInternal( + 0 /* sensorId */, + FingerprintSensorProperties.STRENGTH_STRONG, + 1 /* maxEnrollmentsPerUser */, + new ArrayList<>() /* componentInfo */, + FingerprintSensorProperties.TYPE_UNKNOWN, + true /* halControlsIllumination */, + true /* resetLockoutRequiresHardwareAuthToken */, + new ArrayList<>() /* sensorLocations */)); + } + + @Test + public void getSensorPropertiesInternal_noBinderCalls() throws RemoteException { + verify(mService).addAuthenticatorsRegisteredCallback(mCaptor.capture()); + + mCaptor.getValue().onAllAuthenticatorsRegistered(mProps); + List actual = + mFingerprintManager.getSensorPropertiesInternal(); + + assertThat(actual).isEqualTo(mProps); + verify(mService, never()).getSensorPropertiesInternal(any()); + } +} diff --git a/services/tests/mockingservicestests/src/android/hardware/fingerprint/OWNERS b/services/tests/mockingservicestests/src/android/hardware/fingerprint/OWNERS new file mode 100644 index 0000000000000..3edcf70d7e672 --- /dev/null +++ b/services/tests/mockingservicestests/src/android/hardware/fingerprint/OWNERS @@ -0,0 +1 @@ +include /services/core/java/com/android/server/biometrics/OWNERS \ No newline at end of file