diff options
| author | 2024-03-19 17:10:20 -0700 | |
|---|---|---|
| committer | 2024-03-19 17:55:25 -0700 | |
| commit | dcca5d8b2c8c5adc613873849e5819aa5ff9e19a (patch) | |
| tree | 2cdc98deddfac3b0bb21ccdb7830de54c3ae1d4a | |
| parent | 3c1f216e635c9dfcf048282a04b1a552b4aa52f5 (diff) | |
Don't stop current user's profile even if excess
In Ie08342bb77544117cab4423b543af74cbeb109e2 we fixed some bugs relating
to stopping profiles' parents. However, this could cause the "force"
parameter to not be handled properly. We almost never actually want it,
but:
The case of stopping users when exceeding the maximum running users
is one of the few cases where we want "force==false" to truly not
stop a profile if its parent is the current user. As a result, we cannot
simply ignore the notion of "force".
We need to really refactor this notion of "force", but in the meantime,
here's a quick fix and test.
Test: atest UserControllerTest#testStoppingExcessRunningUsersAfterSwitch_currentProfileNotStopped
Bug: 324647580
Bug: 310249114
Change-Id: I4f9c5a11908fd9f7ad381d337d11af5c3058bc3d
| -rw-r--r-- | services/core/java/com/android/server/am/UserController.java | 7 | ||||
| -rw-r--r-- | services/tests/servicestests/src/com/android/server/am/UserControllerTest.java | 82 |
2 files changed, 89 insertions, 0 deletions
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index 41d52046b6e8..45a5fa318cb5 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -991,6 +991,13 @@ class UserController implements Handler.Callback { if (isCurrentUserLU(userId)) { return USER_OP_IS_CURRENT; } + // TODO(b/324647580): Refactor the idea of "force" and clean up. In the meantime... + final int parentId = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID); + if (parentId != UserInfo.NO_PROFILE_GROUP_ID && parentId != userId) { + if ((UserHandle.USER_SYSTEM == parentId || isCurrentUserLU(parentId)) && !force) { + return USER_OP_ERROR_RELATED_USERS_CANNOT_STOP; + } + } TimingsTraceAndSlog t = new TimingsTraceAndSlog(); int[] usersToStop = getUsersToStopLU(userId); // If one of related users is system or current, no related users should be stopped 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 a254fcf103f4..ad81e9a8275d 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -123,6 +123,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -678,6 +679,87 @@ public class UserControllerTest { } /** + * Test that, when exceeding the maximum number of running users, a profile of the current user + * is not stopped. + */ + @Test + public void testStoppingExcessRunningUsersAfterSwitch_currentProfileNotStopped() + throws Exception { + mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true, + /* maxRunningUsers= */ 5, /* 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(newHashSet( + SYSTEM_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + int numberOfUserSwitches = 1; + addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, + numberOfUserSwitches, false); + mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); + waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS); + assertTrue(mUserController.canStartMoreUsers()); + assertEquals(newHashSet( + SYSTEM_USER_ID, PARENT_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + assertThat(mUserController.startProfile(PROFILE1_ID, true, null)).isTrue(); + assertEquals(newHashSet( + SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + numberOfUserSwitches++; + addForegroundUserAndContinueUserSwitch(FG_USER_ID, PARENT_ID, + numberOfUserSwitches, false); + mUserController.finishUserSwitch(mUserStates.get(FG_USER_ID)); + waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS); + assertTrue(mUserController.canStartMoreUsers()); + assertEquals(newHashSet( + SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, FG_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND); + assertEquals(newHashSet( + SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, BG_USER_ID, FG_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + // Now we exceed the maxRunningUsers parameter (of 5): + assertThat(mUserController.startProfile(PROFILE2_ID, true, null)).isTrue(); + // Currently, starting a profile doesn't trigger evaluating whether we've exceeded max, so + // we expect no users to be stopped. This policy may change in the future. Log but no fail. + if (!newHashSet(SYSTEM_USER_ID, PROFILE1_ID, BG_USER_ID, PROFILE2_ID, PARENT_ID, FG_USER_ID) + .equals(new HashSet<>(mUserController.getRunningUsersLU()))) { + Log.w(TAG, "Starting a profile that exceeded max running users didn't lead to " + + "expectations: " + mUserController.getRunningUsersLU()); + } + + numberOfUserSwitches++; + addForegroundUserAndContinueUserSwitch(PARENT_ID, FG_USER_ID, + numberOfUserSwitches, false); + mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); + waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS); + // We've now done a user switch and should notice that we've exceeded the maximum number of + // users. The oldest background user should be stopped (BG_USER); even though PROFILE1 was + // older, it should not be stopped since it's a profile of the (new) current user. + assertFalse(mUserController.canStartMoreUsers()); + assertEquals(newHashSet( + SYSTEM_USER_ID, PROFILE1_ID, PROFILE2_ID, FG_USER_ID, PARENT_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + } + + /** * Test that, in getRunningUsersLU, parents come after their profile, even if the profile was * started afterwards. */ |