Skip disabled users in UserManager.getPreviousForegroundUser() method.

When a guest user is reset in Settings, UM.markGuestForDeletion() is
called for the previous guest user and it gets marked with
FLAG_DISABLED. After the reset, if
UserManager.getPreviousForegroundUser() is called, it returns that
previous guest user which is marked as disabled, but it shouldn't.
This CL adds an additional userData.info.isEnabled() check to skip the
disabled users.

Bug: 283106632
Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest
Test: atest UserManagerServiceCreateProfileTest
Test: atest UserManagerServiceIdRecyclingTest
Change-Id: I544b96a7fd42a9542862eb804a5557f19f5a4104
This commit is contained in:
Yasin Kilicdere
2023-06-27 13:11:26 +01:00
parent 8b58515d9b
commit 413bfbf323
4 changed files with 87 additions and 4 deletions

View File

@@ -1037,7 +1037,7 @@ public class UserManagerService extends IUserManager.Stub {
final UserData userData = mUsers.valueAt(i);
final int userId = userData.info.id;
if (userId != currentUser && userData.info.isFull() && !userData.info.partial
&& !mRemovingUserIds.get(userId)) {
&& userData.info.isEnabled() && !mRemovingUserIds.get(userId)) {
final long userEnteredTime = userData.mLastEnteredForegroundTimeMillis;
if (userEnteredTime > latestEnteredTime) {
latestEnteredTime = userEnteredTime;
@@ -5589,8 +5589,14 @@ public class UserManagerService extends IUserManager.Stub {
}
}
@GuardedBy("mUsersLock")
@VisibleForTesting
void addRemovingUserId(@UserIdInt int userId) {
synchronized (mUsersLock) {
addRemovingUserIdLocked(userId);
}
}
@GuardedBy("mUsersLock")
void addRemovingUserIdLocked(@UserIdInt int userId) {
// We remember deleted user IDs to prevent them from being
// reused during the current boot; they can still be reused

View File

@@ -325,6 +325,83 @@ public final class UserManagerServiceTest {
() -> mUmi.getBootUser(/* waitUntilSet= */ false));
}
@Test
public void testGetPreviousFullUserToEnterForeground() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
assertWithMessage("getPreviousFullUserToEnterForeground")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(OTHER_USER_ID);
}
@Test
public void testGetPreviousFullUserToEnterForeground_SkipsCurrentUser() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
mockCurrentUser(OTHER_USER_ID);
assertWithMessage("getPreviousFullUserToEnterForeground should skip current user")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(USER_ID);
}
@Test
public void testGetPreviousFullUserToEnterForeground_SkipsNonFullUsers() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
mUsers.get(OTHER_USER_ID).info.flags &= ~UserInfo.FLAG_FULL;
assertWithMessage("getPreviousFullUserToEnterForeground should skip non-full users")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(USER_ID);
}
@Test
public void testGetPreviousFullUserToEnterForeground_SkipsPartialUsers() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
mUsers.get(OTHER_USER_ID).info.partial = true;
assertWithMessage("getPreviousFullUserToEnterForeground should skip partial users")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(USER_ID);
}
@Test
public void testGetPreviousFullUserToEnterForeground_SkipsDisabledUsers() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
mUsers.get(OTHER_USER_ID).info.flags |= UserInfo.FLAG_DISABLED;
assertWithMessage("getPreviousFullUserToEnterForeground should skip disabled users")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(USER_ID);
}
@Test
public void testGetPreviousFullUserToEnterForeground_SkipsRemovingUsers() throws Exception {
addUser(USER_ID);
setLastForegroundTime(USER_ID, 1_000_000L);
addUser(OTHER_USER_ID);
setLastForegroundTime(OTHER_USER_ID, 2_000_000L);
mUms.addRemovingUserId(OTHER_USER_ID);
assertWithMessage("getPreviousFullUserToEnterForeground should skip removing users")
.that(mUms.getPreviousFullUserToEnterForeground())
.isEqualTo(USER_ID);
}
private void mockCurrentUser(@UserIdInt int userId) {
mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal);

View File

@@ -182,7 +182,7 @@ public class UserManagerServiceCreateProfileTest {
UserInfo secondaryUser = addUser();
UserInfo profile = addProfile(secondaryUser);
// Add the profile it to the users being removed.
mUserManagerService.addRemovingUserIdLocked(profile.id);
mUserManagerService.addRemovingUserId(profile.id);
// We should reuse the badge from the profile being removed.
assertEquals("Badge index not reused while removing a user", 0,
mUserManagerService.getFreeProfileBadgeLU(secondaryUser.id,

View File

@@ -111,7 +111,7 @@ public class UserManagerServiceIdRecyclingTest {
private void removeUser(int userId) {
mUserManagerService.removeUserInfo(userId);
mUserManagerService.addRemovingUserIdLocked(userId);
mUserManagerService.addRemovingUserId(userId);
}
private void assertNoNextIdAvailable(String message) {