Added more unit tests for UserManagerInternal.

This CL adds tests for the following 2 methods: - void assignUserToDisplay(int userId) - void unassignUserFromDisplay(int displayId) It also makes some minor changes on those methods to throw exceptions on invalid usage. Bug: 244452695 Bug: 244644281 Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest UserManagerInternalTest Change-Id: Ieb17e7e8a43f7d697f98a45d1b0660d999ca28ec
2022-09-07 18:03:36 -07:00
parent 0e7702f7c8
commit 9e932dddc8
4 changed files with 278 additions and 16 deletions
--- 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 {
     *
     * <p>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).
+     *
+     * <p><b>NOTE: </b>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 {
     *
     * <p>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).
+     *
+     * <p><b>NOTE: </b>this method is meant to be used only by {@code UserController} when a user is
+     * stopped.
     */
    public abstract void unassignUserFromDisplay(@UserIdInt int userId);

--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -6711,10 +6711,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) {
@@ -6722,7 +6736,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);
@@ -6741,7 +6755,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
--- 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);
--- 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 <T> void mockGetLocalService(Class<T> serviceClass, T service) {
+    protected final <T> void mockGetLocalService(Class<T> 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<Integer, Integer>... others) {
+        Object[] otherObjects = new Object[others.length * 2];
+        for (int i = 0; i < others.length; i++) {
+            Pair<Integer, Integer> 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<Integer, Integer> 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<Integer, Integer> usersOnSecondaryDisplaysAsMap() {
+        int size = mUsersOnSecondaryDisplays.size();
+        Map<Integer, Integer> 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")