LogoutUser relies on SYSTEM.supportsSwitchTo
logoutUser always switches to the system user. All that therefore
matters is that the device can, in fact, switch to the system user. This
is irrespective of whether it is HSUM or not, so we don't need any
special-casing; we can just request whether we can switch to user 0.
This is a general policy: we avoid checking modes and instead check
properties - in this case, whether SYSTEM_USER has the property allowing
us to switch to it.
Also fixes some UserControllerTest logoutUser tests that weren't working
properly since they weren't ever actually switching users.
Test: UserControllerTest
Bug: 411696141
Bug: 428046912
Flag: EXEMPT trivial equivalent code
Change-Id: I14bd7e92679c51f799737ca22faecf77fc9039b7
diff --git a/core/java/android/content/pm/UserInfo.java b/core/java/android/content/pm/UserInfo.java
index 5c3ecc0..5daea9b 100644
--- a/core/java/android/content/pm/UserInfo.java
+++ b/core/java/android/content/pm/UserInfo.java
@@ -468,7 +468,7 @@
// Don't support switching to pre-created users until they become "real" users.
return false;
}
- return isFull() || canSwitchToHeadlessSystemUser();
+ return isFull() || isSwitchableHeadlessSystemUser();
}
/**
@@ -476,7 +476,7 @@
* {@link com.android.internal.R.bool#config_canSwitchToHeadlessSystemUser} is true.
*/
@android.ravenwood.annotation.RavenwoodThrow
- private boolean canSwitchToHeadlessSystemUser() {
+ private boolean isSwitchableHeadlessSystemUser() {
return UserManager.USER_TYPE_SYSTEM_HEADLESS.equals(userType) && Resources.getSystem()
.getBoolean(com.android.internal.R.bool.config_canSwitchToHeadlessSystemUser);
}
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index 0e69ec7..ee33195 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -1134,10 +1134,7 @@
// system user (not the previous foreground user). Thus we cannot support HSUM devices
// without interactive headless system user. To solve it for the long term, a refactor might
// be needed, see more details in the bug.
- // TODO(b/411696141): Use the more proper API to check if headless system user is
- // interactive, once it's ready.
- if (mInjector.isHeadlessSystemUserMode()
- && !mInjector.getUserManager().canSwitchToHeadlessSystemUser()) {
+ if (!mInjector.doesUserSupportSwitchTo(getUserInfo(UserHandle.USER_SYSTEM))) {
throw new UnsupportedOperationException("device does not support logoutUser");
}
boolean shouldSwitchUser = false;
@@ -2508,7 +2505,7 @@
Slogf.w(TAG, "No user info for user #" + targetUserId);
return false;
}
- if (!targetUserInfo.supportsSwitchTo()) {
+ if (!mInjector.doesUserSupportSwitchTo(targetUserInfo)) {
Slogf.w(TAG, "Cannot switch to User #" + targetUserId + ": not supported");
return false;
}
@@ -4493,6 +4490,10 @@
return UserManager.isHeadlessSystemUserMode();
}
+ boolean doesUserSupportSwitchTo(UserInfo user) {
+ return user.supportsSwitchTo();
+ }
+
boolean isUsersOnSecondaryDisplaysEnabled() {
return UserManager.isVisibleBackgroundUsersEnabled();
}
diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index 2dd8329..3de1ef3 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -8957,7 +8957,6 @@
writeUserLP(userData);
return true;
}
-
}
/**
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 107ad40..03533fa 100644
--- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
@@ -294,8 +294,6 @@
@Test
public void testStartUser_sendsNoBroadcastsForSystemUserInNonHeadlessMode() {
- setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,
- UserManager.USER_TYPE_FULL_SYSTEM);
mockIsHeadlessSystemUserMode(false);
mUserController.startUser(SYSTEM_USER_ID, USER_START_MODE_FOREGROUND);
@@ -306,8 +304,6 @@
@Test
public void testStartUser_sendsBroadcastsForSystemUserInHeadlessMode() {
- setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,
- UserManager.USER_TYPE_SYSTEM_HEADLESS);
mockIsHeadlessSystemUserMode(true);
mUserController.startUser(SYSTEM_USER_ID, USER_START_MODE_FOREGROUND);
@@ -536,14 +532,11 @@
continueUserSwitchAssertions(oldUserId, TEST_USER_ID, false, false);
}
- private void mockCanSwitchToHeadlessSystemUser(boolean canSwitch) {
- doReturn(canSwitch).when(mInjector.mUserManagerMock)
- .canSwitchToHeadlessSystemUser();
- }
-
@Test
public void testLogoutUserDuringSwitchToSameUser_nonHsum()
throws InterruptedException {
+ // TODO(b/428046912): This test doesn't actually test anything. The switch isn't sufficient,
+ // so the list of running users will never contain the test user anyway.
mockIsHeadlessSystemUserMode(false);
// Start user -- this will update state of mUserController
@@ -562,8 +555,9 @@
@Test
public void testLogoutUserDuringSwitchToSameUser_hsumAndInteractiveSystemUser()
throws InterruptedException {
- mockIsHeadlessSystemUserMode(true);
- mockCanSwitchToHeadlessSystemUser(true);
+ // TODO(b/428046912): This test doesn't actually test anything. The switch isn't sufficient,
+ // so the list of running users will never contain the test user anyway.
+ mockIsSwitchableHeadlessSystemUserMode();
// Start user -- this will update state of mUserController
mUserController.startUser(TEST_USER_ID1, USER_START_MODE_FOREGROUND);
@@ -581,38 +575,47 @@
@Test
public void testLogoutUser_nonHsum() throws InterruptedException {
mockIsHeadlessSystemUserMode(false);
+ mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
+ /* maxRunningUsers= */ 3, /* delayUserDataLocking= */ false,
+ /* backgroundUserScheduledStopTimeSecs= */ -1);
- // Start user -- this will update state of mUserController
- mUserController.switchUser(UserHandle.USER_SYSTEM);
- mUserController.switchUser(TEST_USER_ID);
- waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
+ // Switch to the test user.
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID, 1,
+ /* expectOldUserStopping= */false,
+ /* expectScheduleBackgroundUserStopping= */ false);
+ assertTrue(mUserController.getRunningUsersLU().contains(TEST_USER_ID));
- // Logout user.
+ // Logout the test user.
mUserController.logoutUser(TEST_USER_ID);
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
- // Verify that TEST_USER_ID is not running.
- List<Integer> runningUserIds = mUserController.getRunningUsersLU();
- assertFalse(runningUserIds.contains(TEST_USER_ID));
+ // Verify that TEST_USER_ID is no longer running.
+ assertFalse(mUserController.getRunningUsersLU().contains(TEST_USER_ID));
}
@Test
public void testLogoutUser_hsumAndInteractiveSystemUser() throws InterruptedException {
- mockIsHeadlessSystemUserMode(true);
- mockCanSwitchToHeadlessSystemUser(true);
+ mockIsSwitchableHeadlessSystemUserMode();
+ mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
+ /* maxRunningUsers= */ 3, /* delayUserDataLocking= */ false,
+ /* backgroundUserScheduledStopTimeSecs= */ -1);
- // Start user -- this will update state of mUserController
- mUserController.switchUser(UserHandle.USER_SYSTEM);
- mUserController.switchUser(TEST_USER_ID);
- waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
+ // Switch to the test user.
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID, 1,
+ /* expectOldUserStopping= */false,
+ /* expectScheduleBackgroundUserStopping= */ false);
+ assertRunningUsersIgnoreOrder(SYSTEM_USER_ID, TEST_USER_ID);
+ assertEquals("Unexpected current user", TEST_USER_ID, mUserController.getCurrentUserId());
- // Logout user.
+ // Logout the test user.
mUserController.logoutUser(TEST_USER_ID);
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
- // Verify that TEST_USER_ID is not running.
- List<Integer> runningUserIds = mUserController.getRunningUsersLU();
- assertFalse(runningUserIds.contains(TEST_USER_ID));
+ // Verify that TEST_USER_ID is no longer running.
+ assertRunningUsersIgnoreOrder(SYSTEM_USER_ID);
+ // Current policy is that interactive HSUM should specifically log out to the system user.
+ assertEquals("Logout should always be to the system user in iHSUM",
+ SYSTEM_USER_ID, mUserController.getCurrentOrTargetUserId());
}
private void continueUserSwitchAssertions(int expectedOldUserId, int expectedNewUserId,
@@ -677,7 +680,7 @@
// Switch to TEST_USER_ID from user 0
int numberOfUserSwitches = 0;
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,
++numberOfUserSwitches,
/* expectOldUserStopping= */false,
/* expectScheduleBackgroundUserStopping= */ false);
@@ -922,7 +925,7 @@
// Switch to TEST_USER_ID from user 0
int numberOfUserSwitches = 0;
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,
++numberOfUserSwitches, false,
/* expectScheduleBackgroundUserStopping= */ false);
assertRunningUsersInOrder(SYSTEM_USER_ID, TEST_USER_ID);
@@ -1057,7 +1060,7 @@
setUpUser(TEST_USER_ID1, 0);
setUpUser(TEST_USER_ID2, 0);
int numberOfUserSwitches = 1;
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,
numberOfUserSwitches, false, false);
// running: user 0, USER_ID
assertTrue(mUserController.canStartMoreUsers());
@@ -1099,7 +1102,7 @@
setUpUser(TEST_USER_ID1, 0);
setUpUser(TEST_USER_ID2, 0);
int numberOfUserSwitches = 1;
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,
numberOfUserSwitches, false, false);
// running: user 0, USER_ID
assertTrue(mUserController.canStartMoreUsers());
@@ -1197,7 +1200,7 @@
assertRunningUsersInOrder(TEST_USER_ID1, SYSTEM_USER_ID);
// Start a user in the foreground. This exceeds max. Make sure we cut down to 2 users.
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, SYSTEM_USER_ID,
1, false, false);
mUserController.finishUserSwitch(mUserStates.get(TEST_USER_ID2));
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1245,7 +1248,7 @@
assertRunningUsersInOrder(SYSTEM_USER_ID);
- addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, SYSTEM_USER_ID,
1, false, false);
mUserController.finishUserSwitch(mUserStates.get(TEST_USER_ID1));
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1301,7 +1304,7 @@
assertRunningUsersIgnoreOrder(BG_USER_ID, SYSTEM_USER_ID);
// Start PARENT_ID
- addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, 1, false, false);
+ addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID, 1, false, false);
// We call startProfiles(), but the profiles were configured to not auto-start.
mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); // Calls startProfiles()
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1355,7 +1358,7 @@
assertRunningUsersIgnoreOrder(BG_USER_ID, SYSTEM_USER_ID);
// Start PARENT_ID
- addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, 1, false, false);
+ addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID, 1, false, false);
// We call startProfiles(), which auto-starts the profiles.
mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); // Calls startProfiles()
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1393,7 +1396,7 @@
assertRunningUsersInOrder(SYSTEM_USER_ID);
int numberOfUserSwitches = 1;
- addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID,
numberOfUserSwitches, false, false);
mUserController.finishUserSwitch(mUserStates.get(PARENT_ID));
waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1515,7 +1518,7 @@
assertRunningUsersInOrder(SYSTEM_USER_ID);
int numberOfUserSwitches = 1;
- addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,
+ addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID,
numberOfUserSwitches, false, false);
assertRunningUsersInOrder(SYSTEM_USER_ID, PARENT_ID);
@@ -1561,7 +1564,7 @@
assertRunningUsersInOrder(SYSTEM_USER_ID);
- addForegroundUserAndContinueUserSwitch(CURRENT_ID, UserHandle.USER_SYSTEM, 1, false, false);
+ addForegroundUserAndContinueUserSwitch(CURRENT_ID, SYSTEM_USER_ID, 1, false, false);
assertRunningUsersInOrder(SYSTEM_USER_ID, CURRENT_ID);
mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND);
@@ -2178,8 +2181,24 @@
}
}
- private void mockIsHeadlessSystemUserMode(boolean value) {
- when(mInjector.isHeadlessSystemUserMode()).thenReturn(value);
+ /** Specify whether to mock the device as being in regular (non-switchable) HSUM. */
+ private void mockIsHeadlessSystemUserMode(boolean isHsum) {
+ mockIsHeadlessSystemUserMode(isHsum, false);
+ }
+
+ /** Mocks the device as being in interactive (switchable) HSUM. */
+ private void mockIsSwitchableHeadlessSystemUserMode() {
+ mockIsHeadlessSystemUserMode(true, true);
+ }
+
+ private void mockIsHeadlessSystemUserMode(boolean isHsum, boolean canSwitch) {
+ when(mInjector.isHeadlessSystemUserMode()).thenReturn(isHsum);
+ UserInfo sysInfo = setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,
+ isHsum ? UserManager.USER_TYPE_SYSTEM_HEADLESS : UserManager.USER_TYPE_FULL_SYSTEM);
+ if (isHsum) {
+ doReturn(canSwitch).when(mInjector.mUserManagerMock).canSwitchToHeadlessSystemUser();
+ doReturn(canSwitch).when(mInjector).doesUserSupportSwitchTo(eq(sysInfo));
+ }
}
private void mockIsUsersOnSecondaryDisplaysEnabled(boolean value) {
