summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Adam Bookatz <bookatz@google.com> 2024-03-19 17:10:20 -0700
committer Adam Bookatz <bookatz@google.com> 2024-03-19 17:55:25 -0700
commitdcca5d8b2c8c5adc613873849e5819aa5ff9e19a (patch)
tree2cdc98deddfac3b0bb21ccdb7830de54c3ae1d4a
parent3c1f216e635c9dfcf048282a04b1a552b4aa52f5 (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.java7
-rw-r--r--services/tests/servicestests/src/com/android/server/am/UserControllerTest.java82
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.
*/