Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Changes user start to enforce profiles are assigned to the parent display."

Browse Source
This commit is contained in:
Felipe Leme
2022-09-14 00:10:28 +00:00
committed by Android (Google) Code Review
parent e4c0e42c12 f7e65e5882
commit cb867afc58
7 changed files with 125 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
services/core/java/com/android/server/am/ActivityManagerService.java
View File

@@ -16145,8 +16145,7 @@ public class ActivityManagerService extends IActivityManager.Stub
@Override
public boolean startUserInBackgroundOnSecondaryDisplay(int userId, int displayId) {
// Permission check done inside UserController.
return mUserController.startUserOnSecondaryDisplay(userId, displayId,
/* unlockListener= */ null);
return mUserController.startUserOnSecondaryDisplay(userId, displayId);
}
/**

19
services/core/java/com/android/server/am/UserController.java
View File

@@ -1375,8 +1375,9 @@ class UserController implements Handler.Callback {
}
final int profilesToStartSize = profilesToStart.size();
int i = 0;
// TODO(b/239982558): pass displayId
for (; i < profilesToStartSize && i < (getMaxRunningUsers() - 1); ++i) {
// NOTE: this method is setting the profiles of the current user - which is always
// assigned to the default display - so there's no need to pass PARENT_DISPLAY
startUser(profilesToStart.get(i).id, /* foreground= */ false);
}
if (i < profilesToStartSize) {
@@ -1419,8 +1420,10 @@ class UserController implements Handler.Callback {
return false;
}
// TODO(b/239982558): pass proper displayId
return startUserNoChecks(userId, Display.DEFAULT_DISPLAY, /* foreground= */ false,
int displayId = mInjector.isUsersOnSecondaryDisplaysEnabled()
? UserManagerInternal.PARENT_DISPLAY
: Display.DEFAULT_DISPLAY;
return startUserNoChecks(userId, displayId, /* foreground= */ false,
/* unlockListener= */ null);
}
@@ -1472,8 +1475,7 @@ class UserController implements Handler.Callback {
// TODO(b/239982558): add javadoc (need to wait until the intents / SystemService callbacks are
// defined
boolean startUserOnSecondaryDisplay(@UserIdInt int userId, int displayId,
@Nullable IProgressListener unlockListener) {
boolean startUserOnSecondaryDisplay(@UserIdInt int userId, int displayId) {
checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay",
MANAGE_USERS, CREATE_USERS);
@@ -1482,7 +1484,8 @@ class UserController implements Handler.Callback {
"Cannot use DEFAULT_DISPLAY");
try {
return startUserNoChecks(userId, displayId, /* foreground= */ false, unlockListener);
return startUserNoChecks(userId, displayId, /* foreground= */ false,
/* unlockListener= */ null);
} catch (RuntimeException e) {
Slogf.w(TAG, "startUserOnSecondaryDisplay(%d, %d) failed: %s", userId, displayId, e);
return false;
@@ -3484,5 +3487,9 @@ class UserController implements Handler.Callback {
}
}, reason);
}
boolean isUsersOnSecondaryDisplaysEnabled() {
return UserManager.isUsersOnSecondaryDisplaysEnabled();
}
}
}

9
services/core/java/com/android/server/pm/UserManagerInternal.java
View File

