Merge "Revert "Revert "Fixed UserVisibilityMediator profile scenario.""" into udc-dev am: 1a8c7c147d

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21616568

Change-Id: Ie77a56d5ac38ad4e82327db34cc50fa819149112
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/pm/UserManagerInternal.java

@@ -64,12 +64,11 @@ public abstract class UserManagerInternal {
     })
     public @interface UserAssignmentResult {}
 
-    // TODO(b/248408342): Move keep annotation to the method referencing these fields reflectively.
-    @Keep public static final int USER_START_MODE_FOREGROUND = 1;
-    @Keep public static final int USER_START_MODE_BACKGROUND = 2;
-    @Keep public static final int USER_START_MODE_BACKGROUND_VISIBLE = 3;
-
     private static final String PREFIX_USER_START_MODE = "USER_START_MODE_";
+
+    /**
+     * Type used to indicate how a user started.
+     */
     @IntDef(flag = false, prefix = {PREFIX_USER_START_MODE}, value = {
             USER_START_MODE_FOREGROUND,
             USER_START_MODE_BACKGROUND,
@@ -77,6 +76,32 @@ public abstract class UserManagerInternal {
     })
     public @interface UserStartMode {}
 
+    // TODO(b/248408342): Move keep annotations below to the method referencing these fields
+    // reflectively.
+
+    /** (Full) user started on foreground (a.k.a. "current user"). */
+    @Keep public static final int USER_START_MODE_FOREGROUND = 1;
+
+    /**
+     * User (full or profile) started on background and is
+     * {@link UserManager#isUserVisible() invisible}.
+     *
+     * <p>This is the "traditional" way of starting a background user, and can be used to start
+     * profiles as well, although starting an invisible profile is not common from the System UI
+     * (it could be done through APIs or adb, though).
+     */
+    @Keep public static final int USER_START_MODE_BACKGROUND = 2;
+
+    /**
+     * User (full or profile) started on background and is
+     * {@link UserManager#isUserVisible() visible}.
+     *
+     * <p>This is the "traditional" way of starting a profile (i.e., when the profile of the current
+     * user is the current foreground user), but it can also be used to start a full user associated
+     * with a display (which is the case on automotives with passenger displays).
+     */
+    @Keep public static final int USER_START_MODE_BACKGROUND_VISIBLE = 3;
+
     public interface UserRestrictionsListener {
         /**
          * Called when a user restriction changes.

services/core/java/com/android/server/pm/UserVisibilityMediator.java

@@ -42,6 +42,7 @@ import android.util.Dumpable;
 import android.util.EventLog;
 import android.util.IndentingPrintWriter;
 import android.util.IntArray;
+import android.util.Log;
 import android.util.SparseIntArray;
 import android.view.Display;
 
@@ -55,6 +56,8 @@ import com.android.server.pm.UserManagerInternal.UserVisibilityListener;
 import com.android.server.utils.Slogf;
 
 import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
@@ -77,11 +80,11 @@ import java.util.concurrent.CopyOnWriteArrayList;
  */
 public final class UserVisibilityMediator implements Dumpable {
 
-    private static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE
-    private static final boolean VERBOSE = false; // DO NOT SUBMIT WITH TRUE
-
     private static final String TAG = UserVisibilityMediator.class.getSimpleName();
 
+    private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG);
+    private static final boolean VERBOSE = false; // DO NOT SUBMIT WITH TRUE
+
     private static final String PREFIX_SECONDARY_DISPLAY_MAPPING = "SECONDARY_DISPLAY_MAPPING_";
     public static final int SECONDARY_DISPLAY_MAPPING_NEEDED = 1;
     public static final int SECONDARY_DISPLAY_MAPPING_NOT_NEEDED = 2;
@@ -98,7 +101,7 @@ public final class UserVisibilityMediator implements Dumpable {
     })
     public @interface SecondaryDisplayMappingStatus {}
 
-    // TODO(b/242195409): might need to change this if boot logic is refactored for HSUM devices
+    // TODO(b/266158156): might need to change this if boot logic is refactored for HSUM devices
     @VisibleForTesting
     static final int INITIAL_CURRENT_USER_ID = USER_SYSTEM;
 
@@ -132,10 +135,23 @@ public final class UserVisibilityMediator implements Dumpable {
     private final SparseIntArray mExtraDisplaysAssignedToUsers;
 
     /**
-     * Mapping from each started user to its profile group.
+     * Mapping of each user that started visible (key) to its profile group id (value).
+     *
+     * <p>It's used to determine not just if the user is visible, but also
+     * {@link #isProfile(int, int) if it's a profile}.
      */
     @GuardedBy("mLock")
-    private final SparseIntArray mStartedProfileGroupIds = new SparseIntArray();
+    private final SparseIntArray mStartedVisibleProfileGroupIds = new SparseIntArray();
+
+    /**
+     * List of profiles that have explicitly started invisible.
+     *
+     * <p>Only used for debugging purposes (and set when {@link #DBG} is {@code true}), hence we
+     * don't care about autoboxing.
+     */
+    @GuardedBy("mLock")
+    @Nullable
+    private final List<Integer> mStartedInvisibleProfileUserIds;
 
     /**
      * Handler user to call listeners
@@ -164,9 +180,14 @@ public final class UserVisibilityMediator implements Dumpable {
             mUsersAssignedToDisplayOnStart = null;
             mExtraDisplaysAssignedToUsers = null;
         }
+        mStartedInvisibleProfileUserIds = DBG ? new ArrayList<>(4) : null;
         mHandler = handler;
-        // TODO(b/242195409): might need to change this if boot logic is refactored for HSUM devices
+        // TODO(b/266158156): might need to change this if boot logic is refactored for HSUM devices
-        mStartedProfileGroupIds.put(INITIAL_CURRENT_USER_ID, INITIAL_CURRENT_USER_ID);
+        mStartedVisibleProfileGroupIds.put(INITIAL_CURRENT_USER_ID, INITIAL_CURRENT_USER_ID);
+
+        if (DBG) {
+            Slogf.i(TAG, "UserVisibilityMediator created with DBG on");
+        }
     }
 
     /**
@@ -177,6 +198,8 @@ public final class UserVisibilityMediator implements Dumpable {
             int displayId) {
         Preconditions.checkArgument(!isSpecialUserId(userId), "user id cannot be generic: %d",
                 userId);
+        validateUserStartMode(userStartMode);
+
         // This method needs to perform 4 actions:
         //
         // 1. Check if the user can be started given the provided arguments
@@ -224,14 +247,29 @@ public final class UserVisibilityMediator implements Dumpable {
 
             visibleUsersBefore = getVisibleUsers();
 
-            // Set current user / profiles state
+            // Set current user / started users state
-            if (userStartMode == USER_START_MODE_FOREGROUND) {
+            switch (userStartMode) {
-                mCurrentUserId = userId;
+                case USER_START_MODE_FOREGROUND:
+                    mCurrentUserId = userId;
+                    // Fallthrough
+                case USER_START_MODE_BACKGROUND_VISIBLE:
+                    if (DBG) {
+                        Slogf.d(TAG, "adding visible user / profile group id mapping (%d -> %d)",
+                                userId, profileGroupId);
+                    }
+                    mStartedVisibleProfileGroupIds.put(userId, profileGroupId);
+                    break;
+                case USER_START_MODE_BACKGROUND:
+                    if (mStartedInvisibleProfileUserIds != null
+                            && isProfile(userId, profileGroupId)) {
+                        Slogf.d(TAG, "adding user %d to list of invisible profiles", userId);
+                        mStartedInvisibleProfileUserIds.add(userId);
+                    }
+                    break;
+                default:
+                    Slogf.wtf(TAG,  "invalid userStartMode passed to assignUserToDisplayOnStart: "
+                            + "%d", userStartMode);
             }
-            if (DBG) {
-                Slogf.d(TAG, "adding user / profile mapping (%d -> %d)", userId, profileGroupId);
-            }
-            mStartedProfileGroupIds.put(userId, profileGroupId);
 
             //  Set user / display state
             switch (mappingResult) {
@@ -297,38 +335,44 @@ public final class UserVisibilityMediator implements Dumpable {
         boolean foreground = userStartMode == USER_START_MODE_FOREGROUND;
         if (displayId != DEFAULT_DISPLAY) {
             if (foreground) {
-                Slogf.w(TAG, "getUserVisibilityOnStartLocked(%d, %d, %b, %d) failed: cannot start "
+                Slogf.w(TAG, "getUserVisibilityOnStartLocked(%d, %d, %s, %d) failed: cannot start "
                         + "foreground user on secondary display", userId, profileGroupId,
-                        foreground, displayId);
+                        userStartModeToString(userStartMode), displayId);
                 return USER_ASSIGNMENT_RESULT_FAILURE;
             }
             if (!mVisibleBackgroundUsersEnabled) {
-                Slogf.w(TAG, "getUserVisibilityOnStartLocked(%d, %d, %b, %d) failed: called on "
+                Slogf.w(TAG, "getUserVisibilityOnStartLocked(%d, %d, %s, %d) failed: called on "
                         + "device that doesn't support multiple users on multiple displays",
-                        userId, profileGroupId, foreground, displayId);
+                        userId, profileGroupId, userStartModeToString(userStartMode), displayId);
                 return USER_ASSIGNMENT_RESULT_FAILURE;
             }
         }
 
         if (isProfile(userId, profileGroupId)) {
             if (displayId != DEFAULT_DISPLAY) {
-                Slogf.w(TAG, "canStartUserLocked(%d, %d, %b, %d) failed: cannot start profile user "
+                Slogf.w(TAG, "canStartUserLocked(%d, %d, %s, %d) failed: cannot start profile user "
-                        + "on secondary display", userId, profileGroupId, foreground,
+                        + "on secondary display", userId, profileGroupId,
-                        displayId);
+                        userStartModeToString(userStartMode), displayId);
                 return USER_ASSIGNMENT_RESULT_FAILURE;
             }
-            if (foreground) {
+            switch (userStartMode) {
-                Slogf.w(TAG, "startUser(%d, %d, %b, %d) failed: cannot start profile user in "
+                case USER_START_MODE_FOREGROUND:
-                        + "foreground", userId, profileGroupId, foreground, displayId);
+                    Slogf.w(TAG, "startUser(%d, %d, %s, %d) failed: cannot start profile user in "
-                return USER_ASSIGNMENT_RESULT_FAILURE;
+                            + "foreground", userId, profileGroupId,
-            } else {
+                            userStartModeToString(userStartMode), displayId);
-                boolean isParentVisibleOnDisplay = isUserVisible(profileGroupId, displayId);
+                    return USER_ASSIGNMENT_RESULT_FAILURE;
-                if (DBG) {
+                case USER_START_MODE_BACKGROUND_VISIBLE:
-                    Slogf.d(TAG, "parent visible on display: %b", isParentVisibleOnDisplay);
+                    boolean isParentVisibleOnDisplay = isUserVisible(profileGroupId, displayId);
-                }
+                    if (!isParentVisibleOnDisplay) {
-                return isParentVisibleOnDisplay
+                        Slogf.w(TAG, "getUserVisibilityOnStartLocked(%d, %d, %s, %d) failed: cannot"
-                        ? USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE
+                                + " start profile user visible when its parent is not visible in "
-                        : USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
+                                + "that display", userId, profileGroupId,
+                                userStartModeToString(userStartMode), displayId);
+                        return USER_ASSIGNMENT_RESULT_FAILURE;
+                    }
+                    return USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE;
+                case USER_START_MODE_BACKGROUND:
+                    return USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
             }
         } else if (mUsersAssignedToDisplayOnStart != null
                 && isUserAssignedToDisplayOnStartLocked(userId, displayId)) {
@@ -353,8 +397,9 @@ public final class UserVisibilityMediator implements Dumpable {
             if (mVisibleBackgroundUserOnDefaultDisplayAllowed
                     && userStartMode == USER_START_MODE_BACKGROUND_VISIBLE) {
                 int userStartedOnDefaultDisplay = getUserStartedOnDisplay(DEFAULT_DISPLAY);
-                if (userStartedOnDefaultDisplay != USER_NULL) {
+                if (userStartedOnDefaultDisplay != USER_NULL
-                    Slogf.w(TAG, "getUserVisibilityOnStartLocked(): cannot start user %d visible on"
+                        && userStartedOnDefaultDisplay != profileGroupId) {
+                    Slogf.w(TAG, "canAssignUserToDisplayLocked(): cannot start user %d visible on"
                             + " default display because user %d already did so", userId,
                             userStartedOnDefaultDisplay);
                     return SECONDARY_DISPLAY_MAPPING_FAILED;
@@ -468,7 +513,7 @@ public final class UserVisibilityMediator implements Dumpable {
                         userId, displayId);
                 return false;
             }
-            if (isStartedProfile(userId)) {
+            if (isStartedVisibleProfileLocked(userId)) {
                 Slogf.w(TAG, "assignUserToExtraDisplay(%d, %d): failed because user is a profile",
                         userId, displayId);
                 return false;
@@ -556,10 +601,14 @@ public final class UserVisibilityMediator implements Dumpable {
     @GuardedBy("mLock")
     private void unassignUserFromAllDisplaysOnStopLocked(@UserIdInt int userId) {
         if (DBG) {
-            Slogf.d(TAG, "Removing %d from mStartedProfileGroupIds (%s)", userId,
+            Slogf.d(TAG, "Removing %d from mStartedVisibleProfileGroupIds (%s)", userId,
-                    mStartedProfileGroupIds);
+                    mStartedVisibleProfileGroupIds);
+        }
+        mStartedVisibleProfileGroupIds.delete(userId);
+        if (mStartedInvisibleProfileUserIds != null) {
+            Slogf.d(TAG, "Removing %d from list of invisible profiles", userId);
+            mStartedInvisibleProfileUserIds.remove(Integer.valueOf(userId));
         }
-        mStartedProfileGroupIds.delete(userId);
 
         if (!mVisibleBackgroundUsersEnabled) {
             // Don't need to update mUsersAssignedToDisplayOnStart because methods (such as
@@ -589,7 +638,8 @@ public final class UserVisibilityMediator implements Dumpable {
      * See {@link UserManagerInternal#isUserVisible(int)}.
      */
     public boolean isUserVisible(@UserIdInt int userId) {
-        // First check current foreground user and their profiles (on main display)
+        // For optimization (as most devices don't support visible background users), check for
+        // current foreground user and their profiles first
         if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) {
             if (VERBOSE) {
                 Slogf.v(TAG, "isUserVisible(%d): true to current user or profile", userId);
@@ -598,19 +648,31 @@ public final class UserVisibilityMediator implements Dumpable {
         }
 
         if (!mVisibleBackgroundUsersEnabled) {
-            if (DBG) {
+            if (VERBOSE) {
-                Slogf.d(TAG, "isUserVisible(%d): false for non-current user (or its profiles) when"
+                Slogf.v(TAG, "isUserVisible(%d): false for non-current user (or its profiles) when"
                         + " device doesn't support visible background users", userId);
             }
             return false;
         }
 
-        boolean visible;
+
         synchronized (mLock) {
-            visible = mUsersAssignedToDisplayOnStart.indexOfKey(userId) >= 0;
+            int profileGroupId;
+            synchronized (mLock) {
+                profileGroupId = mStartedVisibleProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID);
+            }
+            if (isProfile(userId, profileGroupId)) {
+                return isUserAssignedToDisplayOnStartLocked(profileGroupId);
+            }
+            return isUserAssignedToDisplayOnStartLocked(userId);
         }
-        if (DBG) {
+    }
-            Slogf.d(TAG, "isUserVisible(%d): %b from mapping", userId, visible);
+
+    @GuardedBy("mLock")
+    private boolean isUserAssignedToDisplayOnStartLocked(@UserIdInt int userId) {
+        boolean visible = mUsersAssignedToDisplayOnStart.indexOfKey(userId) >= 0;
+        if (VERBOSE) {
+            Slogf.v(TAG, "isUserAssignedToDisplayOnStartLocked(%d): %b", userId, visible);
         }
         return visible;
     }
@@ -640,7 +702,8 @@ public final class UserVisibilityMediator implements Dumpable {
             return false;
         }
 
-        // Current user is always visible on:
+        // For optimization (as most devices don't support visible background users), check for
+        // current user and profile first. Current user is always visible on:
         // - Default display
         // - Secondary displays when device doesn't support visible bg users
         //   - Or when explicitly added (which is checked below)
@@ -662,16 +725,28 @@ public final class UserVisibilityMediator implements Dumpable {
         }
 
         synchronized (mLock) {
-            if (mUsersAssignedToDisplayOnStart.get(userId, Display.INVALID_DISPLAY) == displayId) {
+            int profileGroupId;
-                // User assigned to display on start
+            synchronized (mLock) {
-                return true;
+                profileGroupId = mStartedVisibleProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID);
             }
-
+            if (isProfile(userId, profileGroupId)) {
-            // Check for extra display assignment
+                return isFullUserVisibleOnBackgroundLocked(profileGroupId, displayId);
-            return mExtraDisplaysAssignedToUsers.get(displayId, USER_NULL) == userId;
+            }
+            return isFullUserVisibleOnBackgroundLocked(userId, displayId);
         }
     }
 
+    // NOTE: it doesn't check if the userId is a full user, it's up to the caller to check that
+    @GuardedBy("mLock")
+    private boolean isFullUserVisibleOnBackgroundLocked(@UserIdInt int userId, int displayId) {
+        if (mUsersAssignedToDisplayOnStart.get(userId, Display.INVALID_DISPLAY) == displayId) {
+            // User assigned to display on start
+            return true;
+        }
+        // Check for extra display assignment
+        return mExtraDisplaysAssignedToUsers.get(displayId, USER_NULL) == userId;
+    }
+
     /**
      * See {@link UserManagerInternal#getDisplayAssignedToUser(int)}.
      */
@@ -737,7 +812,7 @@ public final class UserVisibilityMediator implements Dumpable {
                     continue;
                 }
                 int userId = mUsersAssignedToDisplayOnStart.keyAt(i);
-                if (!isStartedProfile(userId)) {
+                if (!isStartedVisibleProfileLocked(userId)) {
                     return userId;
                 } else if (DBG) {
                     Slogf.d(TAG, "getUserAssignedToDisplay(%d): skipping user %d because it's "
@@ -770,8 +845,8 @@ public final class UserVisibilityMediator implements Dumpable {
         // number of users is too small, the gain is probably not worth the increase on complexity.
         IntArray visibleUsers = new IntArray();
         synchronized (mLock) {
-            for (int i = 0; i < mStartedProfileGroupIds.size(); i++) {
+            for (int i = 0; i < mStartedVisibleProfileGroupIds.size(); i++) {
-                int userId = mStartedProfileGroupIds.keyAt(i);
+                int userId = mStartedVisibleProfileGroupIds.keyAt(i);
                 if (isUserVisible(userId)) {
                     visibleUsers.add(userId);
                 }
@@ -804,7 +879,7 @@ public final class UserVisibilityMediator implements Dumpable {
         }
     }
 
-    // TODO(b/242195409): remove this method if not needed anymore
+    // TODO(b/266158156): remove this method if not needed anymore
     /**
      * Nofify all listeners that the system user visibility changed.
      */
@@ -866,6 +941,9 @@ public final class UserVisibilityMediator implements Dumpable {
         ipw.println("UserVisibilityMediator");
         ipw.increaseIndent();
 
+        ipw.print("DBG: ");
+        ipw.println(DBG);
+
         synchronized (mLock) {
             ipw.print("Current user id: ");
             ipw.println(mCurrentUserId);
@@ -873,8 +951,12 @@ public final class UserVisibilityMediator implements Dumpable {
             ipw.print("Visible users: ");
             ipw.println(getVisibleUsers());
 
-            dumpSparseIntArray(ipw, mStartedProfileGroupIds, "started user / profile group",
+            dumpSparseIntArray(ipw, mStartedVisibleProfileGroupIds,
-                    "u", "pg");
+                    "started visible user / profile group", "u", "pg");
+            if (mStartedInvisibleProfileUserIds != null) {
+                ipw.print("Profiles started invisible: ");
+                ipw.println(mStartedInvisibleProfileUserIds);
+            }
 
             ipw.print("Supports visible background users on displays: ");
             ipw.println(mVisibleBackgroundUsersEnabled);
@@ -982,22 +1064,25 @@ public final class UserVisibilityMediator implements Dumpable {
             if (mCurrentUserId == userId) {
                 return true;
             }
-            return mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID) == mCurrentUserId;
+            return mStartedVisibleProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID)
+                    == mCurrentUserId;
         }
     }
 
-    private boolean isStartedProfile(@UserIdInt int userId) {
+    @GuardedBy("mLock")
-        int profileGroupId;
+    private boolean isStartedVisibleProfileLocked(@UserIdInt int userId) {
-        synchronized (mLock) {
+        int profileGroupId = mStartedVisibleProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID);
-            profileGroupId = mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID);
-        }
         return isProfile(userId, profileGroupId);
     }
 
-    private @UserIdInt int getStartedProfileGroupId(@UserIdInt int userId) {
+    private void validateUserStartMode(@UserStartMode int userStartMode) {
-        synchronized (mLock) {
+        switch (userStartMode) {
-            return mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID);
+            case USER_START_MODE_FOREGROUND:
+            case USER_START_MODE_BACKGROUND:
+            case USER_START_MODE_BACKGROUND_VISIBLE:
+                return;
         }
+        throw new IllegalArgumentException("Invalid user start mode: " + userStartMode);
     }
 
     private static String secondaryDisplayMappingStatusToString(

services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorMUPANDTest.java

@@ -20,6 +20,8 @@ import static android.view.Display.DEFAULT_DISPLAY;
 import static android.view.Display.INVALID_DISPLAY;
 
 import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_FAILURE;
+import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_ALREADY_VISIBLE;
+import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
 import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE;
 import static com.android.server.pm.UserVisibilityChangedEvent.onInvisible;
 import static com.android.server.pm.UserVisibilityChangedEvent.onVisible;
@@ -73,8 +75,8 @@ public final class UserVisibilityMediatorMUPANDTest
         assertUserCanBeAssignedExtraDisplay(USER_ID, OTHER_SECONDARY_DISPLAY_ID);
 
         // Make sure another user cannot be started on default display
-        int result2 = mMediator.assignUserToDisplayOnStart(otherUserId, visibleBgUserId,
+        int result2 = mMediator.assignUserToDisplayOnStart(otherUserId, otherUserId, BG_VISIBLE,
-                BG_VISIBLE, DEFAULT_DISPLAY);
+                DEFAULT_DISPLAY);
         assertStartUserResult(result2, USER_ASSIGNMENT_RESULT_FAILURE,
                 "when user (%d) is starting on default display after it was started by user %d",
                 otherUserId, visibleBgUserId);
@@ -117,8 +119,8 @@ public final class UserVisibilityMediatorMUPANDTest
         assertUserCanBeAssignedExtraDisplay(USER_ID, OTHER_SECONDARY_DISPLAY_ID);
 
         // Make sure another user cannot be started on default display
-        int result2 = mMediator.assignUserToDisplayOnStart(otherUserId, visibleBgUserId,
+        int result2 = mMediator.assignUserToDisplayOnStart(otherUserId, otherUserId, BG_VISIBLE,
-                BG_VISIBLE, DEFAULT_DISPLAY);
+                DEFAULT_DISPLAY);
         assertStartUserResult(result2, USER_ASSIGNMENT_RESULT_FAILURE,
                 "when user (%d) is starting on default display after it was started by user %d",
                 otherUserId, visibleBgUserId);
@@ -127,8 +129,6 @@ public final class UserVisibilityMediatorMUPANDTest
         listener.verify();
     }
 
-  /* TODO(b/261538337): re-add after the reverted CL is merged again
-
     @Test
     public void
        testStartVisibleBgProfile_onDefaultDisplay_whenParentIsStartedVisibleOnBgOnSecondaryDisplay()
@@ -226,5 +226,4 @@ public final class UserVisibilityMediatorMUPANDTest
 
         listener.verify();
     }
-  */
 }

services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorTestCase.java

@@ -44,7 +44,6 @@ import android.text.TextUtils;
 import android.util.IntArray;
 import android.util.Log;
 
-import com.android.internal.util.Preconditions;
 import com.android.server.DumpableDumperRule;
 import com.android.server.ExpectableTestCase;
 
@@ -151,6 +150,12 @@ abstract class UserVisibilityMediatorTestCase extends ExpectableTestCase {
                 .assignUserToDisplayOnStart(USER_CURRENT_OR_SELF, USER_ID, FG, DEFAULT_DISPLAY));
     }
 
+    @Test
+    public final void testAssignUserToDisplayOnStart_invalidUserStartMode() {
+        assertThrows(IllegalArgumentException.class, () -> mMediator
+                .assignUserToDisplayOnStart(USER_ID, USER_ID, 666, DEFAULT_DISPLAY));
+    }
+
     @Test
     public final void testStartFgUser_onSecondaryDisplay() throws Exception {
         AsyncUserVisibilityListener listener = addListenerForNoEvents();
@@ -286,7 +291,7 @@ abstract class UserVisibilityMediatorTestCase extends ExpectableTestCase {
 
         int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
                 BG_VISIBLE, DEFAULT_DISPLAY);
-        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_FAILURE);
 
         expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
         expectNoDisplayAssignedToUser(PROFILE_USER_ID);
@@ -302,14 +307,14 @@ abstract class UserVisibilityMediatorTestCase extends ExpectableTestCase {
 
         int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
                 BG_VISIBLE, DEFAULT_DISPLAY);
-        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_FAILURE);
 
         expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
 
         expectNoDisplayAssignedToUser(PROFILE_USER_ID);
         expectInitialCurrentUserAssignedToDisplay(DEFAULT_DISPLAY);
 
-        assertUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+        assertInvisibleUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
 
         listener.verify();
     }
@@ -334,6 +339,41 @@ abstract class UserVisibilityMediatorTestCase extends ExpectableTestCase {
         listener.verify();
     }
 
+    @Test
+    public final void testStartBgProfile_onDefaultDisplay_whenParentIsNotStarted()
+            throws Exception {
+        AsyncUserVisibilityListener listener = addListenerForNoEvents();
+
+        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID, BG,
+                DEFAULT_DISPLAY);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE);
+
+        expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
+        expectNoDisplayAssignedToUser(PROFILE_USER_ID);
+
+        listener.verify();
+    }
+
+    @Test
+    public final void testStartBgProfile_onDefaultDisplay_whenParentIsStartedOnBg()
+            throws Exception {
+        AsyncUserVisibilityListener listener = addListenerForNoEvents();
+        startBackgroundUser(PARENT_USER_ID);
+
+        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID, BG,
+                DEFAULT_DISPLAY);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE);
+
+        expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
+
+        expectNoDisplayAssignedToUser(PROFILE_USER_ID);
+        expectInitialCurrentUserAssignedToDisplay(DEFAULT_DISPLAY);
+
+        assertUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+
+        listener.verify();
+    }
+
     @Test
     public final void testStartBgProfile_onSecondaryDisplay() throws Exception {
         AsyncUserVisibilityListener listener = addListenerForNoEvents();
@@ -488,8 +528,6 @@ abstract class UserVisibilityMediatorTestCase extends ExpectableTestCase {
      * se.
      */
     protected final void startUserInSecondaryDisplay(@UserIdInt int userId, int displayId) {
-        Preconditions.checkArgument(displayId != INVALID_DISPLAY && displayId != DEFAULT_DISPLAY,
-                "must pass a secondary display, not %d", displayId);
         Log.d(TAG, "startUserInSecondaryDisplay(" + userId + ", " + displayId + ")");
         int result = mMediator.assignUserToDisplayOnStart(userId, userId, BG_VISIBLE, displayId);
         if (result != USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE) {

services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorVisibleBackgroundUserTestCase.java

@@ -108,34 +108,6 @@ abstract class UserVisibilityMediatorVisibleBackgroundUserTestCase
         listener.verify();
     }
 
-    @Test
-    public final void testStartVisibleBgProfile_onDefaultDisplay_whenParentIsCurrentUser()
-            throws Exception {
-        AsyncUserVisibilityListener listener = addListenerForEvents(
-                onInvisible(INITIAL_CURRENT_USER_ID),
-                onVisible(PARENT_USER_ID),
-                onVisible(PROFILE_USER_ID));
-        startForegroundUser(PARENT_USER_ID);
-
-        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
-                BG_VISIBLE, DEFAULT_DISPLAY);
-        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE);
-        expectUserCannotBeUnassignedFromDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY);
-
-        expectUserIsVisible(PROFILE_USER_ID);
-        expectUserIsNotVisibleOnDisplay(PROFILE_USER_ID, INVALID_DISPLAY);
-        expectUserIsNotVisibleOnDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
-        expectUserIsVisibleOnDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY);
-        expectVisibleUsers(PARENT_USER_ID, PROFILE_USER_ID);
-
-        expectDisplayAssignedToUser(PROFILE_USER_ID, DEFAULT_DISPLAY);
-        expectUserAssignedToDisplay(DEFAULT_DISPLAY, PARENT_USER_ID);
-
-        assertUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
-
-        listener.verify();
-    }
-
     @Test
     public final void testStartFgUser_onInvalidDisplay() throws Exception {
         AsyncUserVisibilityListener listener = addListenerForNoEvents();
@@ -300,15 +272,84 @@ abstract class UserVisibilityMediatorVisibleBackgroundUserTestCase
         listener2.verify();
     }
 
+    @Test
+    public final void testStartVisibleBgProfile_onDefaultDisplay_whenParentIsCurrentUser()
+            throws Exception {
+        AsyncUserVisibilityListener listener = addListenerForEvents(
+                onInvisible(INITIAL_CURRENT_USER_ID),
+                onVisible(PARENT_USER_ID),
+                onVisible(PROFILE_USER_ID));
+        startForegroundUser(PARENT_USER_ID);
+
+        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
+                BG_VISIBLE, DEFAULT_DISPLAY);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE);
+        expectUserCannotBeUnassignedFromDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY);
+
+        expectUserIsVisible(PROFILE_USER_ID);
+        expectUserIsNotVisibleOnDisplay(PROFILE_USER_ID, INVALID_DISPLAY);
+        expectUserIsNotVisibleOnDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+        expectUserIsVisibleOnDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY);
+        expectVisibleUsers(PARENT_USER_ID, PROFILE_USER_ID);
+
+        expectDisplayAssignedToUser(PROFILE_USER_ID, DEFAULT_DISPLAY);
+        expectUserAssignedToDisplay(DEFAULT_DISPLAY, PARENT_USER_ID);
+
+        assertUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+
+        listener.verify();
+    }
+
     @Test
     public final void
