diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index a1f3dae39dcb8..043da55b55cb6 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -1036,7 +1036,13 @@ class UserController implements Handler.Callback { if (uss.state != UserState.STATE_STOPPING && uss.state != UserState.STATE_SHUTDOWN) { uss.setState(UserState.STATE_STOPPING); - mInjector.getUserManagerInternal().setUserState(userId, uss.state); + UserManagerInternal userManagerInternal = mInjector.getUserManagerInternal(); + userManagerInternal.setUserState(userId, uss.state); + // TODO(b/239982558): for now we're just updating the user's visibility, but most likely + // we'll need to remove this call and handle that as part of the user state workflow + // instead. + userManagerInternal.unassignUserFromDisplay(userId); + updateStartedUserArrayLU(); final boolean allowDelayedLockingCopied = allowDelayedLocking; @@ -1076,12 +1082,6 @@ class UserController implements Handler.Callback { Binder.getCallingPid(), UserHandle.USER_ALL); }); } - - // TODO(b/239982558): for now we're just updating the user's visibility, but most likely - // we'll need to remove this call and handle that as part of the user state workflow - // instead. - // TODO(b/240613396) also check if multi-display is supported - mInjector.getUserManagerInternal().assignUserToDisplay(userId, Display.INVALID_DISPLAY); } private void finishUserStopping(final int userId, final UserState uss, @@ -1391,7 +1391,6 @@ class UserController implements Handler.Callback { && !user.isQuietModeEnabled(); } - // TODO(b/239982558): might need to infer the display id based on parent user /** * Starts a user only if it's a profile, with a more relaxed permission requirement: * {@link android.Manifest.permission#MANAGE_USERS} or @@ -1420,6 +1419,7 @@ class UserController implements Handler.Callback { return false; } + // TODO(b/239982558): pass proper displayId return startUserNoChecks(userId, Display.DEFAULT_DISPLAY, /* foreground= */ false, /* unlockListener= */ null); } @@ -1477,7 +1477,7 @@ class UserController implements Handler.Callback { checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay", MANAGE_USERS, CREATE_USERS); - // DEFAULT_DISPLAY is used for "regular" start user operations + // DEFAULT_DISPLAY is used for the current foreground user only Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY, "Cannot use DEFAULT_DISPLAY"); @@ -1510,27 +1510,13 @@ class UserController implements Handler.Callback { foreground ? " in foreground" : ""); } - // TODO(b/239982558): move logic below to a different class (like DisplayAssignmentManager) if (displayId != Display.DEFAULT_DISPLAY) { - // This is called by startUserOnSecondaryDisplay() - if (!UserManager.isUsersOnSecondaryDisplaysEnabled()) { - // TODO(b/239824814): add CTS test and/or unit test for all exceptional cases - throw new UnsupportedOperationException("Not supported by device"); - } - - // 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 start system user" - + " on secondary display (%d)", displayId); Preconditions.checkArgument(!foreground, "Cannot start user %d in foreground AND " + "on secondary display (%d)", userId, displayId); - - // TODO(b/239982558): for now we're just updating the user's visibility, but most likely - // we'll need to remove this call and handle that as part of the user state workflow - // instead. - mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId); } + mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId); + // TODO(b/239982558): log display id (or use a new event) EventLog.writeEvent(EventLogTags.UC_START_USER_INTERNAL, userId); final int callingUid = Binder.getCallingUid(); diff --git a/services/core/java/com/android/server/pm/UserManagerInternal.java b/services/core/java/com/android/server/pm/UserManagerInternal.java index 8f89fdd40b32c..abc0ee4c89f64 100644 --- a/services/core/java/com/android/server/pm/UserManagerInternal.java +++ b/services/core/java/com/android/server/pm/UserManagerInternal.java @@ -313,9 +313,22 @@ public abstract class UserManagerInternal { */ public abstract @Nullable UserProperties getUserProperties(@UserIdInt int userId); - /** TODO(b/239982558): add javadoc / mention invalid_id is used to unassing */ + /** + * Assigns a user to a display. + * + *

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). + */ public abstract void assignUserToDisplay(@UserIdInt int userId, int displayId); + /** + * Unassigns a user from its current display. + * + *

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). + */ + public abstract void unassignUserFromDisplay(@UserIdInt int userId); + /** * Returns {@code true} if the user is visible (as defined by * {@link UserManager#isUserVisible()} in the given display. diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 3b898f8760722..ab40a4fff1ffd 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -173,6 +173,8 @@ public class UserManagerService extends IUserManager.Stub { private static final String LOG_TAG = "UserManagerService"; static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE + // For Multiple Users on Multiple Displays + static final boolean DBG_MUMD = false; // DO NOT SUBMIT WITH TRUE private static final boolean DBG_WITH_STACKTRACE = false; // DO NOT SUBMIT WITH TRUE // Can be used for manual testing of id recycling private static final boolean RELEASE_DELETED_USER_ID = false; // DO NOT SUBMIT WITH TRUE @@ -624,15 +626,16 @@ public class UserManagerService extends IUserManager.Stub { @GuardedBy("mUserStates") private final WatchedUserStates mUserStates = new WatchedUserStates(); + + /** - * Used on devices that support background users (key) running on secondary displays (value). - * - *

Is {@code null} by default and instantiated on demand when the users are started on - * secondary displays. + * Set on on devices that support background users (key) running on secondary displays (value). */ + // TODO(b/239982558): move such logic to a different class (like UserDisplayAssigner) @Nullable - @GuardedBy("mUsersLock") - private SparseIntArray mUsersOnSecondaryDisplays; + @GuardedBy("mUsersOnSecondaryDisplays") + private final SparseIntArray mUsersOnSecondaryDisplays; + private final boolean mUsersOnSecondaryDisplaysEnabled; private static UserManagerService sInstance; @@ -750,6 +753,8 @@ public class UserManagerService extends IUserManager.Stub { mUserStates.put(UserHandle.USER_SYSTEM, UserState.STATE_BOOTING); mUser0Allocations = DBG_ALLOCATION ? new AtomicInteger() : null; emulateSystemUserModeIfNeeded(); + mUsersOnSecondaryDisplaysEnabled = UserManager.isUsersOnSecondaryDisplaysEnabled(); + mUsersOnSecondaryDisplays = mUsersOnSecondaryDisplaysEnabled ? new SparseIntArray() : null; } void systemReady() { @@ -1720,15 +1725,15 @@ public class UserManagerService extends IUserManager.Stub { return true; } - // TODO(b/239824814): STOPSHIP - add CTS tests (requires change on testing infra) - synchronized (mUsersLock) { - if (mUsersOnSecondaryDisplays != null) { - // TODO(b/239824814): make sure it handles profile as well - return (mUsersOnSecondaryDisplays.indexOfKey(userId) >= 0); - } + // Device doesn't support multiple users on multiple displays, so only users checked above + // can be visible + if (!mUsersOnSecondaryDisplaysEnabled) { + return false; } - return false; + synchronized (mUsersOnSecondaryDisplays) { + return mUsersOnSecondaryDisplays.indexOfKey(userId) >= 0; + } } // TODO(b/239982558): add unit test @@ -1736,10 +1741,13 @@ public class UserManagerService extends IUserManager.Stub { if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) { return Display.DEFAULT_DISPLAY; } - synchronized (mUsersLock) { - return mUsersOnSecondaryDisplays == null - ? Display.INVALID_DISPLAY - : mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY); + + if (!mUsersOnSecondaryDisplaysEnabled) { + return Display.INVALID_DISPLAY; + } + + synchronized (mUsersOnSecondaryDisplays) { + return mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY); } } @@ -1760,18 +1768,20 @@ public class UserManagerService extends IUserManager.Stub { return false; } - // TODO(b/239982558): currently used just by shell command, might need to move to - // UserManagerInternal if needed by other components (like WindowManagerService) - // TODO(b/239982558): add unit test - // TODO(b/239982558): try to merge with isUserVisibleUnchecked() + // TODO(b/239982558): try to merge with isUserVisibleUnchecked() (once both are unit tested) boolean isUserVisibleOnDisplay(@UserIdInt int userId, int displayId) { if (displayId == Display.DEFAULT_DISPLAY) { return isCurrentUserOrRunningProfileOfCurrentUser(userId); } - synchronized (mUsersLock) { - // TODO(b/239824814): make sure it handles profile as well - return mUsersOnSecondaryDisplays != null && mUsersOnSecondaryDisplays.get(userId, - Display.INVALID_DISPLAY) == displayId; + + // Device doesn't support multiple users on multiple displays, so only users checked above + // can be visible + if (!mUsersOnSecondaryDisplaysEnabled) { + return false; + } + + synchronized (mUsersOnSecondaryDisplays) { + return mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY) == displayId; } } @@ -6054,11 +6064,14 @@ public class UserManagerService extends IUserManager.Stub { } // synchronized (mPackagesLock) // Multiple Users on Multiple Display info - pw.println(" Supports users on secondary displays: " + pw.println(" Supports users on secondary displays: " + mUsersOnSecondaryDisplaysEnabled); + // mUsersOnSecondaryDisplaysEnabled is set on constructor, while the UserManager API is + // set dynamically, so print both to help cases where the developer changed it on the fly + pw.println(" UM.isUsersOnSecondaryDisplaysEnabled(): " + UserManager.isUsersOnSecondaryDisplaysEnabled()); - synchronized (mUsersLock) { - if (mUsersOnSecondaryDisplays != null) { - pw.print(" Users on secondary displays: "); + if (mUsersOnSecondaryDisplaysEnabled) { + pw.print(" Users on secondary displays: "); + synchronized (mUsersOnSecondaryDisplays) { pw.println(mUsersOnSecondaryDisplays); } } @@ -6628,44 +6641,32 @@ public class UserManagerService extends IUserManager.Stub { @Override public void assignUserToDisplay(int userId, int displayId) { - if (DBG) { - Slogf.d(LOG_TAG, "assignUserToDisplay(%d, %d): mUsersOnSecondaryDisplays=%s", - userId, displayId, mUsersOnSecondaryDisplays); + if (DBG_MUMD) { + Slogf.d(LOG_TAG, "assignUserToDisplay(%d, %d)", userId, displayId); } - // TODO(b/240613396) throw exception if feature not supported - if (displayId == Display.INVALID_DISPLAY) { - synchronized (mUsersLock) { - if (mUsersOnSecondaryDisplays == null) { - if (false) { - // TODO(b/240613396): remove this if once we check for support above - Slogf.wtf(LOG_TAG, "assignUserToDisplay(%d, %d): no " - + "mUsersOnSecondaryDisplays", userId, displayId); - } - return; - } - if (DBG) { - Slogf.d(LOG_TAG, "Removing %d from mUsersOnSecondaryDisplays", userId); - } - mUsersOnSecondaryDisplays.delete(userId); - if (mUsersOnSecondaryDisplays.size() == 0) { - if (DBG) { - Slogf.d(LOG_TAG, "Removing mUsersOnSecondaryDisplays"); - } - mUsersOnSecondaryDisplays = null; - } + if (displayId == Display.DEFAULT_DISPLAY) { + // Don't need to do anything because methods (such as isUserVisible()) already know + // that the current user (and their profiles) is assigned to the default display. + if (DBG_MUMD) { + Slogf.d(LOG_TAG, "ignoring on default display"); } return; } - synchronized (mUsersLock) { - if (mUsersOnSecondaryDisplays == null) { - if (DBG) { - Slogf.d(LOG_TAG, "Creating mUsersOnSecondaryDisplays"); - } - mUsersOnSecondaryDisplays = new SparseIntArray(); - } - if (DBG) { + if (!mUsersOnSecondaryDisplaysEnabled) { + throw new UnsupportedOperationException("assignUserToDisplay(" + userId + ", " + + displayId + ") called on device that doesn't support multiple " + + "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); + + synchronized (mUsersOnSecondaryDisplays) { + if (DBG_MUMD) { Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays", userId, displayId); } @@ -6703,6 +6704,29 @@ public class UserManagerService extends IUserManager.Stub { } } + @Override + public void unassignUserFromDisplay(@UserIdInt int userId) { + if (DBG_MUMD) { + Slogf.d(LOG_TAG, "unassignUserFromDisplay(%d)", userId); + } + if (!mUsersOnSecondaryDisplaysEnabled) { + // Don't need to do anything because methods (such as isUserVisible()) already know + // that the current user (and their profiles) is assigned to the default display. + if (DBG_MUMD) { + Slogf.d(LOG_TAG, "ignoring when device doesn't support MUMD"); + } + return; + } + + synchronized (mUsersOnSecondaryDisplays) { + if (DBG_MUMD) { + Slogf.d(LOG_TAG, "Removing %d from mUsersOnSecondaryDisplays (%s)", userId, + mUsersOnSecondaryDisplays); + } + mUsersOnSecondaryDisplays.delete(userId); + } + } + @Override public boolean isUserVisible(int userId, int displayId) { return isUserVisibleOnDisplay(userId, displayId);