@@ -46,6 +46,15 @@ public abstract class UserManagerInternal {
public @interface OwnerType {
}
// TODO(b/245963156): move to Display.java (and @hide) if we decide to support profiles on MUMD
/**
* Used only when starting a profile (on systems that
* {@link android.os.UserManager#isUsersOnSecondaryDisplaysSupported() support users running on
* secondary displays}), to indicate the profile should be started in the same display as its
* parent user.
*/
public static final int PARENT_DISPLAY = -2;
public interface UserRestrictionsListener {
/**
* Called when a user restriction changes.

69
services/core/java/com/android/server/pm/UserManagerService.java
View File

@@ -6825,53 +6825,44 @@ public class UserManagerService extends IUserManager.Stub {
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);
"Cannot assign current user (%d) to other displays", currentUserId);
synchronized (mUsersOnSecondaryDisplays) {
if (DBG_MUMD) {
Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays",
userId, displayId);
}
if (isProfile) {
if (isProfileUnchecked(userId)) {
// Profile can only start in the same display as parent
Preconditions.checkArgument(displayId == UserManagerInternal.PARENT_DISPLAY,
"Profile user can only be started 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
if (DBG_MUMD) {
Slogf.d(LOG_TAG, "Adding profile user %d -> display %d", userId,
parentDisplayId);
}
mUsersOnSecondaryDisplays.put(userId, parentDisplayId);
return;
}
// 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
}
if (DBG_MUMD) {
Slogf.d(LOG_TAG, "Adding full user %d -> display %d", userId, displayId);
}
mUsersOnSecondaryDisplays.put(userId, displayId);
}
}

18
services/tests/mockingservicestests/src/com/android/server/pm/UserManagerInternalTest.java
View File

@@ -19,6 +19,8 @@ import static android.os.UserHandle.USER_SYSTEM;
import static android.view.Display.DEFAULT_DISPLAY;
import static android.view.Display.INVALID_DISPLAY;
import static com.android.server.pm.UserManagerInternal.PARENT_DISPLAY;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertThrows;
@@ -119,9 +121,6 @@ public final class UserManagerInternalTest extends UserManagerServiceOrInternalT
() -> 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();
}
@@ -136,10 +135,6 @@ public final class UserManagerInternalTest extends UserManagerServiceOrInternalT
() -> 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();
}
@@ -173,7 +168,7 @@ public final class UserManagerInternalTest extends UserManagerServiceOrInternalT
addDefaultProfileAndParent();
mUmi.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID);
mUmi.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);
mUmi.assignUserToDisplay(PROFILE_USER_ID, PARENT_DISPLAY);
assertUsersAssignedToDisplays(PARENT_USER_ID, SECONDARY_DISPLAY_ID,
pair(PROFILE_USER_ID, SECONDARY_DISPLAY_ID));
@@ -185,13 +180,10 @@ public final class UserManagerInternalTest extends UserManagerServiceOrInternalT
addDefaultProfileAndParent();
mUmi.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID);
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> mUmi.assignUserToDisplay(PROFILE_USER_ID, OTHER_SECONDARY_DISPLAY_ID));
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 + ".*" + OTHER_SECONDARY_DISPLAY_ID
+ ".*parent.*" + PARENT_USER_ID + ".*" + SECONDARY_DISPLAY_ID + ".*");
assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID);
}

8
services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java
View File

@@ -101,11 +101,6 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC
*/
protected static final int OTHER_SECONDARY_DISPLAY_ID = 108;
/**
* Id of another secondary display (i.e, not {@link android.view.Display.DEFAULT_DISPLAY}).
*/
private static final int ANOTHER_SECONDARY_DISPLAY_ID = 108;
private final Object mPackagesLock = new Object();
private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation()
.getTargetContext();
@@ -422,10 +417,9 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC
assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID);
assertWithMessage("isUserVisibleOnDisplay(%s, %s)", USER_ID, SECONDARY_DISPLAY_ID)
.that(isUserVisibleOnDisplay(USER_ID, ANOTHER_SECONDARY_DISPLAY_ID)).isFalse();
.that(isUserVisibleOnDisplay(USER_ID, OTHER_SECONDARY_DISPLAY_ID)).isFalse();
}
// NOTE: we don't need to add tests for profiles (started / stopped profiles of bg user), as
// isUserVisibleOnDisplay() for bg users relies only on the user / display assignments

70
services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
View File