-            testStartVisibleBgProfile_onDefaultDisplay_whenParentVisibleOnSecondaryDisplay()
+            testStartVisibleBgProfile_onDefaultDisplay_whenParentIsStartedVisibleOnAnotherDisplay()
-                    throws Exception {
+            throws Exception {
         AsyncUserVisibilityListener listener = addListenerForEvents(onVisible(PARENT_USER_ID));
         startUserInSecondaryDisplay(PARENT_USER_ID, OTHER_SECONDARY_DISPLAY_ID);
 
         int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
                 BG_VISIBLE, DEFAULT_DISPLAY);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_FAILURE);
+
+        expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
+        expectNoDisplayAssignedToUser(PROFILE_USER_ID);
+        expectUserAssignedToDisplay(OTHER_SECONDARY_DISPLAY_ID, PARENT_USER_ID);
+
+        assertInvisibleUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+
+        listener.verify();
+    }
+
+    // Not supported - profiles can only be started on default display
+    @Test
+    public final void
+            testStartVisibleBgProfile_onSecondaryDisplay_whenParentIsStartedVisibleOnThatDisplay()
+            throws Exception {
+        AsyncUserVisibilityListener listener = addListenerForEvents(onVisible(PARENT_USER_ID));
+        startUserInSecondaryDisplay(PARENT_USER_ID, OTHER_SECONDARY_DISPLAY_ID);
+
+        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID,
+                BG_VISIBLE, DEFAULT_DISPLAY);
+        assertStartUserResult(result, USER_ASSIGNMENT_RESULT_FAILURE);
+
+        expectUserIsNotVisibleAtAll(PROFILE_USER_ID);
+        expectNoDisplayAssignedToUser(PROFILE_USER_ID);
+        expectUserAssignedToDisplay(OTHER_SECONDARY_DISPLAY_ID, PARENT_USER_ID);
+
+        assertInvisibleUserCannotBeAssignedExtraDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
+
+        listener.verify();
+    }
+
+    @Test
+    public final void
+            testStartProfile_onDefaultDisplay_whenParentIsStartedVisibleOnSecondaryDisplay()
+            throws Exception {
+        AsyncUserVisibilityListener listener = addListenerForEvents(onVisible(PARENT_USER_ID));
+        startUserInSecondaryDisplay(PARENT_USER_ID, OTHER_SECONDARY_DISPLAY_ID);
+
+        int result = mMediator.assignUserToDisplayOnStart(PROFILE_USER_ID, PARENT_USER_ID, BG,
+                DEFAULT_DISPLAY);
         assertStartUserResult(result, USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE);
 
         expectUserIsNotVisibleAtAll(PROFILE_USER_ID);