From ae2482212dc5fdb4b0accfdb4a75b4f0c5196600 Mon Sep 17 00:00:00 2001 From: kholoud mohamed Date: Wed, 6 Apr 2022 17:08:08 +0100 Subject: [PATCH] Move device management resource retrieval calls to bg thread Fixes: 217176608 Test: added Assert.isNotMainThread and manually verified it doesn't crash Change-Id: I506123117567128fc4cb0a712d6c8c0ff683d20f --- .../biometrics/AuthContainerView.java | 13 +++-- .../systemui/biometrics/AuthController.java | 18 ++++--- .../biometrics/AuthCredentialView.java | 52 ++++++++++++------- .../android/systemui/qs/QSSecurityFooter.java | 38 ++++++++------ .../statusbar/phone/PhoneStatusBarPolicy.java | 9 ++-- .../biometrics/AuthContainerViewTest.kt | 11 ++-- .../biometrics/AuthControllerTest.java | 10 +++- 7 files changed, 97 insertions(+), 54 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthContainerView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthContainerView.java index b2673e9230086..233f3648aeb39 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthContainerView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthContainerView.java @@ -56,7 +56,9 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.widget.LockPatternUtils; import com.android.systemui.R; import com.android.systemui.animation.Interpolators; +import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.keyguard.WakefulnessLifecycle; +import com.android.systemui.util.concurrency.DelayableExecutor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -110,6 +112,8 @@ public class AuthContainerView extends LinearLayout @ContainerState private int mContainerState = STATE_UNKNOWN; private final Set mFailedModalities = new HashSet(); + private final @Background DelayableExecutor mBackgroundExecutor; + // Non-null only if the dialog is in the act of dismissing and has not sent the reason yet. @Nullable @AuthDialogCallback.DismissedReason private Integer mPendingCallbackReason; // HAT received from LockSettingsService when credential is verified. @@ -192,7 +196,7 @@ public class AuthContainerView extends LinearLayout return this; } - public AuthContainerView build(int[] sensorIds, + public AuthContainerView build(@Background DelayableExecutor bgExecutor, int[] sensorIds, @Nullable List fpProps, @Nullable List faceProps, @NonNull WakefulnessLifecycle wakefulnessLifecycle, @@ -200,7 +204,7 @@ public class AuthContainerView extends LinearLayout @NonNull LockPatternUtils lockPatternUtils) { mConfig.mSensorIds = sensorIds; return new AuthContainerView(mConfig, fpProps, faceProps, wakefulnessLifecycle, - userManager, lockPatternUtils, new Handler(Looper.getMainLooper())); + userManager, lockPatternUtils, new Handler(Looper.getMainLooper()), bgExecutor); } } @@ -253,7 +257,8 @@ public class AuthContainerView extends LinearLayout @NonNull WakefulnessLifecycle wakefulnessLifecycle, @NonNull UserManager userManager, @NonNull LockPatternUtils lockPatternUtils, - @NonNull Handler mainHandler) { + @NonNull Handler mainHandler, + @NonNull @Background DelayableExecutor bgExecutor) { super(config.mContext); mConfig = config; @@ -277,6 +282,7 @@ public class AuthContainerView extends LinearLayout mBackgroundView = mFrameLayout.findViewById(R.id.background); mPanelView = mFrameLayout.findViewById(R.id.panel); mPanelController = new AuthPanelController(mContext, mPanelView); + mBackgroundExecutor = bgExecutor; // Inflate biometric view only if necessary. if (Utils.isBiometricAllowed(mConfig.mPromptInfo)) { @@ -384,6 +390,7 @@ public class AuthContainerView extends LinearLayout mCredentialView.setPromptInfo(mConfig.mPromptInfo); mCredentialView.setPanelController(mPanelController, animatePanel); mCredentialView.setShouldAnimateContents(animateContents); + mCredentialView.setBackgroundExecutor(mBackgroundExecutor); mFrameLayout.addView(mCredentialView); } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java index aaf18b309db2e..b05bc245a79fa 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java @@ -64,11 +64,13 @@ import com.android.internal.widget.LockPatternUtils; import com.android.systemui.CoreStartable; import com.android.systemui.assist.ui.DisplayUtils; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.doze.DozeReceiver; import com.android.systemui.keyguard.WakefulnessLifecycle; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.CommandQueue; +import com.android.systemui.util.concurrency.DelayableExecutor; import com.android.systemui.util.concurrency.Execution; import java.util.ArrayList; @@ -139,6 +141,7 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba private boolean mAllAuthenticatorsRegistered; @NonNull private final UserManager mUserManager; @NonNull private final LockPatternUtils mLockPatternUtils; + private final @Background DelayableExecutor mBackgroundExecutor; @VisibleForTesting final TaskStackListener mTaskStackListener = new TaskStackListener() { @@ -507,13 +510,15 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba @NonNull UserManager userManager, @NonNull LockPatternUtils lockPatternUtils, @NonNull StatusBarStateController statusBarStateController, - @Main Handler handler) { + @Main Handler handler, + @Background DelayableExecutor bgExecutor) { super(context); mExecution = execution; mWakefulnessLifecycle = wakefulnessLifecycle; mUserManager = userManager; mLockPatternUtils = lockPatternUtils; mHandler = handler; + mBackgroundExecutor = bgExecutor; mCommandQueue = commandQueue; mActivityTaskManager = activityTaskManager; mFingerprintManager = fingerprintManager; @@ -839,6 +844,7 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba // Create a new dialog but do not replace the current one yet. final AuthDialog newDialog = buildDialog( + mBackgroundExecutor, promptInfo, requireConfirmation, userId, @@ -934,9 +940,9 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba } } - protected AuthDialog buildDialog(PromptInfo promptInfo, boolean requireConfirmation, - int userId, int[] sensorIds, String opPackageName, - boolean skipIntro, long operationId, long requestId, + protected AuthDialog buildDialog(@Background DelayableExecutor bgExecutor, + PromptInfo promptInfo, boolean requireConfirmation, int userId, int[] sensorIds, + String opPackageName, boolean skipIntro, long operationId, long requestId, @BiometricMultiSensorMode int multiSensorConfig, @NonNull WakefulnessLifecycle wakefulnessLifecycle, @NonNull UserManager userManager, @@ -951,8 +957,8 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba .setOperationId(operationId) .setRequestId(requestId) .setMultiSensorConfig(multiSensorConfig) - .build(sensorIds, mFpProps, mFaceProps, wakefulnessLifecycle, userManager, - lockPatternUtils); + .build(bgExecutor, sensorIds, mFpProps, mFaceProps, wakefulnessLifecycle, + userManager, lockPatternUtils); } /** diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java index ed84a37682e41..4fa835e038ecc 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java @@ -53,6 +53,8 @@ import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.VerifyCredentialResponse; import com.android.systemui.R; import com.android.systemui.animation.Interpolators; +import com.android.systemui.dagger.qualifiers.Background; +import com.android.systemui.util.concurrency.DelayableExecutor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -99,6 +101,8 @@ public abstract class AuthCredentialView extends LinearLayout { protected int mEffectiveUserId; protected ErrorTimer mErrorTimer; + protected @Background DelayableExecutor mBackgroundExecutor; + interface Callback { void onCredentialMatched(byte[] attestation); } @@ -217,6 +221,10 @@ public abstract class AuthCredentialView extends LinearLayout { mContainerView = containerView; } + void setBackgroundExecutor(@Background DelayableExecutor bgExecutor) { + mBackgroundExecutor = bgExecutor; + } + @Override protected void onAttachedToWindow() { super.onAttachedToWindow(); @@ -377,27 +385,33 @@ public abstract class AuthCredentialView extends LinearLayout { } private void showLastAttemptBeforeWipeDialog() { - final AlertDialog alertDialog = new AlertDialog.Builder(mContext) - .setTitle(R.string.biometric_dialog_last_attempt_before_wipe_dialog_title) - .setMessage( - getLastAttemptBeforeWipeMessage(getUserTypeForWipe(), mCredentialType)) - .setPositiveButton(android.R.string.ok, null) - .create(); - alertDialog.getWindow().setType(WindowManager.LayoutParams.TYPE_STATUS_BAR_SUB_PANEL); - alertDialog.show(); + mBackgroundExecutor.execute(() -> { + final AlertDialog alertDialog = new AlertDialog.Builder(mContext) + .setTitle(R.string.biometric_dialog_last_attempt_before_wipe_dialog_title) + .setMessage( + getLastAttemptBeforeWipeMessage(getUserTypeForWipe(), mCredentialType)) + .setPositiveButton(android.R.string.ok, null) + .create(); + alertDialog.getWindow().setType(WindowManager.LayoutParams.TYPE_STATUS_BAR_SUB_PANEL); + mHandler.post(alertDialog::show); + }); } private void showNowWipingDialog() { - final AlertDialog alertDialog = new AlertDialog.Builder(mContext) - .setMessage(getNowWipingMessage(getUserTypeForWipe())) - .setPositiveButton( - com.android.settingslib.R.string.failed_attempts_now_wiping_dialog_dismiss, - null /* OnClickListener */) - .setOnDismissListener( - dialog -> mContainerView.animateAway(AuthDialogCallback.DISMISSED_ERROR)) - .create(); - alertDialog.getWindow().setType(WindowManager.LayoutParams.TYPE_STATUS_BAR_SUB_PANEL); - alertDialog.show(); + mBackgroundExecutor.execute(() -> { + String nowWipingMessage = getNowWipingMessage(getUserTypeForWipe()); + final AlertDialog alertDialog = new AlertDialog.Builder(mContext) + .setMessage(nowWipingMessage) + .setPositiveButton( + com.android.settingslib.R.string.failed_attempts_now_wiping_dialog_dismiss, + null /* OnClickListener */) + .setOnDismissListener( + dialog -> mContainerView.animateAway( + AuthDialogCallback.DISMISSED_ERROR)) + .create(); + alertDialog.getWindow().setType(WindowManager.LayoutParams.TYPE_STATUS_BAR_SUB_PANEL); + mHandler.post(alertDialog::show); + }); } private @UserType int getUserTypeForWipe() { @@ -412,6 +426,7 @@ public abstract class AuthCredentialView extends LinearLayout { } } + // This should not be called on the main thread to avoid making an IPC. private String getLastAttemptBeforeWipeMessage( @UserType int userType, @Utils.CredentialType int credentialType) { switch (userType) { @@ -442,6 +457,7 @@ public abstract class AuthCredentialView extends LinearLayout { } } + // This should not be called on the main thread to avoid making an IPC. private String getLastAttemptBeforeWipeProfileMessage( @Utils.CredentialType int credentialType) { return mDevicePolicyManager.getResources().getString( diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSSecurityFooter.java b/packages/SystemUI/src/com/android/systemui/qs/QSSecurityFooter.java index 7e0410c0674ba..dd99db49c1d27 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSSecurityFooter.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSSecurityFooter.java @@ -143,9 +143,10 @@ class QSSecurityFooter extends ViewController @Inject QSSecurityFooter(@Named(QS_SECURITY_FOOTER_VIEW) View rootView, - UserTracker userTracker, @Main Handler mainHandler, ActivityStarter activityStarter, - SecurityController securityController, DialogLaunchAnimator dialogLaunchAnimator, - @Background Looper bgLooper, BroadcastDispatcher broadcastDispatcher) { + UserTracker userTracker, @Main Handler mainHandler, + ActivityStarter activityStarter, SecurityController securityController, + DialogLaunchAnimator dialogLaunchAnimator, @Background Looper bgLooper, + BroadcastDispatcher broadcastDispatcher) { super(rootView); mFooterText = mView.findViewById(R.id.footer_text); mPrimaryFooterIcon = mView.findViewById(R.id.primary_footer_icon); @@ -493,21 +494,23 @@ class QSSecurityFooter extends ViewController private void createDialog() { mShouldUseSettingsButton.set(false); - final View view = createDialogView(); - mMainHandler.post(() -> { - mDialog = new SystemUIDialog(mContext, 0); // Use mContext theme - mDialog.requestWindowFeature(Window.FEATURE_NO_TITLE); - mDialog.setButton(DialogInterface.BUTTON_POSITIVE, getPositiveButton(), this); - mDialog.setButton(DialogInterface.BUTTON_NEGATIVE, - mShouldUseSettingsButton.get() ? getSettingsButton() : getNegativeButton(), - this); + mHandler.post(() -> { + String settingsButtonText = getSettingsButton(); + final View view = createDialogView(); + mMainHandler.post(() -> { + mDialog = new SystemUIDialog(mContext, 0); // Use mContext theme + mDialog.requestWindowFeature(Window.FEATURE_NO_TITLE); + mDialog.setButton(DialogInterface.BUTTON_POSITIVE, getPositiveButton(), this); + mDialog.setButton(DialogInterface.BUTTON_NEGATIVE, mShouldUseSettingsButton.get() + ? settingsButtonText : getNegativeButton(), this); - mDialog.setView(view); - if (mView.isAggregatedVisible()) { - mDialogLaunchAnimator.showFromView(mDialog, mView); - } else { - mDialog.show(); - } + mDialog.setView(view); + if (mView.isAggregatedVisible()) { + mDialogLaunchAnimator.showFromView(mDialog, mView); + } else { + mDialog.show(); + } + }); }); } @@ -650,6 +653,7 @@ class QSSecurityFooter extends ViewController } } + // This should not be called on the main thread to avoid making an IPC. @VisibleForTesting String getSettingsButton() { return mDpm.getResources().getString( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java index d7abf7430cac1..026141898d482 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java @@ -292,9 +292,7 @@ public class PhoneStatusBarPolicy mIconController.setIconVisibility(mSlotHotspot, mHotspot.isHotspotEnabled()); // managed profile - mIconController.setIcon(mSlotManagedProfile, R.drawable.stat_sys_managed_profile_status, - getManagedProfileAccessibilityString()); - mIconController.setIconVisibility(mSlotManagedProfile, mManagedProfileIconVisible); + updateManagedProfile(); // data saver mIconController.setIcon(mSlotDataSaver, R.drawable.stat_sys_data_saver, @@ -521,7 +519,7 @@ public class PhoneStatusBarPolicy } private void updateManagedProfile() { - // getLastResumedActivityUserId needds to acquire the AM lock, which may be contended in + // getLastResumedActivityUserId needs to acquire the AM lock, which may be contended in // some cases. Since it doesn't really matter here whether it's updated in this frame // or in the next one, we call this method from our UI offload thread. mUiBgExecutor.execute(() -> { @@ -529,6 +527,7 @@ public class PhoneStatusBarPolicy try { userId = ActivityTaskManager.getService().getLastResumedActivityUserId(); boolean isManagedProfile = mUserManager.isManagedProfile(userId); + String accessibilityString = getManagedProfileAccessibilityString(); mHandler.post(() -> { final boolean showIcon; if (isManagedProfile && (!mKeyguardStateController.isShowing() @@ -536,7 +535,7 @@ public class PhoneStatusBarPolicy showIcon = true; mIconController.setIcon(mSlotManagedProfile, R.drawable.stat_sys_managed_profile_status, - getManagedProfileAccessibilityString()); + accessibilityString); } else { showIcon = false; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthContainerViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthContainerViewTest.kt index 666c9e481adc1..2341928b2565d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthContainerViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthContainerViewTest.kt @@ -40,6 +40,9 @@ import com.android.internal.widget.LockPatternUtils import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.keyguard.WakefulnessLifecycle +import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import org.junit.After import org.junit.Rule @@ -314,7 +317,8 @@ class AuthContainerViewTest : SysuiTestCase() { wakefulnessLifecycle, userManager, lockPatternUtils, - Handler(TestableLooper.get(this).looper) + Handler(TestableLooper.get(this).looper), + FakeExecutor(FakeSystemClock()) ) if (addToView) { @@ -331,10 +335,11 @@ class AuthContainerViewTest : SysuiTestCase() { wakefulnessLifecycle: WakefulnessLifecycle, userManager: UserManager, lockPatternUtils: LockPatternUtils, - mainHandler: Handler + mainHandler: Handler, + bgExecutor: DelayableExecutor ) : AuthContainerView( config, fpProps, faceProps, - wakefulnessLifecycle, userManager, lockPatternUtils, mainHandler + wakefulnessLifecycle, userManager, lockPatternUtils, mainHandler, bgExecutor ) { override fun postOnAnimation(runnable: Runnable) { runnable.run() diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthControllerTest.java index 190228d80cde8..4858ab5234f89 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthControllerTest.java @@ -80,8 +80,11 @@ import com.android.systemui.SysuiTestCase; import com.android.systemui.keyguard.WakefulnessLifecycle; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.CommandQueue; +import com.android.systemui.util.concurrency.DelayableExecutor; import com.android.systemui.util.concurrency.Execution; import com.android.systemui.util.concurrency.FakeExecution; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Rule; @@ -156,6 +159,7 @@ public class AuthControllerTest extends SysuiTestCase { private Execution mExecution; private TestableLooper mTestableLooper; private Handler mHandler; + private DelayableExecutor mBackgroundExecutor; private TestableAuthController mAuthController; @Before @@ -164,6 +168,7 @@ public class AuthControllerTest extends SysuiTestCase { mExecution = new FakeExecution(); mTestableLooper = TestableLooper.get(this); mHandler = new Handler(mTestableLooper.getLooper()); + mBackgroundExecutor = new FakeExecutor(new FakeSystemClock()); when(mContextSpy.getPackageManager()).thenReturn(mPackageManager); when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_FACE)) @@ -759,11 +764,12 @@ public class AuthControllerTest extends SysuiTestCase { super(context, execution, commandQueue, activityTaskManager, windowManager, fingerprintManager, faceManager, udfpsControllerFactory, sidefpsControllerFactory, mDisplayManager, mWakefulnessLifecycle, - mUserManager, mLockPatternUtils, statusBarStateController, mHandler); + mUserManager, mLockPatternUtils, statusBarStateController, mHandler, + mBackgroundExecutor); } @Override - protected AuthDialog buildDialog(PromptInfo promptInfo, + protected AuthDialog buildDialog(DelayableExecutor bgExecutor, PromptInfo promptInfo, boolean requireConfirmation, int userId, int[] sensorIds, String opPackageName, boolean skipIntro, long operationId, long requestId, @BiometricManager.BiometricMultiSensorMode int multiSensorConfig,