@@ -48,6 +48,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Matchers.any;
@@ -64,7 +65,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.validateMockitoUsage;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertThrows;
import android.annotation.Nullable;
import android.annotation.UserIdInt;
@@ -90,6 +90,7 @@ import android.os.UserManager;
import android.os.storage.IStorageManager;
import android.platform.test.annotations.Presubmit;
import android.util.Log;
import android.view.Display;
import androidx.test.filters.SmallTest;
@@ -177,10 +178,11 @@ public class UserControllerTest {
doNothing().when(mInjector).activityManagerOnUserStopped(anyInt());
doNothing().when(mInjector).clearBroadcastQueueForUser(anyInt());
doNothing().when(mInjector).taskSupervisorRemoveUser(anyInt());
mockIsUsersOnSecondaryDisplaysEnabled(false);
// All UserController params are set to default.
mUserController = new UserController(mInjector);
setUpUser(TEST_USER_ID, NO_USERINFO_FLAGS);
setUpUser(TEST_PRE_CREATED_USER_ID, NO_USERINFO_FLAGS, /* preCreated=*/ true, null);
setUpUser(TEST_PRE_CREATED_USER_ID, NO_USERINFO_FLAGS, /* preCreated= */ true, null);
});
}
@@ -199,11 +201,37 @@ public class UserControllerTest {
verify(mInjector.getWindowManager()).setSwitchingUser(true);
verify(mInjector).clearAllLockedTasks(anyString());
startForegroundUserAssertions();
verifyUserAssignedToDisplay(TEST_USER_ID, Display.DEFAULT_DISPLAY);
}
@Test
public void testStartUser_background() {
mUserController.startUser(TEST_USER_ID, false /* foreground */);
boolean started = mUserController.startUser(TEST_USER_ID, /* foreground= */ false);
assertWithMessage("startUser(%s, foreground=false)", TEST_USER_ID).that(started).isTrue();
verify(mInjector.getWindowManager(), never()).startFreezingScreen(anyInt(), anyInt());
verify(mInjector.getWindowManager(), never()).setSwitchingUser(anyBoolean());
verify(mInjector, never()).clearAllLockedTasks(anyString());
startBackgroundUserAssertions();
verifyUserAssignedToDisplay(TEST_USER_ID, Display.DEFAULT_DISPLAY);
}
@Test
public void testStartUserOnSecondaryDisplay_defaultDisplay() {
assertThrows(IllegalArgumentException.class, () -> mUserController
.startUserOnSecondaryDisplay(TEST_USER_ID, Display.DEFAULT_DISPLAY));
verifyUserNeverAssignedToDisplay();
}
@Test
public void testStartUserOnSecondaryDisplay() {
boolean started = mUserController.startUserOnSecondaryDisplay(TEST_USER_ID, 42);
assertWithMessage("startUserOnSecondaryDisplay(%s, %s)", TEST_USER_ID, 42).that(started)
.isTrue();
verifyUserAssignedToDisplay(TEST_USER_ID, 42);
// TODO(b/239982558): might need to change assertions
verify(mInjector.getWindowManager(), never()).startFreezingScreen(anyInt(), anyInt());
verify(mInjector.getWindowManager(), never()).setSwitchingUser(anyBoolean());
verify(mInjector, never()).clearAllLockedTasks(anyString());
@@ -611,6 +639,7 @@ public class UserControllerTest {
setUpAndStartUserInBackground(TEST_USER_ID1);
assertThrows(IllegalArgumentException.class,
() -> mUserController.stopProfile(TEST_USER_ID1));
verifyUserUnassignedFromDisplayNeverCalled(TEST_USER_ID);
}
@Test
@@ -623,13 +652,26 @@ public class UserControllerTest {
@Test
public void testStartProfile() throws Exception {
setUpAndStartProfileInBackground(TEST_USER_ID1);
startBackgroundUserAssertions();
verifyUserAssignedToDisplay(TEST_USER_ID1, Display.DEFAULT_DISPLAY);
}
@Test
public void testStartProfile_whenUsersOnSecondaryDisplaysIsEnabled() throws Exception {
mockIsUsersOnSecondaryDisplaysEnabled(true);
setUpAndStartProfileInBackground(TEST_USER_ID1);
startBackgroundUserAssertions();
verifyUserAssignedToDisplay(TEST_USER_ID1, UserManagerInternal.PARENT_DISPLAY);
}
@Test
public void testStopProfile() throws Exception {
setUpAndStartProfileInBackground(TEST_USER_ID1);
assertProfileLockedOrUnlockedAfterStopping(TEST_USER_ID1, /* expectLocking= */ true);
verifyUserUnassignedFromDisplay(TEST_USER_ID1);
}
/** Tests handleIncomingUser() for a variety of permissions and situations. */
@@ -680,7 +722,7 @@ public class UserControllerTest {
eq(INTERACT_ACROSS_PROFILES), anyInt(), anyInt(), any())).thenReturn(false);
checkHandleIncomingUser(user1a.id, user2.id, ALLOW_NON_FULL, true);
checkHandleIncomingUser(user1a.id, user2.id, ALLOW_NON_FULL_IN_PROFILE, false);
checkHandleIncomingUser(user1a.id, user2.id, ALLOW_NON_FULL_IN_PROFILE, false);
checkHandleIncomingUser(user1a.id, user2.id, ALLOW_FULL_ONLY, false);
checkHandleIncomingUser(user1a.id, user2.id, ALLOW_PROFILES_OR_NON_FULL, true);
@@ -876,6 +918,26 @@ public class UserControllerTest {
}
}
private void mockIsUsersOnSecondaryDisplaysEnabled(boolean value) {
when(mInjector.isUsersOnSecondaryDisplaysEnabled()).thenReturn(value);
}
private void verifyUserAssignedToDisplay(@UserIdInt int userId, int displayId) {
verify(mInjector.getUserManagerInternal()).assignUserToDisplay(userId, displayId);
}
private void verifyUserNeverAssignedToDisplay() {
verify(mInjector.getUserManagerInternal(), never()).assignUserToDisplay(anyInt(), anyInt());
}
private void verifyUserUnassignedFromDisplay(@UserIdInt int userId) {
verify(mInjector.getUserManagerInternal()).unassignUserFromDisplay(userId);
}
private void verifyUserUnassignedFromDisplayNeverCalled(@UserIdInt int userId) {
verify(mInjector.getUserManagerInternal(), never()).unassignUserFromDisplay(userId);
}
// Should be public to allow mocking
private static class TestInjector extends UserController.Injector {
public final TestHandler mHandler;