diff --git a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java index 7946cb74f1174..0b94798b170e5 100644 --- a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java +++ b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java @@ -906,7 +906,8 @@ final class ActivityManagerShellCommand extends ShellCommand { } } - // TODO(b/239982558): might need to support --displayId as well + // NOTE: current profiles can only be started on default display (even on automotive builds with + // passenger displays), so there's no need to pass a display-id private int runProfile(PrintWriter pw) throws RemoteException { final PrintWriter err = getErrPrintWriter(); String profileFile = null; diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index af0bd633c4710..3f8333a7a04fd 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -1073,9 +1073,6 @@ class UserController implements Handler.Callback { uss.setState(UserState.STATE_STOPPING); 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.unassignUserFromDisplayOnStop(userId); updateStartedUserArrayLU(); @@ -1505,13 +1502,32 @@ class UserController implements Handler.Callback { return startUserNoChecks(userId, Display.DEFAULT_DISPLAY, foreground, unlockListener); } - // TODO(b/239982558): add javadoc (need to wait until the intents / SystemService callbacks are - // defined + /** + * Starts a user in background and make it visible in the given display. + * + *

This call will trigger the usual "user started" lifecycle events (i.e., `SystemService` + * callbacks and app intents), plus a call to + * {@link UserManagerInternal.UserVisibilityListener#onUserVisibilityChanged(int, boolean)} if + * the user visibility changed. Notice that the visibility change is independent of the user + * workflow state, and they can mismatch in some corner events (for example, if the user was + * already running in the background but not associated with a display, this call for that user + * would not trigger any lifecycle event but would trigger {@code onUserVisibilityChanged}). + * + *

See {@link ActivityManager#startUserInBackgroundOnSecondaryDisplay(int, int)} for more + * semantics. + * + * @param userId user to be started + * @param displayId display where the user will be visible + * + * @return whether the user was started + */ boolean startUserOnSecondaryDisplay(@UserIdInt int userId, int displayId) { checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay", MANAGE_USERS, INTERACT_ACROSS_USERS); // DEFAULT_DISPLAY is used for the current foreground user only + // TODO(b/245939659): might need to move this check to UserVisibilityMediator to support + // passenger-only screens Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY, "Cannot use DEFAULT_DISPLAY"); @@ -1519,7 +1535,7 @@ class UserController implements Handler.Callback { return startUserNoChecks(userId, displayId, /* foreground= */ false, /* unlockListener= */ null); } catch (RuntimeException e) { - Slogf.w(TAG, "startUserOnSecondaryDisplay(%d, %d) failed: %s", userId, displayId, e); + Slogf.e(TAG, "startUserOnSecondaryDisplay(%d, %d) failed: %s", userId, displayId, e); return false; } } @@ -1618,7 +1634,6 @@ class UserController implements Handler.Callback { return false; } - // TODO(b/239982558): might need something similar for bg users on secondary display if (foreground && isUserSwitchUiEnabled()) { t.traceBegin("startFreezingScreen"); mInjector.getWindowManager().startFreezingScreen( @@ -1674,9 +1689,9 @@ class UserController implements Handler.Callback { userSwitchUiEnabled = mUserSwitchUiEnabled; } mInjector.updateUserConfiguration(); - // TODO(b/244644281): updateProfileRelatedCaches() is called on both if and else - // parts, ideally it should be moved outside, but for now it's not as there are many - // calls to external components here afterwards + // NOTE: updateProfileRelatedCaches() is called on both if and else parts, ideally + // it should be moved outside, but for now it's not as there are many calls to + // external components here afterwards updateProfileRelatedCaches(); mInjector.getWindowManager().setCurrentUser(userId); mInjector.reportCurWakefulnessUsageEvent(); diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java deleted file mode 100644 index 6c85b7ab49855..0000000000000 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java +++ /dev/null @@ -1,213 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.android.server.pm; - -import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; -import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; - -import static com.google.common.truth.Truth.assertWithMessage; - -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; - -import android.annotation.UserIdInt; -import android.app.ActivityManagerInternal; -import android.content.Context; -import android.content.pm.UserInfo; -import android.os.UserManager; -import android.util.Log; -import android.util.SparseArray; - -import androidx.test.annotation.UiThreadTest; - -import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder; -import com.android.server.ExtendedMockitoTestCase; -import com.android.server.LocalServices; -import com.android.server.am.UserState; -import com.android.server.pm.UserManagerService.UserData; - -import org.junit.After; -import org.junit.Before; -import org.mockito.Mock; - -/** - * Base class for {@link UserManagerInternalTest} and {@link UserManagerInternalTest}. - * - *

{@link UserManagerService} and its {@link UserManagerInternal} implementation have a - * "symbiotic relationship - some methods of the former simply call the latter and vice versa. - * - *

Ideally, only one of them should have the logic, but since that's not the case, this class - * provides the infra to make it easier to test both (which in turn would make it easier / safer to - * refactor their logic later). - */ -// TODO(b/244644281): there is no UserManagerInternalTest anymore as the logic being tested there -// moved to UserVisibilityController, so it might be simpler to merge this class into -// UserManagerServiceTest (once the UserVisibilityController -> UserManagerService dependency is -// fixed) -abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestCase { - - private static final String TAG = UserManagerServiceOrInternalTestCase.class.getSimpleName(); - - /** - * Id for a simple user (that doesn't have profiles). - */ - protected static final int USER_ID = 600; - - /** - * Id for another simple user. - */ - protected static final int OTHER_USER_ID = 666; - - /** - * Id for a user that has one profile (whose id is {@link #PROFILE_USER_ID}. - * - *

You can use {@link #addDefaultProfileAndParent()} to add both of this user to the service. - */ - protected static final int PARENT_USER_ID = 642; - - /** - * Id for a profile whose parent is {@link #PARENTUSER_ID}. - * - *

You can use {@link #addDefaultProfileAndParent()} to add both of this user to the service. - */ - protected static final int PROFILE_USER_ID = 643; - - private final Object mPackagesLock = new Object(); - private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation() - .getTargetContext(); - private final SparseArray mUsers = new SparseArray<>(); - - private Context mSpiedContext; - - private @Mock PackageManagerService mMockPms; - private @Mock UserDataPreparer mMockUserDataPreparer; - private @Mock ActivityManagerInternal mActivityManagerInternal; - - /** - * Reference to the {@link UserManagerService} being tested. - */ - protected UserManagerService mUms; - - /** - * Reference to the {@link UserManagerInternal} being tested. - */ - protected UserManagerInternal mUmi; - - @Override - protected void initializeSession(StaticMockitoSessionBuilder builder) { - builder - .spyStatic(UserManager.class) - .spyStatic(LocalServices.class); - } - - @Before - @UiThreadTest // Needed to initialize main handler - public final void setFixtures() { - mSpiedContext = spy(mRealContext); - - // Called when WatchedUserStates is constructed - doNothing().when(() -> UserManager.invalidateIsUserUnlockedCache()); - - // Must construct UserManagerService in the UiThread - mUms = new UserManagerService(mSpiedContext, mMockPms, mMockUserDataPreparer, - mPackagesLock, mRealContext.getDataDir(), mUsers); - mUmi = LocalServices.getService(UserManagerInternal.class); - assertWithMessage("LocalServices.getService(UserManagerInternal.class)").that(mUmi) - .isNotNull(); - } - - @After - public final void resetUserManagerInternal() { - // LocalServices follows the "Highlander rule" - There can be only one! - LocalServices.removeServiceForTest(UserManagerInternal.class); - } - - /////////////////////////////////////////// - // Helper methods exposed to sub-classes // - /////////////////////////////////////////// - - protected final void mockCurrentUser(@UserIdInt int userId) { - mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal); - - when(mActivityManagerInternal.getCurrentUserId()).thenReturn(userId); - } - - protected final void mockGetLocalService(Class serviceClass, T service) { - doReturn(service).when(() -> LocalServices.getService(serviceClass)); - } - - protected final void addDefaultProfileAndParent() { - addUser(PARENT_USER_ID); - addProfile(PROFILE_USER_ID, PARENT_USER_ID); - } - - 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; - - addUserData(profileData); - } - - protected final void addUser(@UserIdInt int userId) { - TestUserData userData = new TestUserData(userId); - - addUserData(userData); - } - - protected final void startDefaultProfile() { - startUser(PROFILE_USER_ID); - } - - protected final void stopDefaultProfile() { - stopUser(PROFILE_USER_ID); - } - - protected final void startUser(@UserIdInt int userId) { - setUserState(userId, UserState.STATE_RUNNING_UNLOCKED); - } - - protected final void stopUser(@UserIdInt int userId) { - setUserState(userId, UserState.STATE_STOPPING); - } - - protected final void setUserState(@UserIdInt int userId, int userState) { - mUmi.setUserState(userId, userState); - } - - /////////////////// - // Private infra // - /////////////////// - - private void addUserData(TestUserData userData) { - Log.d(TAG, "Adding " + userData); - mUsers.put(userData.info.id, userData); - } - - private static final class TestUserData extends UserData { - - @SuppressWarnings("deprecation") - TestUserData(@UserIdInt int userId) { - info = new UserInfo(); - info.id = userId; - } - - @Override - public String toString() { - return "TestUserData[" + info.toFullString() + "]"; - } - } -} diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java index b5ffe5f243381..1367af8eede33 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java @@ -15,17 +15,116 @@ */ package com.android.server.pm; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; + import static com.google.common.truth.Truth.assertWithMessage; -import android.app.ActivityManagerInternal; -import android.os.UserHandle; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import android.annotation.UserIdInt; +import android.app.ActivityManagerInternal; +import android.content.Context; +import android.content.pm.UserInfo; +import android.os.UserHandle; +import android.os.UserManager; +import android.util.Log; +import android.util.SparseArray; + +import androidx.test.annotation.UiThreadTest; + +import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder; +import com.android.server.ExtendedMockitoTestCase; +import com.android.server.LocalServices; +import com.android.server.am.UserState; +import com.android.server.pm.UserManagerService.UserData; + +import org.junit.After; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; /** * Run as {@code atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest} */ -public final class UserManagerServiceTest extends UserManagerServiceOrInternalTestCase { +public final class UserManagerServiceTest extends ExtendedMockitoTestCase { + + private static final String TAG = UserManagerServiceTest.class.getSimpleName(); + + /** + * Id for a simple user (that doesn't have profiles). + */ + private static final int USER_ID = 600; + + /** + * Id for another simple user. + */ + private static final int OTHER_USER_ID = 666; + + /** + * Id for a user that has one profile (whose id is {@link #PROFILE_USER_ID}. + * + *

You can use {@link #addDefaultProfileAndParent()} to add both of this user to the service. + */ + private static final int PARENT_USER_ID = 642; + + /** + * Id for a profile whose parent is {@link #PARENTUSER_ID}. + * + *

You can use {@link #addDefaultProfileAndParent()} to add both of this user to the service. + */ + private static final int PROFILE_USER_ID = 643; + + private final Object mPackagesLock = new Object(); + private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation() + .getTargetContext(); + private final SparseArray mUsers = new SparseArray<>(); + + private Context mSpiedContext; + + private @Mock PackageManagerService mMockPms; + private @Mock UserDataPreparer mMockUserDataPreparer; + private @Mock ActivityManagerInternal mActivityManagerInternal; + + /** + * Reference to the {@link UserManagerService} being tested. + */ + private UserManagerService mUms; + + /** + * Reference to the {@link UserManagerInternal} being tested. + */ + private UserManagerInternal mUmi; + + @Override + protected void initializeSession(StaticMockitoSessionBuilder builder) { + builder + .spyStatic(UserManager.class) + .spyStatic(LocalServices.class); + } + + @Before + @UiThreadTest // Needed to initialize main handler + public void setFixtures() { + mSpiedContext = spy(mRealContext); + + // Called when WatchedUserStates is constructed + doNothing().when(() -> UserManager.invalidateIsUserUnlockedCache()); + + // Must construct UserManagerService in the UiThread + mUms = new UserManagerService(mSpiedContext, mMockPms, mMockUserDataPreparer, + mPackagesLock, mRealContext.getDataDir(), mUsers); + mUmi = LocalServices.getService(UserManagerInternal.class); + assertWithMessage("LocalServices.getService(UserManagerInternal.class)").that(mUmi) + .isNotNull(); + } + + @After + public void resetUserManagerInternal() { + // LocalServices follows the "Highlander rule" - There can be only one! + LocalServices.removeServiceForTest(UserManagerInternal.class); + } @Test public void testGetCurrentUserId_amInternalNotReady() { @@ -123,4 +222,72 @@ public final class UserManagerServiceTest extends UserManagerServiceOrInternalTe assertWithMessage("isUserRunning(%s)", PROFILE_USER_ID) .that(mUms.isUserRunning(PROFILE_USER_ID)).isFalse(); } + + private void mockCurrentUser(@UserIdInt int userId) { + mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal); + + when(mActivityManagerInternal.getCurrentUserId()).thenReturn(userId); + } + + private void mockGetLocalService(Class serviceClass, T service) { + doReturn(service).when(() -> LocalServices.getService(serviceClass)); + } + + private void addDefaultProfileAndParent() { + addUser(PARENT_USER_ID); + addProfile(PROFILE_USER_ID, PARENT_USER_ID); + } + + private void addProfile(@UserIdInt int profileId, @UserIdInt int parentId) { + TestUserData profileData = new TestUserData(profileId); + profileData.info.flags = UserInfo.FLAG_PROFILE; + profileData.info.profileGroupId = parentId; + + addUserData(profileData); + } + + private void addUser(@UserIdInt int userId) { + TestUserData userData = new TestUserData(userId); + + addUserData(userData); + } + + private void startDefaultProfile() { + startUser(PROFILE_USER_ID); + } + + private void stopDefaultProfile() { + stopUser(PROFILE_USER_ID); + } + + private void startUser(@UserIdInt int userId) { + setUserState(userId, UserState.STATE_RUNNING_UNLOCKED); + } + + private void stopUser(@UserIdInt int userId) { + setUserState(userId, UserState.STATE_STOPPING); + } + + private void setUserState(@UserIdInt int userId, int userState) { + mUmi.setUserState(userId, userState); + } + + private void addUserData(TestUserData userData) { + Log.d(TAG, "Adding " + userData); + mUsers.put(userData.info.id, userData); + } + + private static final class TestUserData extends UserData { + + @SuppressWarnings("deprecation") + TestUserData(@UserIdInt int userId) { + info = new UserInfo(); + info.id = userId; + } + + @Override + public String toString() { + return "TestUserData[" + info.toFullString() + "]"; + } + } } diff --git a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java index 537d04fa35e9a..6e446f04b2fa7 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -255,7 +255,6 @@ public class UserControllerTest { .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());