Revert "Move showing keyguard after the UserSwitchObservers."
This reverts commit ad64f73046717374d4f9bc613307d01e93e19ebe.
Reason for revert: This CL caused b/360838273, we need revert it and show the keyguard at the beginning of the user switch as we were doing before. Otherwise, it can cause security issues such as the ones described in b/360838273#comment22.
This needs to be submitted together with the proper fix in `WindowManagerService.lockDeviceNow()`.
Bug: 331853529
Fixes: 360838273
Change-Id: I8f25255ac204160092fb88c9f099c697b222dc88
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index b186eaa..fb55700 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -156,6 +156,9 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
@@ -223,18 +226,9 @@
private static final int USER_SWITCH_CALLBACKS_TIMEOUT_MS = 5 * 1000;
/**
- * Amount of time waited for
- * {@link ActivityTaskManagerInternal.ScreenObserver#onKeyguardStateChanged} callbacks to be
- * called after calling {@link WindowManagerService#lockDeviceNow}.
- * Otherwise, we should throw a {@link RuntimeException} and never dismiss the
- * {@link UserSwitchingDialog}.
- */
- static final int SHOW_KEYGUARD_TIMEOUT_MS = 20 * 1000;
-
- /**
* Amount of time waited for {@link WindowManagerService#dismissKeyguard} callbacks to be
* called after dismissing the keyguard.
- * Otherwise, we should move on to dismiss the dialog {@link #dismissUserSwitchDialog}}
+ * Otherwise, we should move on to dismiss the dialog {@link #dismissUserSwitchDialog()}
* and report user switch is complete {@link #REPORT_USER_SWITCH_COMPLETE_MSG}.
*/
private static final int DISMISS_KEYGUARD_TIMEOUT_MS = 2 * 1000;
@@ -1988,8 +1982,15 @@
updateProfileRelatedCaches();
mInjector.getWindowManager().setCurrentUser(userId);
mInjector.reportCurWakefulnessUsageEvent();
+ // Once the internal notion of the active user has switched, we lock the device
+ // with the option to show the user switcher on the keyguard.
if (userSwitchUiEnabled) {
mInjector.getWindowManager().setSwitchingUser(true);
+ // Only lock if the user has a secure keyguard PIN/Pattern/Pwd
+ if (mInjector.getKeyguardManager().isDeviceSecure(userId)) {
+ // Make sure the device is locked before moving on with the user switch
+ mInjector.lockDeviceNowAndWaitForKeyguardShown();
+ }
}
} else {
@@ -2606,56 +2607,34 @@
@VisibleForTesting
void completeUserSwitch(int oldUserId, int newUserId) {
- final Runnable sendUserSwitchCompleteMessage = () -> {
- mHandler.removeMessages(REPORT_USER_SWITCH_COMPLETE_MSG);
- mHandler.sendMessage(mHandler.obtainMessage(
- REPORT_USER_SWITCH_COMPLETE_MSG, oldUserId, newUserId));
- };
- if (isUserSwitchUiEnabled()) {
- if (mInjector.getKeyguardManager().isDeviceSecure(newUserId)) {
- this.showKeyguard(() -> dismissUserSwitchDialog(sendUserSwitchCompleteMessage));
- } else {
- this.dismissKeyguard(() -> dismissUserSwitchDialog(sendUserSwitchCompleteMessage));
- }
+ final boolean isUserSwitchUiEnabled = isUserSwitchUiEnabled();
+ // serialize each conditional step
+ await(
+ // STEP 1 - If there is no challenge set, dismiss the keyguard right away
+ isUserSwitchUiEnabled && !mInjector.getKeyguardManager().isDeviceSecure(newUserId),
+ mInjector::dismissKeyguard,
+ () -> await(
+ // STEP 2 - If user switch ui was enabled, dismiss user switch dialog
+ isUserSwitchUiEnabled,
+ this::dismissUserSwitchDialog,
+ () -> {
+ // STEP 3 - Send REPORT_USER_SWITCH_COMPLETE_MSG to broadcast
+ // ACTION_USER_SWITCHED & call UserSwitchObservers.onUserSwitchComplete
+ mHandler.removeMessages(REPORT_USER_SWITCH_COMPLETE_MSG);
+ mHandler.sendMessage(mHandler.obtainMessage(
+ REPORT_USER_SWITCH_COMPLETE_MSG, oldUserId, newUserId));
+ }
+ ));
+ }
+
+ private void await(boolean condition, Consumer<Runnable> conditionalStep, Runnable nextStep) {
+ if (condition) {
+ conditionalStep.accept(nextStep);
} else {
- sendUserSwitchCompleteMessage.run();
+ nextStep.run();
}
}
- protected void showKeyguard(Runnable runnable) {
- runWithTimeout(mInjector::showKeyguard, SHOW_KEYGUARD_TIMEOUT_MS, runnable, () -> {
- throw new RuntimeException(
- "Keyguard is not shown in " + SHOW_KEYGUARD_TIMEOUT_MS + " ms.");
- }, "showKeyguard");
- }
-
- protected void dismissKeyguard(Runnable runnable) {
- runWithTimeout(mInjector::dismissKeyguard, DISMISS_KEYGUARD_TIMEOUT_MS, runnable, runnable,
- "dismissKeyguard");
- }
-
- private void runWithTimeout(Consumer<Runnable> task, int timeoutMs, Runnable onSuccess,
- Runnable onTimeout, String traceMsg) {
- final AtomicInteger state = new AtomicInteger(0); // state = 0 (RUNNING)
-
- asyncTraceBegin(traceMsg, 0);
-
- mHandler.postDelayed(() -> {
- if (state.compareAndSet(0, 1)) { // state = 1 (TIMEOUT)
- asyncTraceEnd(traceMsg, 0);
- Slogf.w(TAG, "Timeout: %s did not finish in %d ms", traceMsg, timeoutMs);
- onTimeout.run();
- }
- }, timeoutMs);
-
- task.accept(() -> {
- if (state.compareAndSet(0, 2)) { // state = 2 (SUCCESS)
- asyncTraceEnd(traceMsg, 0);
- onSuccess.run();
- }
- });
- }
-
private void moveUserToForeground(UserState uss, int newUserId) {
boolean homeInFront = mInjector.taskSupervisorSwitchUser(newUserId, uss);
if (homeInFront) {
@@ -4100,45 +4079,29 @@
return IStorageManager.Stub.asInterface(ServiceManager.getService("mount"));
}
- protected void showKeyguard(Runnable runnable) {
- if (getWindowManager().isKeyguardLocked()) {
- runnable.run();
- return;
- }
- getActivityTaskManagerInternal().registerScreenObserver(
- new ActivityTaskManagerInternal.ScreenObserver() {
- @Override
- public void onAwakeStateChanged(boolean isAwake) {
-
- }
-
- @Override
- public void onKeyguardStateChanged(boolean isShowing) {
- if (isShowing) {
- getActivityTaskManagerInternal().unregisterScreenObserver(this);
- runnable.run();
- }
- }
- }
- );
- getWindowManager().lockDeviceNow();
- }
-
protected void dismissKeyguard(Runnable runnable) {
+ final AtomicBoolean isFirst = new AtomicBoolean(true);
+ final Runnable runOnce = () -> {
+ if (isFirst.getAndSet(false)) {
+ runnable.run();
+ }
+ };
+
+ mHandler.postDelayed(runOnce, DISMISS_KEYGUARD_TIMEOUT_MS);
getWindowManager().dismissKeyguard(new IKeyguardDismissCallback.Stub() {
@Override
public void onDismissError() throws RemoteException {
- runnable.run();
+ mHandler.post(runOnce);
}
@Override
public void onDismissSucceeded() throws RemoteException {
- runnable.run();
+ mHandler.post(runOnce);
}
@Override
public void onDismissCancelled() throws RemoteException {
- runnable.run();
+ mHandler.post(runOnce);
}
}, /* message= */ null);
}
@@ -4164,5 +4127,43 @@
void onSystemUserVisibilityChanged(boolean visible) {
getUserManagerInternal().onSystemUserVisibilityChanged(visible);
}
+
+ void lockDeviceNowAndWaitForKeyguardShown() {
+ if (getWindowManager().isKeyguardLocked()) {
+ return;
+ }
+
+ final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
+ t.traceBegin("lockDeviceNowAndWaitForKeyguardShown");
+
+ final CountDownLatch latch = new CountDownLatch(1);
+ ActivityTaskManagerInternal.ScreenObserver screenObserver =
+ new ActivityTaskManagerInternal.ScreenObserver() {
+ @Override
+ public void onAwakeStateChanged(boolean isAwake) {
+
+ }
+
+ @Override
+ public void onKeyguardStateChanged(boolean isShowing) {
+ if (isShowing) {
+ latch.countDown();
+ }
+ }
+ };
+
+ getActivityTaskManagerInternal().registerScreenObserver(screenObserver);
+ getWindowManager().lockDeviceNow();
+ try {
+ if (!latch.await(20, TimeUnit.SECONDS)) {
+ throw new RuntimeException("Keyguard is not shown in 20 seconds");
+ }
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ } finally {
+ getActivityTaskManagerInternal().unregisterScreenObserver(screenObserver);
+ t.traceEnd();
+ }
+ }
}
}
diff --git a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
index a25621a..d97a0fa 100644
--- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
@@ -61,7 +61,6 @@
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doNothing;
@@ -95,7 +94,6 @@
import android.os.Message;
import android.os.PowerManagerInternal;
import android.os.RemoteException;
-import android.os.SystemClock;
import android.os.UserHandle;
import android.os.UserManager;
import android.os.storage.IStorageManager;
@@ -214,10 +212,7 @@
doNothing().when(mInjector).activityManagerOnUserStopped(anyInt());
doNothing().when(mInjector).clearBroadcastQueueForUser(anyInt());
doNothing().when(mInjector).taskSupervisorRemoveUser(anyInt());
- doAnswer(invocation -> {
- ((Runnable) invocation.getArgument(0)).run();
- return null;
- }).when(mInjector).showKeyguard(any());
+ doNothing().when(mInjector).lockDeviceNowAndWaitForKeyguardShown();
mockIsUsersOnSecondaryDisplaysEnabled(false);
// All UserController params are set to default.
@@ -554,6 +549,7 @@
expectedCodes.add(REPORT_USER_SWITCH_COMPLETE_MSG);
if (backgroundUserStopping) {
expectedCodes.add(CLEAR_USER_JOURNEY_SESSION_MSG);
+ expectedCodes.add(0); // this is for directly posting in stopping.
}
if (expectScheduleBackgroundUserStopping) {
expectedCodes.add(SCHEDULED_STOP_BACKGROUND_USER_MSG);
@@ -1579,13 +1575,21 @@
// mock the device to be secure in order to expect the keyguard to be shown
when(mInjector.mKeyguardManagerMock.isDeviceSecure(anyInt())).thenReturn(true);
- // call real showKeyguard method for this test
- doCallRealMethod().when(mInjector).showKeyguard(any());
+ // call real lockDeviceNowAndWaitForKeyguardShown method for this test
+ doCallRealMethod().when(mInjector).lockDeviceNowAndWaitForKeyguardShown();
- mUserController.completeUserSwitch(TEST_USER_ID1, TEST_USER_ID2);
+ // call startUser on a thread because we're expecting it to be blocked
+ Thread threadStartUser = new Thread(()-> {
+ mUserController.startUser(TEST_USER_ID, USER_START_MODE_FOREGROUND);
+ });
+ threadStartUser.start();
- // make sure the switch is stalled by checking the UserSwitchingDialog is not dismissed yet
- verify(mInjector, never()).dismissUserSwitchingDialog(any());
+ // make sure the switch is stalled...
+ Thread.sleep(2000);
+ // by checking REPORT_USER_SWITCH_MSG is not sent yet
+ assertNull(mInjector.mHandler.getMessageForCode(REPORT_USER_SWITCH_MSG));
+ // and the thread is still alive
+ assertTrue(threadStartUser.isAlive());
// mock send the keyguard shown event
ArgumentCaptor<ActivityTaskManagerInternal.ScreenObserver> captor = ArgumentCaptor.forClass(
@@ -1593,42 +1597,12 @@
verify(mInjector.mActivityTaskManagerInternal).registerScreenObserver(captor.capture());
captor.getValue().onKeyguardStateChanged(true);
- // verify the switch now moves on by checking the UserSwitchingDialog is dismissed
- verify(mInjector, atLeastOnce()).dismissUserSwitchingDialog(any());
-
- // verify that SHOW_KEYGUARD_TIMEOUT is ignored and does not crash the system
- try {
- mInjector.mHandler.processPostDelayedCallbacksWithin(
- UserController.SHOW_KEYGUARD_TIMEOUT_MS);
- } catch (RuntimeException e) {
- throw new AssertionError(
- "SHOW_KEYGUARD_TIMEOUT is not ignored and crashed the system", e);
- }
- }
-
- @Test
- public void testRuntimeExceptionIsThrownIfTheKeyguardIsNotShown() throws Exception {
- // enable user switch ui, because keyguard is only shown then
- mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
- /* maxRunningUsers= */ 3, /* delayUserDataLocking= */ false,
- /* backgroundUserScheduledStopTimeSecs= */ -1);
-
- // mock the device to be secure in order to expect the keyguard to be shown
- when(mInjector.mKeyguardManagerMock.isDeviceSecure(anyInt())).thenReturn(true);
-
- // suppress showKeyguard method for this test
- doNothing().when(mInjector).showKeyguard(any());
-
- mUserController.completeUserSwitch(TEST_USER_ID1, TEST_USER_ID2);
-
- // verify that the system has crashed
- assertThrows("Should have thrown RuntimeException", RuntimeException.class, () -> {
- mInjector.mHandler.processPostDelayedCallbacksWithin(
- UserController.SHOW_KEYGUARD_TIMEOUT_MS);
- });
-
- // make sure the UserSwitchingDialog is not dismissed
- verify(mInjector, never()).dismissUserSwitchingDialog(any());
+ // verify the switch now moves on...
+ Thread.sleep(1000);
+ // by checking REPORT_USER_SWITCH_MSG is sent
+ assertNotNull(mInjector.mHandler.getMessageForCode(REPORT_USER_SWITCH_MSG));
+ // and the thread is finished
+ assertFalse(threadStartUser.isAlive());
}
private void setUpAndStartUserInBackground(int userId) throws Exception {
@@ -1989,9 +1963,7 @@
Set<Integer> getMessageCodes() {
Set<Integer> result = new LinkedHashSet<>();
for (Message msg : mMessages) {
- if (msg.what != 0) { // ignore mHandle.post and mHandler.postDelayed messages
- result.add(msg.what);
- }
+ result.add(msg.what);
}
return result;
}
@@ -2015,28 +1987,14 @@
@Override
public boolean sendMessageAtTime(Message msg, long uptimeMillis) {
- final Runnable cb = msg.getCallback();
- if (cb != null && uptimeMillis <= SystemClock.uptimeMillis()) {
- // run mHandler.post calls immediately
- cb.run();
- return true;
- }
Message copy = new Message();
copy.copyFrom(msg);
- copy.setCallback(cb);
mMessages.add(copy);
- return super.sendMessageAtTime(msg, uptimeMillis);
- }
-
- public void processPostDelayedCallbacksWithin(long millis) {
- final long whenMax = SystemClock.uptimeMillis() + millis;
- for (Message msg : mMessages) {
- final Runnable cb = msg.getCallback();
- if (cb != null && msg.getWhen() <= whenMax) {
- msg.setCallback(null);
- cb.run();
- }
+ if (msg.getCallback() != null) {
+ msg.getCallback().run();
+ msg.setCallback(null);
}
+ return super.sendMessageAtTime(msg, uptimeMillis);
}
}
}
