diff options
| -rw-r--r-- | services/core/java/com/android/server/am/UserController.java | 99 | ||||
| -rw-r--r-- | services/tests/servicestests/src/com/android/server/am/UserControllerTest.java | 184 |
2 files changed, 240 insertions, 43 deletions
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index 13a1807b3563..41d52046b6e8 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -276,7 +276,15 @@ class UserController implements Handler.Callback { private final SparseArray<UserState> mStartedUsers = new SparseArray<>(); /** - * LRU list of history of current users. Most recently current is at the end. + * LRU list of history of running users, in order of when we last needed to start them. + * + * Switching to a user will move it towards the end. Attempting to start a user/profile (even + * if it was already running) will move it towards the end. + * + * <p>Guarantees (by the end of startUser): + * <li>The current user will always be at the end, even if background users were started + * subsequently. + * <li>Parents always come later than (but not necessarily adjacent to) their profiles. */ @GuardedBy("mLock") private final ArrayList<Integer> mUserLru = new ArrayList<>(); @@ -299,6 +307,9 @@ class UserController implements Handler.Callback { /** * Mapping from each known user ID to the profile group ID it is associated with. * <p>Users not present in this array have a profile group of NO_PROFILE_GROUP_ID. + * + * <p>For better or worse, this class sometimes assumes that the profileGroupId of a parent user + * is always identical with its userId. If that ever becomes false, this class needs updating. */ @GuardedBy("mLock") private final SparseIntArray mUserProfileGroupIds = new SparseIntArray(); @@ -499,6 +510,23 @@ class UserController implements Handler.Callback { }); } + /** Adds a user to mUserLru, moving it to the end of the list if it was already present. */ + private void addUserToUserLru(@UserIdInt int userId) { + synchronized (mLock) { + final Integer userIdObj = userId; + mUserLru.remove(userIdObj); + mUserLru.add(userIdObj); + + // Now also move the user's parent to the end (if applicable). + Integer parentIdObj = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID); + if (parentIdObj != UserInfo.NO_PROFILE_GROUP_ID && !parentIdObj.equals(userIdObj) + && mUserLru.remove(parentIdObj)) { + mUserLru.add(parentIdObj); + } + } + } + + /** Returns a list of running users, in order of when they were started (oldest first). */ @GuardedBy("mLock") @VisibleForTesting List<Integer> getRunningUsersLU() { @@ -536,9 +564,9 @@ class UserController implements Handler.Callback { @GuardedBy("mLock") private void stopExcessRunningUsersLU(int maxRunningUsers, ArraySet<Integer> exemptedUsers) { - List<Integer> currentlyRunning = getRunningUsersLU(); - Iterator<Integer> iterator = currentlyRunning.iterator(); - while (currentlyRunning.size() > maxRunningUsers && iterator.hasNext()) { + List<Integer> currentlyRunningLru = getRunningUsersLU(); + Iterator<Integer> iterator = currentlyRunningLru.iterator(); + while (currentlyRunningLru.size() > maxRunningUsers && iterator.hasNext()) { Integer userId = iterator.next(); if (userId == UserHandle.USER_SYSTEM || userId == mCurrentUserId @@ -551,6 +579,10 @@ class UserController implements Handler.Callback { if (stopUsersLU(userId, /* force= */ false, /* allowDelayedLocking= */ true, /* stopUserCallback= */ null, /* keyEvictedCallback= */ null) == USER_OP_SUCCESS) { + // Technically, stopUsersLU can remove more than one user when stopping a parent. + // But mUserLru is designed so that profiles always precede their parent, so this + // normally won't happen here, and therefore won't cause underestimating the number + // removed. iterator.remove(); } } @@ -947,7 +979,7 @@ class UserController implements Handler.Callback { } /** - * Stops the user along with its related users. The method calls + * Stops the user along with its profiles. The method calls * {@link #getUsersToStopLU(int)} to determine the list of users that should be stopped. */ @GuardedBy("mLock") @@ -992,7 +1024,12 @@ class UserController implements Handler.Callback { } /** - * Stops a single User. This can also trigger locking user data out depending on device's + * Stops a single User. + * + * This should only ever be called by {@link #stopUsersLU}, + * which is responsible to making sure any associated users are appropriately stopped too. + * + * This can also trigger locking user data out depending on device's * config ({@code mDelayUserDataLocking}) and arguments. * * In the default configuration for most device and users, users will be locked when stopping. @@ -1425,7 +1462,8 @@ class UserController implements Handler.Callback { /** * Determines the list of users that should be stopped together with the specified - * {@code userId}. The returned list includes {@code userId}. + * {@code userId}, i.e. the user and its profiles (if the given user is a parent). + * The returned list includes {@code userId}. */ @GuardedBy("mLock") private @NonNull int[] getUsersToStopLU(@UserIdInt int userId) { @@ -1433,20 +1471,23 @@ class UserController implements Handler.Callback { IntArray userIds = new IntArray(); userIds.add(userId); int userGroupId = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID); - for (int i = 0; i < startedUsersSize; i++) { - UserState uss = mStartedUsers.valueAt(i); - int startedUserId = uss.mHandle.getIdentifier(); - // Skip unrelated users (profileGroupId mismatch) - int startedUserGroupId = mUserProfileGroupIds.get(startedUserId, - UserInfo.NO_PROFILE_GROUP_ID); - boolean sameGroup = (userGroupId != UserInfo.NO_PROFILE_GROUP_ID) - && (userGroupId == startedUserGroupId); - // userId has already been added - boolean sameUserId = startedUserId == userId; - if (!sameGroup || sameUserId) { - continue; + if (userGroupId == userId) { + // The user is the parent of the profile group. Stop its profiles too. + for (int i = 0; i < startedUsersSize; i++) { + UserState uss = mStartedUsers.valueAt(i); + int startedUserId = uss.mHandle.getIdentifier(); + // Skip unrelated users (profileGroupId mismatch) + int startedUserGroupId = mUserProfileGroupIds.get(startedUserId, + UserInfo.NO_PROFILE_GROUP_ID); + boolean sameGroup = (userGroupId != UserInfo.NO_PROFILE_GROUP_ID) + && (userGroupId == startedUserGroupId); + // userId has already been added + boolean sameUserId = startedUserId == userId; + if (!sameGroup || sameUserId) { + continue; + } + userIds.add(startedUserId); } - userIds.add(startedUserId); } return userIds.toArray(); } @@ -1519,6 +1560,7 @@ class UserController implements Handler.Callback { }); } + /** Starts all applicable profiles of the current user. */ private void startProfiles() { int currentUserId = getCurrentUserId(); if (DEBUG_MU) Slogf.i(TAG, "startProfilesLocked"); @@ -1711,6 +1753,7 @@ class UserController implements Handler.Callback { t.traceBegin("getStartedUserState"); final int oldUserId = getCurrentUserId(); if (oldUserId == userId) { + // The user we're requested to start is already the current user. final UserState state = getStartedUserState(userId); if (state == null) { Slogf.wtf(TAG, "Current user has no UserState"); @@ -1793,10 +1836,12 @@ class UserController implements Handler.Callback { t.traceEnd(); // updateStartedUserArrayStarting return true; } - final Integer userIdInt = userId; - mUserLru.remove(userIdInt); - mUserLru.add(userIdInt); } + + // No matter what, the fact that we're requested to start the user (even if it is + // already running) puts it towards the end of the mUserLru list. + addUserToUserLru(userId); + if (unlockListener != null) { uss.mUnlockProgress.addListener(unlockListener); } @@ -1835,12 +1880,10 @@ class UserController implements Handler.Callback { } } else { - final Integer currentUserIdInt = mCurrentUserId; updateProfileRelatedCaches(); - synchronized (mLock) { - mUserLru.remove(currentUserIdInt); - mUserLru.add(currentUserIdInt); - } + // We are starting a non-foreground user. They have already been added to the end + // of mUserLru, so we need to ensure that the foreground user isn't displaced. + addUserToUserLru(mCurrentUserId); } 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 ea1a68abe37b..a254fcf103f4 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -111,6 +111,8 @@ import com.android.server.pm.UserTypeFactory; import com.android.server.wm.ActivityTaskManagerInternal; import com.android.server.wm.WindowManagerService; +import com.google.common.collect.Range; + import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -155,6 +157,7 @@ public class UserControllerTest { private UserController mUserController; private TestInjector mInjector; private final HashMap<Integer, UserState> mUserStates = new HashMap<>(); + private final HashMap<Integer, UserInfo> mUserInfos = new HashMap<>(); private final KeyEvictedCallback mKeyEvictedCallback = (userId) -> { /* ignore */ }; @@ -587,25 +590,25 @@ public class UserControllerTest { setUpUser(TEST_USER_ID1, 0); setUpUser(TEST_USER_ID2, 0); - int numerOfUserSwitches = 1; + int numberOfUserSwitches = 1; addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM, - numerOfUserSwitches, false); + numberOfUserSwitches, false); // running: user 0, USER_ID assertTrue(mUserController.canStartMoreUsers()); assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID}), mUserController.getRunningUsersLU()); - numerOfUserSwitches++; + numberOfUserSwitches++; addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, TEST_USER_ID, - numerOfUserSwitches, false); + numberOfUserSwitches, false); // running: user 0, USER_ID, USER_ID1 assertFalse(mUserController.canStartMoreUsers()); assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID, TEST_USER_ID1}), mUserController.getRunningUsersLU()); - numerOfUserSwitches++; + numberOfUserSwitches++; addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, TEST_USER_ID1, - numerOfUserSwitches, false); + numberOfUserSwitches, false); UserState ussUser2 = mUserStates.get(TEST_USER_ID2); // skip middle step and call this directly. mUserController.finishUserSwitch(ussUser2); @@ -631,20 +634,20 @@ public class UserControllerTest { setUpUser(TEST_USER_ID1, 0); setUpUser(TEST_USER_ID2, 0); - int numerOfUserSwitches = 1; + int numberOfUserSwitches = 1; addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM, - numerOfUserSwitches, false); + numberOfUserSwitches, false); // running: user 0, USER_ID assertTrue(mUserController.canStartMoreUsers()); assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID}), mUserController.getRunningUsersLU()); - numerOfUserSwitches++; + numberOfUserSwitches++; addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, TEST_USER_ID, - numerOfUserSwitches, true); + numberOfUserSwitches, true); // running: user 0, USER_ID1 // stopped + unlocked: USER_ID - numerOfUserSwitches++; + numberOfUserSwitches++; assertTrue(mUserController.canStartMoreUsers()); assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID1}), mUserController.getRunningUsersLU()); @@ -659,7 +662,7 @@ public class UserControllerTest { .lockCeStorage(anyInt()); addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, TEST_USER_ID1, - numerOfUserSwitches, true); + numberOfUserSwitches, true); // running: user 0, USER_ID2 // stopped + unlocked: USER_ID1 // stopped + locked: USER_ID @@ -675,6 +678,105 @@ public class UserControllerTest { } /** + * Test that, in getRunningUsersLU, parents come after their profile, even if the profile was + * started afterwards. + */ + @Test + public void testRunningUsersListOrder_parentAfterProfile() { + mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true, + /* maxRunningUsers= */ 7, /* delayUserDataLocking= */ false); + + final int PARENT_ID = 200; + final int PROFILE1_ID = 201; + final int PROFILE2_ID = 202; + final int FG_USER_ID = 300; + final int BG_USER_ID = 400; + + setUpUser(PARENT_ID, 0).profileGroupId = PARENT_ID; + setUpUser(PROFILE1_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID; + setUpUser(PROFILE2_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID; + setUpUser(FG_USER_ID, 0).profileGroupId = FG_USER_ID; + setUpUser(BG_USER_ID, 0).profileGroupId = UserInfo.NO_PROFILE_GROUP_ID; + mUserController.onSystemReady(); // To set the profileGroupIds in UserController. + + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID}), + mUserController.getRunningUsersLU()); + + int numberOfUserSwitches = 1; + addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, + numberOfUserSwitches, false); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, PARENT_ID}), + mUserController.getRunningUsersLU()); + + assertThat(mUserController.startProfile(PROFILE1_ID, true, null)).isTrue(); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID}), + mUserController.getRunningUsersLU()); + + numberOfUserSwitches++; + addForegroundUserAndContinueUserSwitch(FG_USER_ID, PARENT_ID, + numberOfUserSwitches, false); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, FG_USER_ID}), + mUserController.getRunningUsersLU()); + + mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, BG_USER_ID, FG_USER_ID}), + mUserController.getRunningUsersLU()); + + assertThat(mUserController.startProfile(PROFILE2_ID, true, null)).isTrue(); + // Note for the future: + // It is not absolutely essential that PROFILE1 come before PROFILE2, + // nor that PROFILE1 come before BG_USER. We can change that policy later if we'd like. + // The important thing is that PROFILE1 and PROFILE2 precede PARENT, + // and that everything precedes OTHER. + assertEquals(Arrays.asList(new Integer[] { + SYSTEM_USER_ID, PROFILE1_ID, BG_USER_ID, PROFILE2_ID, PARENT_ID, FG_USER_ID}), + mUserController.getRunningUsersLU()); + } + + /** + * Test that, in getRunningUsersLU, the current user is always at the end, even if background + * users were started subsequently. + */ + @Test + public void testRunningUsersListOrder_currentAtEnd() { + mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true, + /* maxRunningUsers= */ 7, /* delayUserDataLocking= */ false); + + final int CURRENT_ID = 200; + final int PROFILE_ID = 201; + final int BG_USER_ID = 400; + + setUpUser(CURRENT_ID, 0).profileGroupId = CURRENT_ID; + setUpUser(PROFILE_ID, UserInfo.FLAG_PROFILE).profileGroupId = CURRENT_ID; + setUpUser(BG_USER_ID, 0).profileGroupId = BG_USER_ID; + mUserController.onSystemReady(); // To set the profileGroupIds in UserController. + + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID}), + mUserController.getRunningUsersLU()); + + addForegroundUserAndContinueUserSwitch(CURRENT_ID, UserHandle.USER_SYSTEM, 1, false); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, CURRENT_ID}), + mUserController.getRunningUsersLU()); + + mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, BG_USER_ID, CURRENT_ID}), + mUserController.getRunningUsersLU()); + + assertThat(mUserController.startProfile(PROFILE_ID, true, null)).isTrue(); + assertEquals(Arrays.asList( + new Integer[] {SYSTEM_USER_ID, BG_USER_ID, PROFILE_ID, CURRENT_ID}), + mUserController.getRunningUsersLU()); + } + + /** * Test locking user with mDelayUserDataLocking false. */ @Test @@ -785,6 +887,52 @@ public class UserControllerTest { verifyUserUnassignedFromDisplayNeverCalled(TEST_USER_ID); } + /** Test that stopping a profile doesn't also stop its parent, even if it's in background. */ + @Test + public void testStopProfile_doesNotStopItsParent() throws Exception { + mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true, + /* maxRunningUsers= */ 5, /* delayUserDataLocking= */ false); + + final Range<Integer> RUNNING_RANGE = + Range.closed(UserState.STATE_BOOTING, UserState.STATE_RUNNING_UNLOCKED); + + final int PARENT_ID = TEST_USER_ID1; + final int PROFILE_ID = TEST_USER_ID2; + final int OTHER_ID = TEST_USER_ID3; + + setUpUser(PARENT_ID, 0).profileGroupId = PARENT_ID; + setUpUser(PROFILE_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID; + setUpUser(OTHER_ID, 0).profileGroupId = OTHER_ID; + mUserController.onSystemReady(); // To set the profileGroupIds in UserController. + + // Start the parent in the background + boolean started = mUserController.startUser(PARENT_ID, USER_START_MODE_BACKGROUND); + assertWithMessage("startUser(%s)", PARENT_ID).that(started).isTrue(); + assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE); + + // Start the profile + started = mUserController.startProfile(PROFILE_ID, true, null); + assertWithMessage("startProfile(%s)", PROFILE_ID).that(started).isTrue(); + assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE); + assertThat(mUserController.getStartedUserState(PROFILE_ID).state).isIn(RUNNING_RANGE); + + // Start an unrelated user + started = mUserController.startUser(OTHER_ID, USER_START_MODE_FOREGROUND); + assertWithMessage("startUser(%s)", OTHER_ID).that(started).isTrue(); + assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE); + assertThat(mUserController.getStartedUserState(PROFILE_ID).state).isIn(RUNNING_RANGE); + assertThat(mUserController.getStartedUserState(OTHER_ID).state).isIn(RUNNING_RANGE); + + // Stop the profile and assert that its (background) parent didn't stop too + boolean stopped = mUserController.stopProfile(PROFILE_ID); + assertWithMessage("stopProfile(%s)", PROFILE_ID).that(stopped).isTrue(); + if (mUserController.getStartedUserState(PROFILE_ID) != null) { + assertThat(mUserController.getStartedUserState(PROFILE_ID).state) + .isNotIn(RUNNING_RANGE); + } + assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE); + } + @Test public void testStartProfile_disabledProfileFails() { setUpUser(TEST_USER_ID1, UserInfo.FLAG_PROFILE | UserInfo.FLAG_DISABLED, /* preCreated= */ @@ -1152,11 +1300,11 @@ public class UserControllerTest { continueUserSwitchAssertions(oldUserId, newUserId, expectOldUserStopping); } - private void setUpUser(@UserIdInt int userId, @UserInfoFlag int flags) { - setUpUser(userId, flags, /* preCreated= */ false, /* userType */ null); + private UserInfo setUpUser(@UserIdInt int userId, @UserInfoFlag int flags) { + return setUpUser(userId, flags, /* preCreated= */ false, /* userType */ null); } - private void setUpUser(@UserIdInt int userId, @UserInfoFlag int flags, boolean preCreated, + private UserInfo setUpUser(@UserIdInt int userId, @UserInfoFlag int flags, boolean preCreated, @Nullable String userType) { if (userType == null) { userType = UserInfo.getDefaultUserType(flags); @@ -1171,6 +1319,12 @@ public class UserControllerTest { assertThat(userTypeDetails).isNotNull(); when(mInjector.mUserManagerInternalMock.getUserProperties(eq(userId))) .thenReturn(userTypeDetails.getDefaultUserPropertiesReference()); + + mUserInfos.put(userId, userInfo); + when(mInjector.mUserManagerMock.getUsers(anyBoolean())) + .thenReturn(mUserInfos.values().stream().toList()); + + return userInfo; } private static List<String> getActions(List<Intent> intents) { |