Merge "Added more checks on startUserInBackgroundOnSecondaryDisplay():" (1131477f) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/am/UserController.java

+14 −4

Original line number	Diff line number	Diff line
		@@ -1468,7 +1468,16 @@ class UserController implements Handler.Callback {
		checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay",
		MANAGE_USERS, CREATE_USERS);

		// DEFAULT_DISPLAY is used for "regular" start user operations
		Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY,
		"Cannot use DEFAULT_DISPLAY");

		try {
		return startUserNoChecks(userId, displayId, /* foreground= */ false, unlockListener);
		} catch (RuntimeException e) {
		Slogf.w(TAG, "startUserOnSecondaryDisplay(%d, %d) failed: %s", userId, displayId, e);
		return false;
		}
		}

		private boolean startUserNoChecks(@UserIdInt int userId, int displayId, boolean foreground,
		@@ -1492,13 +1501,15 @@ 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
		// TODO(b/239824814): add CTS test and/or unit test for all exceptional cases
		throw new UnsupportedOperationException("Not supported by device");
		}

		// TODO(b/239982558): add unit test for the exceptional cases below
		// 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);
		@@ -1508,7 +1519,6 @@ class UserController implements Handler.Callback {
		// 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/239982558): check if display is valid
		mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId);
		}

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

+34 −3

Original line number	Diff line number	Diff line
		@@ -1587,6 +1587,10 @@ public class UserManagerService extends IUserManager.Stub {

		public boolean isProfile(@UserIdInt int userId) {
		checkQueryOrInteractPermissionIfCallerInOtherProfileGroup(userId, "isProfile");
		return isProfileUnchecked(userId);
		}

		private boolean isProfileUnchecked(@UserIdInt int userId) {
		synchronized (mUsersLock) {
		UserInfo userInfo = getUserInfoLU(userId);
		return userInfo != null && userInfo.isProfile();
		@@ -6827,9 +6831,6 @@ public class UserManagerService extends IUserManager.Stub {
		return;
		}

		// TODO(b/239982558) check for invalid cases like:
		// - userId already assigned to another display
		// - displayId already assigned to another user
		synchronized (mUsersLock) {
		if (mUsersOnSecondaryDisplays == null) {
		if (DBG) {
		@@ -6841,6 +6842,36 @@ public class UserManagerService extends IUserManager.Stub {
		Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays",
		userId, displayId);
		}

		if (isProfileUnchecked(userId)) {
		// Profile can only start in the same display as parent
		int parentUserId = getProfileParentId(userId);
		int parentDisplayId = mUsersOnSecondaryDisplays.get(parentUserId);
		if (displayId != parentDisplayId) {
		throw new IllegalStateException("Cannot assign profile " + userId + " to "
		+ "display " + displayId + " as its parent (user " + parentUserId
		+ ") is assigned to display " + parentDisplayId);
		}
		} else {
		// Check if display is available
		for (int i = 0; i < mUsersOnSecondaryDisplays.size(); i++) {
		// Make sure display is not used by other users...
		// TODO(b/240736142); currently, if a user was started in a display, it
		// would need to be stopped first, so "switching" a user on secondary
		// diplay requires 2 non-atomic operations (stop and start). Once this logic
		// 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 "
		+ "user " + mUsersOnSecondaryDisplays.keyAt(i));
		}
		// TODO(b/239982558) also check that user is not already assigned to other
		// display (including 0). That would be harder to tested under CTS though
		// (for example, would need to add a new AM method to start user in bg on
		// main display), so it's better to test on unit tests
		}
		}

		mUsersOnSecondaryDisplays.put(userId, displayId);
		}
		}