diff --git a/services/core/java/com/android/server/pm/UserManagerInternal.java b/services/core/java/com/android/server/pm/UserManagerInternal.java index a1d5054534839..62655843a5f2f 100644 --- a/services/core/java/com/android/server/pm/UserManagerInternal.java +++ b/services/core/java/com/android/server/pm/UserManagerInternal.java @@ -318,6 +318,10 @@ public abstract class UserManagerInternal { * *

On most devices this call will be a no-op, but it will be used on devices that support * multiple users on multiple displays (like automotives with passenger displays). + * + *

NOTE: this method is meant to be used only by {@code UserController} when a user is + * started and it doesn't validate if the display exists. + * */ public abstract void assignUserToDisplay(@UserIdInt int userId, int displayId); @@ -326,6 +330,9 @@ public abstract class UserManagerInternal { * *

On most devices this call will be a no-op, but it will be used on devices that support * multiple users on multiple displays (like automotives with passenger displays). + * + *

NOTE: this method is meant to be used only by {@code UserController} when a user is + * stopped. */ public abstract void unassignUserFromDisplay(@UserIdInt int userId); diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 4b9fbd181dc5d..1e30757b8417c 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -6732,10 +6732,24 @@ public class UserManagerService extends IUserManager.Stub { + "users on multiple displays"); } - Preconditions.checkArgument(userId != UserHandle.USER_SYSTEM, "Cannot start system user" - + " on secondary display (%d)", displayId); - // TODO(b/239982558): call DisplayManagerInternal to check if display is valid instead - Preconditions.checkArgument(displayId > 0, "Invalid display id (%d)", displayId); + Preconditions.checkArgument(userId != UserHandle.USER_SYSTEM, "Cannot assign system " + + "user to secondary display (%d)", displayId); + Preconditions.checkArgument(displayId != Display.INVALID_DISPLAY, + "Cannot assign to INVALID_DISPLAY (%d)", displayId); + + int currentUserId = getCurrentUserId(); + Preconditions.checkArgument(userId != currentUserId, + "Cannot assign current user to other displays"); + + boolean isProfile = isProfileUnchecked(userId); + + Preconditions.checkArgument(userId != currentUserId, + "Cannot assign current user to other displays"); + + Preconditions.checkArgument( + !isProfile || getProfileParentIdUnchecked(userId) != currentUserId, + "Cannot assign profile user %d to display %d when its parent is the current " + + "user (%d)", userId, displayId, currentUserId); synchronized (mUsersOnSecondaryDisplays) { if (DBG_MUMD) { @@ -6743,7 +6757,7 @@ public class UserManagerService extends IUserManager.Stub { userId, displayId); } - if (isProfileUnchecked(userId)) { + if (isProfile) { // Profile can only start in the same display as parent int parentUserId = getProfileParentId(userId); int parentDisplayId = mUsersOnSecondaryDisplays.get(parentUserId); @@ -6762,7 +6776,7 @@ public class UserManagerService extends IUserManager.Stub { // is refactored, it should be atomic. if (mUsersOnSecondaryDisplays.valueAt(i) == displayId) { throw new IllegalStateException("Cannot assign " + userId + " to " - + "display " + displayId + " as it's already assigned to " + + "display " + displayId + " as it's already assigned to " + "user " + mUsersOnSecondaryDisplays.keyAt(i)); } // TODO(b/239982558) also check that user is not already assigned to other diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerInternalTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerInternalTest.java index 7d7b70ca19f6c..1cff685522a29 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerInternalTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerInternalTest.java @@ -15,11 +15,206 @@ */ package com.android.server.pm; +import static android.os.UserHandle.USER_SYSTEM; +import static android.view.Display.DEFAULT_DISPLAY; +import static android.view.Display.INVALID_DISPLAY; + +import static com.google.common.truth.Truth.assertWithMessage; + +import static org.junit.Assert.assertThrows; + +import android.util.Log; + +import org.junit.Test; + /** * Run as {@code atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerInternalTest} */ public final class UserManagerInternalTest extends UserManagerServiceOrInternalTestCase { + private static final String TAG = UserManagerInternalTest.class.getSimpleName(); + + // NOTE: most the tests below only apply to MUMD configurations, so we're not adding _mumd_ + // in the test names, but _nonMumd_ instead + + @Test + public void testAssignUserToDisplay_nonMumd_defaultDisplayIgnored() { + mUmi.assignUserToDisplay(USER_ID, DEFAULT_DISPLAY); + + assertNoUserAssignedToDisplay(); + } + + @Test + public void testAssignUserToDisplay_nonMumd_otherDisplay_currentUser() { + mockCurrentUser(USER_ID); + + assertThrows(UnsupportedOperationException.class, + () -> mUmi.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID)); + } + + @Test + public void testAssignUserToDisplay_nonMumd_otherDisplay_startProfileOfcurrentUser() { + mockCurrentUser(PARENT_USER_ID); + addDefaultProfileAndParent(); + startDefaultProfile(); + + assertThrows(UnsupportedOperationException.class, + () -> mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + } + + @Test + public void testAssignUserToDisplay_nonMumd_otherDisplay_stoppedProfileOfcurrentUser() { + mockCurrentUser(PARENT_USER_ID); + addDefaultProfileAndParent(); + stopDefaultProfile(); + + assertThrows(UnsupportedOperationException.class, + () -> mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + } + + @Test + public void testAssignUserToDisplay_defaultDisplayIgnored() { + enableUsersOnSecondaryDisplays(); + + mUmi.assignUserToDisplay(USER_ID, DEFAULT_DISPLAY); + + assertNoUserAssignedToDisplay(); + } + + @Test + public void testAssignUserToDisplay_systemUser() { + enableUsersOnSecondaryDisplays(); + + assertThrows(IllegalArgumentException.class, + () -> mUmi.assignUserToDisplay(USER_SYSTEM, SECONDARY_DISPLAY_ID)); + } + + @Test + public void testAssignUserToDisplay_invalidDisplay() { + enableUsersOnSecondaryDisplays(); + + assertThrows(IllegalArgumentException.class, + () -> mUmi.assignUserToDisplay(USER_ID, INVALID_DISPLAY)); + } + + @Test + public void testAssignUserToDisplay_currentUser() { + enableUsersOnSecondaryDisplays(); + mockCurrentUser(USER_ID); + + assertThrows(IllegalArgumentException.class, + () -> mUmi.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID)); + + assertNoUserAssignedToDisplay(); + } + + @Test + public void testAssignUserToDisplay_startedProfileOfCurrentUser() { + enableUsersOnSecondaryDisplays(); + mockCurrentUser(PARENT_USER_ID); + addDefaultProfileAndParent(); + startDefaultProfile(); + + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + + Log.v(TAG, "Exception: " + e); + assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() + .matches("Cannot.*" + PROFILE_USER_ID + ".*" + SECONDARY_DISPLAY_ID + + ".*parent.*current.*" + PARENT_USER_ID + ".*"); + assertNoUserAssignedToDisplay(); + } + + @Test + public void testAssignUserToDisplay_stoppedProfileOfCurrentUser() { + enableUsersOnSecondaryDisplays(); + mockCurrentUser(PARENT_USER_ID); + addDefaultProfileAndParent(); + stopDefaultProfile(); + + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + + Log.v(TAG, "Exception: " + e); + assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() + .matches("Cannot.*" + PROFILE_USER_ID + ".*" + SECONDARY_DISPLAY_ID + + ".*parent.*current.*" + PARENT_USER_ID + ".*"); + + assertNoUserAssignedToDisplay(); + } + + @Test + public void testAssignUserToDisplay_displayAvailable() { + enableUsersOnSecondaryDisplays(); + + mUmi.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + + assertUserAssignedToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + } + + @Test + public void testAssignUserToDisplay_displayAlreadyAssigned() { + enableUsersOnSecondaryDisplays(); + + mUmi.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> mUmi.assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID)); + + Log.v(TAG, "Exception: " + e); + assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() + .matches("Cannot.*" + OTHER_USER_ID + ".*" + SECONDARY_DISPLAY_ID + ".*already.*" + + USER_ID + ".*"); + } + + @Test + public void testAssignUserToDisplay_profileOnSameDisplayAsParent() { + enableUsersOnSecondaryDisplays(); + addDefaultProfileAndParent(); + + mUmi.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); + mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID); + + assertUsersAssignedToDisplays(PARENT_USER_ID, SECONDARY_DISPLAY_ID, + pair(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + } + + @Test + public void testAssignUserToDisplay_profileOnDifferentDisplayAsParent() { + enableUsersOnSecondaryDisplays(); + addDefaultProfileAndParent(); + + mUmi.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> mUmi.assignUserToDisplay(PROFILE_USER_ID, OTHER_SECONDARY_DISPLAY_ID)); + + Log.v(TAG, "Exception: " + e); + assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() + .matches("Cannot.*" + PROFILE_USER_ID + ".*" + OTHER_SECONDARY_DISPLAY_ID + + ".*parent.*" + PARENT_USER_ID + ".*" + SECONDARY_DISPLAY_ID + ".*"); + assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); + } + + @Test + public void testUnassignUserFromDisplay_nonMumd_ignored() { + mockCurrentUser(USER_ID); + + mUmi.unassignUserFromDisplay(USER_SYSTEM); + mUmi.unassignUserFromDisplay(USER_ID); + mUmi.unassignUserFromDisplay(OTHER_USER_ID); + + assertNoUserAssignedToDisplay(); + } + + @Test + public void testUnassignUserFromDisplay() { + testAssignUserToDisplay_displayAvailable(); + + mUmi.unassignUserFromDisplay(USER_ID); + + assertNoUserAssignedToDisplay(); + } + @Override protected boolean isUserVisible(int userId) { return mUmi.isUserVisible(userId); diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java index e021e767e7c81..4f61b8fff995d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java @@ -33,6 +33,7 @@ import android.content.Context; import android.content.pm.UserInfo; import android.os.UserManager; import android.util.Log; +import android.util.Pair; import android.util.SparseArray; import android.util.SparseIntArray; @@ -49,6 +50,9 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mock; +import java.util.LinkedHashMap; +import java.util.Map; + /** * Base class for {@link UserManagerInternalTest} and {@link UserManagerInternalTest}. * @@ -90,7 +94,12 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC /** * Id of a secondary display (i.e, not {@link android.view.Display.DEFAULT_DISPLAY}). */ - private static final int SECONDARY_DISPLAY_ID = 42; + protected static final int SECONDARY_DISPLAY_ID = 42; + + /** + * Id of another secondary display (i.e, not {@link android.view.Display.DEFAULT_DISPLAY}). + */ + protected static final int OTHER_SECONDARY_DISPLAY_ID = 108; private final Object mPackagesLock = new Object(); private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation() @@ -426,26 +435,26 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC * Change test fixtures to use a version that supports {@code MUMD} (Multiple Users on Multiple * Displays). */ - protected void enableUsersOnSecondaryDisplays() { + protected final void enableUsersOnSecondaryDisplays() { setServiceFixtures(/* usersOnSecondaryDisplaysEnabled= */ true); } - protected void mockCurrentUser(@UserIdInt int userId) { + protected final void mockCurrentUser(@UserIdInt int userId) { mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal); when(mActivityManagerInternal.getCurrentUserId()).thenReturn(userId); } - protected void mockGetLocalService(Class serviceClass, T service) { + protected final void mockGetLocalService(Class serviceClass, T service) { doReturn(service).when(() -> LocalServices.getService(serviceClass)); } - protected void addDefaultProfileAndParent() { + protected final void addDefaultProfileAndParent() { addUser(PARENT_USER_ID); addProfile(PROFILE_USER_ID, PARENT_USER_ID); } - protected void addProfile(@UserIdInt int profileId, @UserIdInt int parentId) { + protected final void addProfile(@UserIdInt int profileId, @UserIdInt int parentId) { TestUserData profileData = new TestUserData(profileId); profileData.info.flags = UserInfo.FLAG_PROFILE; profileData.info.profileGroupId = parentId; @@ -453,27 +462,55 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC addUserData(profileData); } - protected void addUser(@UserIdInt int userId) { + protected final void addUser(@UserIdInt int userId) { TestUserData userData = new TestUserData(userId); addUserData(userData); } - protected void startDefaultProfile() { + protected final void startDefaultProfile() { setUserState(PROFILE_USER_ID, UserState.STATE_RUNNING_UNLOCKED); } - protected void stopDefaultProfile() { + protected final void stopDefaultProfile() { // TODO(b/244798930): should set it to STATE_STOPPING or STATE_SHUTDOWN instead removeUserState(PROFILE_USER_ID); } // NOTE: should only called by tests that indirectly needs to check user assignments (like // isUserVisible), not by tests for the user assignment methods per se. - protected void assignUserToDisplay(@UserIdInt int userId, int displayId) { + protected final void assignUserToDisplay(@UserIdInt int userId, int displayId) { mUsersOnSecondaryDisplays.put(userId, displayId); } + protected final void assertNoUserAssignedToDisplay() { + assertWithMessage("mUsersOnSecondaryDisplays()").that(usersOnSecondaryDisplaysAsMap()) + .isEmpty(); + } + + protected final void assertUserAssignedToDisplay(@UserIdInt int userId, int displayId) { + assertWithMessage("mUsersOnSecondaryDisplays()").that(usersOnSecondaryDisplaysAsMap()) + .containsExactly(userId, displayId); + } + + @SafeVarargs + protected final void assertUsersAssignedToDisplays(@UserIdInt int userId, int displayId, + @SuppressWarnings("unchecked") Pair... others) { + Object[] otherObjects = new Object[others.length * 2]; + for (int i = 0; i < others.length; i++) { + Pair other = others[i]; + otherObjects[i * 2] = other.first; + otherObjects[i * 2 + 1] = other.second; + + } + assertWithMessage("mUsersOnSecondaryDisplays()").that(usersOnSecondaryDisplaysAsMap()) + .containsExactly(userId, displayId, otherObjects); + } + + protected static Pair pair(@UserIdInt int userId, int secondaryDisplayId) { + return new Pair<>(userId, secondaryDisplayId); + } + /////////////////// // Private infra // /////////////////// @@ -508,6 +545,15 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC mUmi.removeUserState(userId); } + private Map usersOnSecondaryDisplaysAsMap() { + int size = mUsersOnSecondaryDisplays.size(); + Map map = new LinkedHashMap<>(size); + for (int i = 0; i < size; i++) { + map.put(mUsersOnSecondaryDisplays.keyAt(i), mUsersOnSecondaryDisplays.valueAt(i)); + } + return map; + } + private static final class TestUserData extends UserData { @SuppressWarnings("deprecation")