From 413bfbf323ed6b15c53135f00483e0d1b7bd5785 Mon Sep 17 00:00:00 2001 From: Yasin Kilicdere Date: Tue, 27 Jun 2023 13:11:26 +0100 Subject: [PATCH] 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 --- .../android/server/pm/UserManagerService.java | 10 ++- .../server/pm/UserManagerServiceTest.java | 77 +++++++++++++++++++ .../UserManagerServiceCreateProfileTest.java | 2 +- .../pm/UserManagerServiceIdRecyclingTest.java | 2 +- 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 7e88e13e17888..280b3749a8bcb 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -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 diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java index e7b3e6f88dce5..617e4ebf11adc 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java @@ -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); diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceCreateProfileTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceCreateProfileTest.java index fdf94bec8c189..39cc6537c759d 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceCreateProfileTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceCreateProfileTest.java @@ -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, diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceIdRecyclingTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceIdRecyclingTest.java index 1f4c9f8cd3431..b6fd65e5d3b64 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceIdRecyclingTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceIdRecyclingTest.java @@ -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) {