From a2c65f11be13758d1535e178843e1ad9e2ab7bdd Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Wed, 26 Oct 2022 13:23:34 -0700 Subject: [PATCH] UserVisibilityMediator refactoring, step 1. The initial version of UserVisibilityMediator was relying on UserManagerService to get the state of the current users (full and profiles), which caused a cyclic dependency between these 2 objects. This CL breaks that dependency by replicating such state into UserVisibilityMediator (and adding methods to start / stop users). There's still room for improvement (just look at the TODOs :-), but they'll be handled on future CLs, so this one minimizes the changes (both in the main code and unit tests). Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest UserVisibilityMediatorMUMDTest UserVisibilityMediatorSUSDTest UserControllerTest Test: atest CtsMultiUserTestCases:android.multiuser.cts.MultipleUsersOnMultipleDisplaysTest Test: adb shell dumpsys user --visibility-mediator Bug: 244644281 Change-Id: I8daa751dbc6fd4c38edd21ab9ef7b7c7ad13c8ff --- .../com/android/server/am/UserController.java | 10 +- .../server/pm/UserManagerInternal.java | 11 +- .../android/server/pm/UserManagerService.java | 14 +- .../server/pm/UserVisibilityMediator.java | 298 +++++++++++++----- .../android/server/DumpableDumperRule.java | 91 ++++++ .../server/ExtendedMockitoTestCase.java | 4 + .../pm/UserVisibilityMediatorMUMDTest.java | 70 ++-- .../pm/UserVisibilityMediatorSUSDTest.java | 12 +- .../pm/UserVisibilityMediatorTestCase.java | 246 ++++++++++++--- .../android/server/am/UserControllerTest.java | 8 +- 10 files changed, 579 insertions(+), 185 deletions(-) create mode 100644 services/tests/mockingservicestests/src/com/android/server/DumpableDumperRule.java diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index af559808d880f..562adad6227dc 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -1626,7 +1626,12 @@ class UserController implements Handler.Callback { return false; } - mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId); + if (!userInfo.preCreated) { + // TODO(b/244644281): UMI should return whether the user is visible. And if fails, + // the user should not be in the mediator's started users structure + mInjector.getUserManagerInternal().assignUserToDisplay(userId, + userInfo.profileGroupId, foreground, displayId); + } // TODO(b/239982558): might need something similar for bg users on secondary display if (foreground && isUserSwitchUiEnabled()) { @@ -1684,6 +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 updateProfileRelatedCaches(); mInjector.getWindowManager().setCurrentUser(userId); mInjector.reportCurWakefulnessUsageEvent(); diff --git a/services/core/java/com/android/server/pm/UserManagerInternal.java b/services/core/java/com/android/server/pm/UserManagerInternal.java index 56ec8e46291d9..04566d90cb70b 100644 --- a/services/core/java/com/android/server/pm/UserManagerInternal.java +++ b/services/core/java/com/android/server/pm/UserManagerInternal.java @@ -331,13 +331,18 @@ public abstract class UserManagerInternal { *

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). * + *

NOTE: this method is meant to be used only by {@code UserController} (when a user + * is started) + * *

NOTE: this method doesn't validate if the display exists, it's up to the caller to * check it. In fact, one of the intended clients for this method is * {@code DisplayManagerService}, which will call it when a virtual display is created (another * client is {@code UserController}, which will call it when a user is started). - * */ - public abstract void assignUserToDisplay(@UserIdInt int userId, int displayId); + // TODO(b/244644281): rename to assignUserToDisplayOnStart() and make sure it's called on boot + // as well + public abstract void assignUserToDisplay(@UserIdInt int userId, @UserIdInt int profileGroupId, + boolean foreground, int displayId); /** * Unassigns a user from its current display. @@ -346,7 +351,7 @@ public abstract class UserManagerInternal { * multiple users on multiple displays (like automotives with passenger displays). * *

NOTE: this method is meant to be used only by {@code UserController} (when a user - * is stopped) and {@code DisplayManagerService} (when a virtual display is destroyed). + * is stopped). */ public abstract void unassignUserFromDisplay(@UserIdInt int userId); diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index f85b11da252ea..aa71bb7498e17 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -626,7 +626,7 @@ public class UserManagerService extends IUserManager.Stub { @GuardedBy("mUserStates") private final WatchedUserStates mUserStates = new WatchedUserStates(); - private final UserVisibilityMediator mUserVisibilityMediator; + private final UserVisibilityMediator mUserVisibilityMediator = new UserVisibilityMediator(); private static UserManagerService sInstance; @@ -749,7 +749,6 @@ public class UserManagerService extends IUserManager.Stub { mUserStates.put(UserHandle.USER_SYSTEM, UserState.STATE_BOOTING); mUser0Allocations = DBG_ALLOCATION ? new AtomicInteger() : null; emulateSystemUserModeIfNeeded(); - mUserVisibilityMediator = new UserVisibilityMediator(this); } void systemReady() { @@ -6146,7 +6145,7 @@ public class UserManagerService extends IUserManager.Stub { dumpUser(pw, UserHandle.parseUserArg(args[1]), sb, now, nowRealtime); return; case "--visibility-mediator": - mUserVisibilityMediator.dump(pw); + mUserVisibilityMediator.dump(pw, args); return; } } @@ -6212,7 +6211,7 @@ public class UserManagerService extends IUserManager.Stub { } // synchronized (mPackagesLock) pw.println(); - mUserVisibilityMediator.dump(pw); + mUserVisibilityMediator.dump(pw, args); pw.println(); // Dump some capabilities @@ -6788,13 +6787,16 @@ public class UserManagerService extends IUserManager.Stub { } @Override - public void assignUserToDisplay(@UserIdInt int userId, int displayId) { - mUserVisibilityMediator.assignUserToDisplay(userId, displayId); + public void assignUserToDisplay(@UserIdInt int userId, @UserIdInt int profileGroupId, + boolean foreground, int displayId) { + mUserVisibilityMediator.startUser(userId, profileGroupId, foreground, displayId); + mUserVisibilityMediator.assignUserToDisplay(userId, profileGroupId, displayId); } @Override public void unassignUserFromDisplay(@UserIdInt int userId) { mUserVisibilityMediator.unassignUserFromDisplay(userId); + mUserVisibilityMediator.stopUser(userId); } @Override diff --git a/services/core/java/com/android/server/pm/UserVisibilityMediator.java b/services/core/java/com/android/server/pm/UserVisibilityMediator.java index f725c486ec1f2..0015fadc6042b 100644 --- a/services/core/java/com/android/server/pm/UserVisibilityMediator.java +++ b/services/core/java/com/android/server/pm/UserVisibilityMediator.java @@ -15,10 +15,18 @@ */ package com.android.server.pm; +import static android.content.pm.UserInfo.NO_PROFILE_GROUP_ID; +import static android.os.UserHandle.USER_CURRENT; +import static android.os.UserHandle.USER_NULL; +import static android.view.Display.DEFAULT_DISPLAY; + +import android.annotation.IntDef; import android.annotation.Nullable; import android.annotation.UserIdInt; import android.os.UserHandle; import android.os.UserManager; +import android.util.DebugUtils; +import android.util.Dumpable; import android.util.IndentingPrintWriter; import android.util.SparseIntArray; import android.view.Display; @@ -29,6 +37,8 @@ import com.android.internal.util.Preconditions; import com.android.server.utils.Slogf; import java.io.PrintWriter; +import java.util.LinkedHashMap; +import java.util.Map; /** * Class responsible for deciding whether a user is visible (or visible for a given display). @@ -36,72 +46,146 @@ import java.io.PrintWriter; *

This class is thread safe. */ // TODO(b/244644281): improve javadoc (for example, explain all cases / modes) -public final class UserVisibilityMediator { +public final class UserVisibilityMediator implements Dumpable { private static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE private static final String TAG = UserVisibilityMediator.class.getSimpleName(); - private final Object mLock = new Object(); + private static final String PREFIX_START_USER_RESULT = "START_USER_"; - // TODO(b/244644281): should not depend on service, but keep its own internal state (like - // current user and profile groups), but it is initially as the code was just moved from UMS - // "as is". Similarly, it shouldn't need to pass the SparseIntArray on constructor (which was - // added to UMS for testing purposes) - private final UserManagerService mService; + // NOTE: it's set as USER_CURRENT instead of USER_NULL because NO_PROFILE_GROUP_ID has the same + // falue of USER_NULL, which would complicate some checks (especially on unit tests) + @VisibleForTesting + static final int INITIAL_CURRENT_USER_ID = USER_CURRENT; + + public static final int START_USER_RESULT_SUCCESS_VISIBLE = 1; + public static final int START_USER_RESULT_SUCCESS_INVISIBLE = 2; + public static final int START_USER_RESULT_FAILURE = -1; + + @IntDef(flag = false, prefix = {PREFIX_START_USER_RESULT}, value = { + START_USER_RESULT_SUCCESS_VISIBLE, + START_USER_RESULT_SUCCESS_INVISIBLE, + START_USER_RESULT_FAILURE + }) + public @interface StartUserResult {} + + private final Object mLock = new Object(); private final boolean mUsersOnSecondaryDisplaysEnabled; + @UserIdInt + @GuardedBy("mLock") + private int mCurrentUserId = INITIAL_CURRENT_USER_ID; + @Nullable @GuardedBy("mLock") - private final SparseIntArray mUsersOnSecondaryDisplays; + private final SparseIntArray mUsersOnSecondaryDisplays = new SparseIntArray(); - UserVisibilityMediator(UserManagerService service) { - this(service, UserManager.isUsersOnSecondaryDisplaysEnabled(), - /* usersOnSecondaryDisplays= */ null); + /** + * Mapping from each started user to its profile group. + */ + @GuardedBy("mLock") + private final SparseIntArray mStartedProfileGroupIds = new SparseIntArray(); + + UserVisibilityMediator() { + this(UserManager.isUsersOnSecondaryDisplaysEnabled()); } @VisibleForTesting - UserVisibilityMediator(UserManagerService service, boolean usersOnSecondaryDisplaysEnabled, - @Nullable SparseIntArray usersOnSecondaryDisplays) { - mService = service; + UserVisibilityMediator(boolean usersOnSecondaryDisplaysEnabled) { mUsersOnSecondaryDisplaysEnabled = usersOnSecondaryDisplaysEnabled; - if (mUsersOnSecondaryDisplaysEnabled) { - mUsersOnSecondaryDisplays = usersOnSecondaryDisplays == null - ? new SparseIntArray() // default behavior - : usersOnSecondaryDisplays; // passed by unit test - } else { - mUsersOnSecondaryDisplays = null; + } + + /** + * TODO(b/244644281): merge with assignUserToDisplay() or add javadoc. + */ + public @StartUserResult int startUser(@UserIdInt int userId, @UserIdInt int profileGroupId, + boolean foreground, int displayId) { + int actualProfileGroupId = profileGroupId == NO_PROFILE_GROUP_ID + ? userId + : profileGroupId; + if (DBG) { + Slogf.d(TAG, "startUser(%d, %d, %b, %d): actualProfileGroupId=%d", + userId, profileGroupId, foreground, displayId, actualProfileGroupId); + } + if (foreground && displayId != DEFAULT_DISPLAY) { + Slogf.w(TAG, "startUser(%d, %d, %b, %d) failed: cannot start foreground user on " + + "secondary display", userId, actualProfileGroupId, foreground, displayId); + return START_USER_RESULT_FAILURE; + } + + int visibility; + synchronized (mLock) { + if (isProfile(userId, actualProfileGroupId)) { + if (displayId != DEFAULT_DISPLAY) { + Slogf.w(TAG, "startUser(%d, %d, %b, %d) failed: cannot start profile user on " + + "secondary display", userId, actualProfileGroupId, foreground, + displayId); + return START_USER_RESULT_FAILURE; + } + if (foreground) { + Slogf.w(TAG, "startUser(%d, %d, %b, %d) failed: cannot start profile user in " + + "foreground"); + return START_USER_RESULT_FAILURE; + } else { + boolean isParentRunning = mStartedProfileGroupIds + .get(actualProfileGroupId) == actualProfileGroupId; + if (DBG) { + Slogf.d(TAG, "profile parent running: %b", isParentRunning); + } + visibility = isParentRunning + ? START_USER_RESULT_SUCCESS_VISIBLE + : START_USER_RESULT_SUCCESS_INVISIBLE; + } + } else if (foreground) { + mCurrentUserId = userId; + visibility = START_USER_RESULT_SUCCESS_VISIBLE; + } else { + visibility = START_USER_RESULT_SUCCESS_INVISIBLE; + } + if (DBG) { + Slogf.d(TAG, "adding user / profile mapping (%d -> %d) and returning %s", + userId, actualProfileGroupId, startUserResultToString(visibility)); + } + mStartedProfileGroupIds.put(userId, actualProfileGroupId); + } + return visibility; + } + + /** + * TODO(b/244644281): merge with unassignUserFromDisplay() or add javadoc (and unit tests) + */ + public void stopUser(@UserIdInt int userId) { + if (DBG) { + Slogf.d(TAG, "stopUser(%d)", userId); + } + synchronized (mLock) { + mStartedProfileGroupIds.delete(userId); } } /** * See {@link UserManagerInternal#assignUserToDisplay(int, int)}. */ - public void assignUserToDisplay(int userId, int displayId) { + public void assignUserToDisplay(int userId, int profileGroupId, int displayId) { if (DBG) { - Slogf.d(TAG, "assignUserToDisplay(%d, %d)", userId, displayId); + Slogf.d(TAG, "assignUserToDisplay(%d, %d): mUsersOnSecondaryDisplaysEnabled=%b", + userId, displayId, mUsersOnSecondaryDisplaysEnabled); } - // NOTE: Using Boolean instead of boolean as it will be re-used below - Boolean isProfile = null; - if (displayId == Display.DEFAULT_DISPLAY) { - if (mUsersOnSecondaryDisplaysEnabled) { - // Profiles are only supported in the default display, but it cannot return yet - // as it needs to check if the parent is also assigned to the DEFAULT_DISPLAY - // (this is done indirectly below when it checks that the profile parent is the - // current user, as the current user is always assigned to the DEFAULT_DISPLAY). - isProfile = isProfileUnchecked(userId); - } - if (isProfile == null || !isProfile) { - // 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) { - Slogf.d(TAG, "ignoring on default display"); - } - return; + if (displayId == DEFAULT_DISPLAY + && (!mUsersOnSecondaryDisplaysEnabled || !isProfile(userId, profileGroupId))) { + // 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. + // But on MUMD devices, it profiles are only supported in the default display, so it + // cannot return yet as it needs to check if the parent is also assigned to the + // DEFAULT_DISPLAY (this is done indirectly below when it checks that the profile parent + // is the current user, as the current user is always assigned to the DEFAULT_DISPLAY). + if (DBG) { + Slogf.d(TAG, "ignoring on default display"); } + return; } if (!mUsersOnSecondaryDisplaysEnabled) { @@ -119,24 +203,21 @@ public final class UserVisibilityMediator { Preconditions.checkArgument(userId != currentUserId, "Cannot assign current user (%d) to other displays", currentUserId); - if (isProfile == null) { - isProfile = isProfileUnchecked(userId); - } - synchronized (mLock) { - if (isProfile) { - // Profile can only start in the same display as parent. And for simplicity, - // that display must be the DEFAULT_DISPLAY. - Preconditions.checkArgument(displayId == Display.DEFAULT_DISPLAY, - "Profile user can only be started in the default display"); - int parentUserId = getProfileParentId(userId); - Preconditions.checkArgument(parentUserId == currentUserId, - "Only profile of current user can be assigned to a display"); - if (DBG) { - Slogf.d(TAG, "Ignoring profile user %d on default display", userId); - } - return; + if (isProfile(userId, profileGroupId)) { + // Profile can only start in the same display as parent. And for simplicity, + // that display must be the DEFAULT_DISPLAY. + Preconditions.checkArgument(displayId == Display.DEFAULT_DISPLAY, + "Profile user can only be started in the default display"); + int parentUserId = getStartedProfileGroupId(userId); + Preconditions.checkArgument(parentUserId == currentUserId, + "Only profile of current user can be assigned to a display"); + if (DBG) { + Slogf.d(TAG, "Ignoring profile user %d on default display", userId); } + return; + } + synchronized (mLock) { // Check if display is available for (int i = 0; i < mUsersOnSecondaryDisplays.size(); i++) { int assignedUserId = mUsersOnSecondaryDisplays.keyAt(i); @@ -289,7 +370,7 @@ public final class UserVisibilityMediator { continue; } int userId = mUsersOnSecondaryDisplays.keyAt(i); - if (!isProfileUnchecked(userId)) { + if (!isStartedProfile(userId)) { return userId; } else if (DBG) { Slogf.d(TAG, "getUserAssignedToDisplay(%d): skipping user %d because it's " @@ -307,23 +388,42 @@ public final class UserVisibilityMediator { } private void dump(IndentingPrintWriter ipw) { - ipw.println("UserVisibilityManager"); + ipw.println("UserVisibilityMediator"); ipw.increaseIndent(); - ipw.print("Supports users on secondary displays: "); - ipw.println(mUsersOnSecondaryDisplaysEnabled); + synchronized (mLock) { + ipw.print("Current user id: "); + ipw.println(mCurrentUserId); - if (mUsersOnSecondaryDisplaysEnabled) { - ipw.print("Users on secondary displays: "); - synchronized (mLock) { - ipw.println(mUsersOnSecondaryDisplays); + ipw.print("Number of started user / profile group mappings: "); + ipw.println(mStartedProfileGroupIds.size()); + if (mStartedProfileGroupIds.size() > 0) { + ipw.increaseIndent(); + for (int i = 0; i < mStartedProfileGroupIds.size(); i++) { + ipw.print("User #"); + ipw.print(mStartedProfileGroupIds.keyAt(i)); + ipw.print(" -> profile #"); + ipw.println(mStartedProfileGroupIds.valueAt(i)); + } + ipw.decreaseIndent(); + } + + ipw.print("Supports users on secondary displays: "); + ipw.println(mUsersOnSecondaryDisplaysEnabled); + + if (mUsersOnSecondaryDisplaysEnabled) { + ipw.print("Users on secondary displays: "); + synchronized (mLock) { + ipw.println(mUsersOnSecondaryDisplays); + } } } ipw.decreaseIndent(); } - void dump(PrintWriter pw) { + @Override + public void dump(PrintWriter pw, String[] args) { if (pw instanceof IndentingPrintWriter) { dump((IndentingPrintWriter) pw); return; @@ -331,20 +431,70 @@ public final class UserVisibilityMediator { dump(new IndentingPrintWriter(pw)); } - // TODO(b/244644281): remove methods below once this class caches that state - private @UserIdInt int getCurrentUserId() { - return mService.getCurrentUserId(); + @VisibleForTesting + Map getUsersOnSecondaryDisplays() { + Map map; + synchronized (mLock) { + int size = mUsersOnSecondaryDisplays.size(); + map = new LinkedHashMap<>(size); + for (int i = 0; i < size; i++) { + map.put(mUsersOnSecondaryDisplays.keyAt(i), mUsersOnSecondaryDisplays.valueAt(i)); + } + } + Slogf.v(TAG, "getUsersOnSecondaryDisplays(): returning %s", map); + return map; } - private boolean isCurrentUserOrRunningProfileOfCurrentUser(@UserIdInt int userId) { - return mService.isCurrentUserOrRunningProfileOfCurrentUser(userId); + /** + * Gets the user-friendly representation of the {@code result}. + */ + public static String startUserResultToString(@StartUserResult int result) { + return DebugUtils.constantToString(UserVisibilityMediator.class, PREFIX_START_USER_RESULT, + result); } - private boolean isProfileUnchecked(@UserIdInt int userId) { - return mService.isProfileUnchecked(userId); + // TODO(b/244644281): methods below are needed because some APIs use the current users (full and + // profiles) state to decide whether a user is visible or not. If we decide to always store that + // info into intermediate maps, we should remove them. + + @VisibleForTesting + @UserIdInt int getCurrentUserId() { + synchronized (mLock) { + return mCurrentUserId; + } } - private @UserIdInt int getProfileParentId(@UserIdInt int userId) { - return mService.getProfileParentId(userId); + @VisibleForTesting + boolean isCurrentUserOrRunningProfileOfCurrentUser(@UserIdInt int userId) { + synchronized (mLock) { + // Special case as NO_PROFILE_GROUP_ID == USER_NULL + if (userId == USER_NULL || mCurrentUserId == USER_NULL) { + return false; + } + if (mCurrentUserId == userId) { + return true; + } + return mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID) == mCurrentUserId; + } + } + + private static boolean isProfile(@UserIdInt int userId, @UserIdInt int profileGroupId) { + return profileGroupId != NO_PROFILE_GROUP_ID && profileGroupId != userId; + } + + @VisibleForTesting + boolean isStartedProfile(@UserIdInt int userId) { + int profileGroupId; + synchronized (mLock) { + profileGroupId = mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID); + } + return isProfile(userId, profileGroupId); + } + + @VisibleForTesting + @UserIdInt int getStartedProfileGroupId(@UserIdInt int userId) { + synchronized (mLock) { + return mStartedProfileGroupIds.get(userId, NO_PROFILE_GROUP_ID); + } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/DumpableDumperRule.java b/services/tests/mockingservicestests/src/com/android/server/DumpableDumperRule.java new file mode 100644 index 0000000000000..33275bd471f60 --- /dev/null +++ b/services/tests/mockingservicestests/src/com/android/server/DumpableDumperRule.java @@ -0,0 +1,91 @@ +/* + * 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; + +import android.util.Dumpable; +import android.util.Log; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; + +/** + * {@code JUnit} rule that logs (using tag {@value #TAG} the contents of + * {@link Dumpable dumpables} in case of failure. + */ +public final class DumpableDumperRule implements TestRule { + + private static final String TAG = DumpableDumperRule.class.getSimpleName(); + + private static final String[] NO_ARGS = {}; + + private final List mDumpables = new ArrayList<>(); + + /** + * Adds a {@link Dumpable} to be logged if the test case fails. + */ + public void addDumpable(Dumpable dumpable) { + mDumpables.add(dumpable); + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + try { + base.evaluate(); + } catch (Throwable t) { + dumpOnFailure(description.getMethodName()); + throw t; + } + } + }; + } + + private void dumpOnFailure(String testName) throws IOException { + if (mDumpables.isEmpty()) { + return; + } + Log.w(TAG, "Dumping " + mDumpables.size() + " dumpables on failure of " + testName); + mDumpables.forEach(d -> logDumpable(d)); + } + + private void logDumpable(Dumpable dumpable) { + try { + try (StringWriter sw = new StringWriter(); PrintWriter pw = new PrintWriter(sw)) { + dumpable.dump(pw, NO_ARGS); + String[] dump = sw.toString().split(System.lineSeparator()); + Log.w(TAG, "Dumping " + dumpable.getDumpableName() + " (" + dump.length + + " lines):"); + for (String line : dump) { + Log.w(TAG, line); + } + + } catch (RuntimeException e) { + Log.e(TAG, "RuntimeException dumping " + dumpable.getDumpableName(), e); + } + } catch (IOException e) { + Log.e(TAG, "IOException dumping " + dumpable.getDumpableName(), e); + } + } +} diff --git a/services/tests/mockingservicestests/src/com/android/server/ExtendedMockitoTestCase.java b/services/tests/mockingservicestests/src/com/android/server/ExtendedMockitoTestCase.java index 9aa28ce741aa6..c0b507068358d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/ExtendedMockitoTestCase.java +++ b/services/tests/mockingservicestests/src/com/android/server/ExtendedMockitoTestCase.java @@ -23,6 +23,7 @@ import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.mockito.MockitoSession; import org.mockito.quality.Strictness; @@ -38,6 +39,9 @@ public abstract class ExtendedMockitoTestCase { private MockitoSession mSession; + @Rule + public final DumpableDumperRule mDumpableDumperRule = new DumpableDumperRule(); + @Before public void startSession() { if (DEBUG) { diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorMUMDTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorMUMDTest.java index 21f541f547906..923c3e385b5e8 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorMUMDTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorMUMDTest.java @@ -44,14 +44,14 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_systemUser() { - assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(USER_SYSTEM, SECONDARY_DISPLAY_ID)); + assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(USER_SYSTEM, USER_SYSTEM, SECONDARY_DISPLAY_ID)); } @Test public void testAssignUserToDisplay_invalidDisplay() { assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(USER_ID, INVALID_DISPLAY)); + () -> mMediator.assignUserToDisplay(USER_ID, USER_ID, INVALID_DISPLAY)); } @Test @@ -59,7 +59,7 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator mockCurrentUser(USER_ID); assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID)); + () -> mMediator.assignUserToDisplay(USER_ID, USER_ID, SECONDARY_DISPLAY_ID)); assertNoUserAssignedToDisplay(); } @@ -67,11 +67,10 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_startedProfileOfCurrentUser() { mockCurrentUser(PARENT_USER_ID); - addDefaultProfileAndParent(); startDefaultProfile(); - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertNoUserAssignedToDisplay(); @@ -80,11 +79,10 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_stoppedProfileOfCurrentUser() { mockCurrentUser(PARENT_USER_ID); - addDefaultProfileAndParent(); stopDefaultProfile(); - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertNoUserAssignedToDisplay(); @@ -92,17 +90,17 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_displayAvailable() { - mMediator.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + mMediator.assignUserToDisplay(USER_ID, USER_ID, SECONDARY_DISPLAY_ID); assertUserAssignedToDisplay(USER_ID, SECONDARY_DISPLAY_ID); } @Test public void testAssignUserToDisplay_displayAlreadyAssigned() { - mMediator.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + mMediator.assignUserToDisplay(USER_ID, USER_ID, SECONDARY_DISPLAY_ID); - IllegalStateException e = assertThrows(IllegalStateException.class, - () -> mMediator.assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID)); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> mMediator + .assignUserToDisplay(OTHER_USER_ID, OTHER_USER_ID, SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() @@ -112,10 +110,10 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_userAlreadyAssigned() { - mMediator.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID); + mMediator.assignUserToDisplay(USER_ID, USER_ID, SECONDARY_DISPLAY_ID); IllegalStateException e = assertThrows(IllegalStateException.class, - () -> mMediator.assignUserToDisplay(USER_ID, OTHER_SECONDARY_DISPLAY_ID)); + () -> mMediator.assignUserToDisplay(USER_ID, USER_ID, OTHER_SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertWithMessage("exception (%s) message", e).that(e).hasMessageThat() @@ -127,11 +125,9 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_profileOnSameDisplayAsParent() { - addDefaultProfileAndParent(); - - mMediator.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + mMediator.assignUserToDisplay(PARENT_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); @@ -139,11 +135,9 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_profileOnDifferentDisplayAsParent() { - addDefaultProfileAndParent(); - - mMediator.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, OTHER_SECONDARY_DISPLAY_ID)); + mMediator.assignUserToDisplay(PARENT_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, OTHER_SECONDARY_DISPLAY_ID)); Log.v(TAG, "Exception: " + e); assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); @@ -151,11 +145,9 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testAssignUserToDisplay_profileDefaultDisplayParentOnSecondaryDisplay() { - addDefaultProfileAndParent(); - - mMediator.assignUserToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY)); + mMediator.assignUserToDisplay(PARENT_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, DEFAULT_DISPLAY)); Log.v(TAG, "Exception: " + e); assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID); @@ -201,7 +193,6 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testIsUserVisibleOnDisplay_startedProfileOfCurrentUserSecondaryDisplayAssignedToAnotherUser() { - addDefaultProfileAndParent(); startDefaultProfile(); mockCurrentUser(PARENT_USER_ID); assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID); @@ -212,7 +203,6 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testIsUserVisibleOnDisplay_stoppedProfileOfCurrentUserSecondaryDisplayAssignedToAnotherUser() { - addDefaultProfileAndParent(); stopDefaultProfile(); mockCurrentUser(PARENT_USER_ID); assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID); @@ -223,7 +213,6 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator @Test public void testIsUserVisibleOnDisplay_startedProfileOfCurrentUserOnUnassignedSecondaryDisplay() { - addDefaultProfileAndParent(); startDefaultProfile(); mockCurrentUser(PARENT_USER_ID); @@ -285,19 +274,6 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator .that(mMediator.getUserAssignedToDisplay(SECONDARY_DISPLAY_ID)).isEqualTo(USER_ID); } - // TODO(b/244644281): scenario below shouldn't happen on "real life", as the profile cannot be - // started on secondary display if its parent isn't, so we might need to remove (or refactor - // this test) if/when the underlying logic changes - @Test - public void testGetUserAssignedToDisplay_profileOnSecondaryDisplay() { - addDefaultProfileAndParent(); - mockCurrentUser(USER_ID); - assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID); - - assertWithMessage("getUserAssignedToDisplay(%s)", SECONDARY_DISPLAY_ID) - .that(mMediator.getUserAssignedToDisplay(SECONDARY_DISPLAY_ID)).isEqualTo(USER_ID); - } - // NOTE: we don't need to add tests for profiles (started / stopped profiles of bg user), as // getUserAssignedToDisplay() for bg users relies only on the user / display assignments } diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorSUSDTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorSUSDTest.java index 7ae8117939495..7af5f5d6b2fe1 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorSUSDTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorSUSDTest.java @@ -39,27 +39,25 @@ public final class UserVisibilityMediatorSUSDTest extends UserVisibilityMediator mockCurrentUser(USER_ID); assertThrows(UnsupportedOperationException.class, - () -> mMediator.assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID)); + () -> mMediator.assignUserToDisplay(USER_ID, USER_ID, SECONDARY_DISPLAY_ID)); } @Test public void testAssignUserToDisplay_otherDisplay_startProfileOfcurrentUser() { mockCurrentUser(PARENT_USER_ID); - addDefaultProfileAndParent(); startDefaultProfile(); - assertThrows(UnsupportedOperationException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + assertThrows(UnsupportedOperationException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID)); } @Test public void testAssignUserToDisplay_otherDisplay_stoppedProfileOfcurrentUser() { mockCurrentUser(PARENT_USER_ID); - addDefaultProfileAndParent(); stopDefaultProfile(); - assertThrows(UnsupportedOperationException.class, - () -> mMediator.assignUserToDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)); + assertThrows(UnsupportedOperationException.class, () -> mMediator + .assignUserToDisplay(PROFILE_USER_ID, PARENT_USER_ID, SECONDARY_DISPLAY_ID)); } @Test diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorTestCase.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorTestCase.java index 22e6e0dcae1c1..7b20092b503a2 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorTestCase.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserVisibilityMediatorTestCase.java @@ -15,23 +15,27 @@ */ package com.android.server.pm; +import static android.content.pm.UserInfo.NO_PROFILE_GROUP_ID; import static android.os.UserHandle.USER_NULL; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.Display.INVALID_DISPLAY; -import static com.android.server.am.UserState.STATE_RUNNING_UNLOCKED; +import static com.android.server.pm.UserVisibilityMediator.INITIAL_CURRENT_USER_ID; +import static com.android.server.pm.UserVisibilityMediator.START_USER_RESULT_FAILURE; +import static com.android.server.pm.UserVisibilityMediator.START_USER_RESULT_SUCCESS_INVISIBLE; +import static com.android.server.pm.UserVisibilityMediator.START_USER_RESULT_SUCCESS_VISIBLE; +import static com.android.server.pm.UserVisibilityMediator.startUserResultToString; import static com.google.common.truth.Truth.assertWithMessage; import android.annotation.UserIdInt; -import android.util.SparseIntArray; +import android.util.Log; + +import com.android.server.ExtendedMockitoTestCase; import org.junit.Before; import org.junit.Test; -import java.util.LinkedHashMap; -import java.util.Map; - /** * Base class for {@link UserVisibilityMediator} tests. * @@ -39,7 +43,33 @@ import java.util.Map; * device mode (for example, whether the device supports concurrent multiple users on multiple * displays or not). */ -abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrInternalTestCase { +abstract class UserVisibilityMediatorTestCase extends ExtendedMockitoTestCase { + + private static final String TAG = UserVisibilityMediatorTestCase.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; /** * Id of a secondary display (i.e, not {@link android.view.Display.DEFAULT_DISPLAY}). @@ -51,12 +81,10 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern */ protected static final int OTHER_SECONDARY_DISPLAY_ID = 108; - private final boolean mUsersOnSecondaryDisplaysEnabled; + private static final boolean FG = true; + private static final boolean BG = false; - // TODO(b/244644281): manipulating mUsersOnSecondaryDisplays directly leaks implementation - // details into the unit test, but it's fine for now as the tests were copied "as is" - it - // would be better to use a geter() instead - protected final SparseIntArray mUsersOnSecondaryDisplays = new SparseIntArray(); + private final boolean mUsersOnSecondaryDisplaysEnabled; protected UserVisibilityMediator mMediator; @@ -66,13 +94,93 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Before public final void setMediator() { - mMediator = new UserVisibilityMediator(mUms, mUsersOnSecondaryDisplaysEnabled, - mUsersOnSecondaryDisplays); + mMediator = new UserVisibilityMediator(mUsersOnSecondaryDisplaysEnabled); + mDumpableDumperRule.addDumpable(mMediator); + } + + @Test + public final void testStartUser_currentUser() { + int result = mMediator.startUser(USER_ID, USER_ID, FG, DEFAULT_DISPLAY); + assertStartUserResult(result, START_USER_RESULT_SUCCESS_VISIBLE); + + assertCurrentUser(USER_ID); + assertIsCurrentUserOrRunningProfileOfCurrentUser(USER_ID); + assertStartedProfileGroupIdOf(USER_ID, USER_ID); + } + + @Test + public final void testStartUser_currentUserSecondaryDisplay() { + int result = mMediator.startUser(USER_ID, USER_ID, FG, SECONDARY_DISPLAY_ID); + assertStartUserResult(result, START_USER_RESULT_FAILURE); + + assertCurrentUser(INITIAL_CURRENT_USER_ID); + assertIsNotCurrentUserOrRunningProfileOfCurrentUser(USER_ID); + assertStartedProfileGroupIdOf(USER_ID, NO_PROFILE_GROUP_ID); + } + + @Test + public final void testStartUser_profileBg_parentStarted() { + mockCurrentUser(PARENT_USER_ID); + + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, BG, DEFAULT_DISPLAY); + assertStartUserResult(result, START_USER_RESULT_SUCCESS_VISIBLE); + + assertCurrentUser(PARENT_USER_ID); + assertIsCurrentUserOrRunningProfileOfCurrentUser(PROFILE_USER_ID); + assertStartedProfileGroupIdOf(PROFILE_USER_ID, PARENT_USER_ID); + assertIsStartedProfile(PROFILE_USER_ID); + } + + @Test + public final void testStartUser_profileBg_parentNotStarted() { + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, BG, DEFAULT_DISPLAY); + assertStartUserResult(result, START_USER_RESULT_SUCCESS_INVISIBLE); + + assertCurrentUser(INITIAL_CURRENT_USER_ID); + assertIsNotCurrentUserOrRunningProfileOfCurrentUser(PROFILE_USER_ID); + assertStartedProfileGroupIdOf(PROFILE_USER_ID, PARENT_USER_ID); + assertIsStartedProfile(PROFILE_USER_ID); + } + + @Test + public final void testStartUser_profileBg_secondaryDisplay() { + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, BG, SECONDARY_DISPLAY_ID); + assertStartUserResult(result, START_USER_RESULT_FAILURE); + + assertCurrentUser(INITIAL_CURRENT_USER_ID); + assertIsNotCurrentUserOrRunningProfileOfCurrentUser(PROFILE_USER_ID); + } + + @Test + public final void testStartUser_profileFg() { + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, FG, DEFAULT_DISPLAY); + assertStartUserResult(result, START_USER_RESULT_FAILURE); + + assertCurrentUser(INITIAL_CURRENT_USER_ID); + assertIsNotCurrentUserOrRunningProfileOfCurrentUser(PROFILE_USER_ID); + assertStartedProfileGroupIdOf(PROFILE_USER_ID, NO_PROFILE_GROUP_ID); + } + + @Test + public final void testStartUser_profileFgSecondaryDisplay() { + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, FG, SECONDARY_DISPLAY_ID); + + assertStartUserResult(result, START_USER_RESULT_FAILURE); + assertCurrentUser(INITIAL_CURRENT_USER_ID); + } + + @Test + public final void testGetStartedProfileGroupId_whenStartedWithNoProfileGroupId() { + int result = mMediator.startUser(USER_ID, NO_PROFILE_GROUP_ID, FG, DEFAULT_DISPLAY); + assertStartUserResult(result, START_USER_RESULT_SUCCESS_VISIBLE); + + assertWithMessage("shit").that(mMediator.getStartedProfileGroupId(USER_ID)) + .isEqualTo(USER_ID); } @Test public final void testAssignUserToDisplay_defaultDisplayIgnored() { - mMediator.assignUserToDisplay(USER_ID, DEFAULT_DISPLAY); + mMediator.assignUserToDisplay(USER_ID, USER_ID, DEFAULT_DISPLAY); assertNoUserAssignedToDisplay(); } @@ -103,18 +211,14 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testIsUserVisible_startedProfileOfcurrentUser() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); startDefaultProfile(); - setUserState(PROFILE_USER_ID, STATE_RUNNING_UNLOCKED); - assertWithMessage("isUserVisible(%s)", PROFILE_USER_ID) .that(mMediator.isUserVisible(PROFILE_USER_ID)).isTrue(); } @Test public final void testIsUserVisible_stoppedProfileOfcurrentUser() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); stopDefaultProfile(); @@ -164,7 +268,6 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testIsUserVisibleOnDisplay_startedProfileOfcurrentUserInvalidDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); startDefaultProfile(); @@ -174,7 +277,6 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testIsUserVisibleOnDisplay_stoppedProfileOfcurrentUserInvalidDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); stopDefaultProfile(); @@ -184,18 +286,14 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testIsUserVisibleOnDisplay_startedProfileOfcurrentUserDefaultDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); startDefaultProfile(); - setUserState(PROFILE_USER_ID, STATE_RUNNING_UNLOCKED); - assertWithMessage("isUserVisible(%s, %s)", PROFILE_USER_ID, DEFAULT_DISPLAY) .that(mMediator.isUserVisible(PROFILE_USER_ID, DEFAULT_DISPLAY)).isTrue(); } @Test public final void testIsUserVisibleOnDisplay_stoppedProfileOfcurrentUserDefaultDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); stopDefaultProfile(); @@ -205,18 +303,14 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testIsUserVisibleOnDisplay_startedProfileOfCurrentUserSecondaryDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); startDefaultProfile(); - setUserState(PROFILE_USER_ID, STATE_RUNNING_UNLOCKED); - assertWithMessage("isUserVisible(%s, %s)", PROFILE_USER_ID, SECONDARY_DISPLAY_ID) .that(mMediator.isUserVisible(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)).isTrue(); } @Test public void testIsUserVisibleOnDisplay_stoppedProfileOfcurrentUserSecondaryDisplay() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); stopDefaultProfile(); @@ -250,11 +344,8 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testGetDisplayAssignedToUser_startedProfileOfcurrentUser() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); startDefaultProfile(); - setUserState(PROFILE_USER_ID, STATE_RUNNING_UNLOCKED); - assertWithMessage("getDisplayAssignedToUser(%s)", PROFILE_USER_ID) .that(mMediator.getDisplayAssignedToUser(PROFILE_USER_ID)) .isEqualTo(DEFAULT_DISPLAY); @@ -262,7 +353,6 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern @Test public final void testGetDisplayAssignedToUser_stoppedProfileOfcurrentUser() { - addDefaultProfileAndParent(); mockCurrentUser(PARENT_USER_ID); stopDefaultProfile(); @@ -296,28 +386,96 @@ abstract class UserVisibilityMediatorTestCase extends UserManagerServiceOrIntern .isEqualTo(USER_ID); } - // NOTE: should only called by tests that indirectly needs to check user assignments (like - // isUserVisible), not by tests for the user assignment methods per se. + // TODO(b/244644281): remove if start & assign are merged; if they aren't, add a note explaining + // it's not meant to be used to test startUser() itself. + protected void mockCurrentUser(@UserIdInt int userId) { + Log.d(TAG, "mockCurrentUser(" + userId + ")"); + int result = mMediator.startUser(userId, userId, FG, DEFAULT_DISPLAY); + if (result != START_USER_RESULT_SUCCESS_VISIBLE) { + throw new IllegalStateException("Failed to mock current user " + userId + + ": mediator returned " + startUserResultToString(result)); + } + } + + // TODO(b/244644281): remove if start & assign are merged; if they aren't, add a note explaining + // it's not meant to be used to test startUser() itself. + protected void startDefaultProfile() { + mockCurrentUser(PARENT_USER_ID); + Log.d(TAG, "starting default profile (" + PROFILE_USER_ID + ") in background after starting" + + " its parent (" + PARENT_USER_ID + ") on foreground"); + + int result = mMediator.startUser(PROFILE_USER_ID, PARENT_USER_ID, BG, DEFAULT_DISPLAY); + if (result != START_USER_RESULT_SUCCESS_VISIBLE) { + throw new IllegalStateException("Failed to start profile user " + PROFILE_USER_ID + + ": mediator returned " + startUserResultToString(result)); + } + } + + // TODO(b/244644281): remove if start & assign are merged; if they aren't, add a note explaining + // it's not meant to be used to test stopUser() itself. + protected void stopDefaultProfile() { + Log.d(TAG, "stopping default profile"); + mMediator.stopUser(PROFILE_USER_ID); + } + + // TODO(b/244644281): remove if start & assign are merged; if they aren't, add a note explaining + // it's not meant to be used to test assignUserToDisplay() itself. protected final void assignUserToDisplay(@UserIdInt int userId, int displayId) { - mUsersOnSecondaryDisplays.put(userId, displayId); + Log.d(TAG, "assignUserToDisplay(" + userId + ", " + displayId + ")"); + int result = mMediator.startUser(userId, userId, BG, displayId); + if (result != START_USER_RESULT_SUCCESS_INVISIBLE) { + throw new IllegalStateException("Failed to startuser " + userId + + " on background: mediator returned " + startUserResultToString(result)); + } + mMediator.assignUserToDisplay(userId, userId, displayId); + } protected final void assertNoUserAssignedToDisplay() { - assertWithMessage("mUsersOnSecondaryDisplays()").that(usersOnSecondaryDisplaysAsMap()) + assertWithMessage("uses on secondary displays") + .that(mMediator.getUsersOnSecondaryDisplays()) .isEmpty(); } protected final void assertUserAssignedToDisplay(@UserIdInt int userId, int displayId) { - assertWithMessage("mUsersOnSecondaryDisplays()").that(usersOnSecondaryDisplaysAsMap()) + assertWithMessage("uses on secondary displays") + .that(mMediator.getUsersOnSecondaryDisplays()) .containsExactly(userId, displayId); } - private Map usersOnSecondaryDisplaysAsMap() { - int size = mUsersOnSecondaryDisplays.size(); - Map map = new LinkedHashMap<>(size); - for (int i = 0; i < size; i++) { - map.put(mUsersOnSecondaryDisplays.keyAt(i), mUsersOnSecondaryDisplays.valueAt(i)); - } - return map; + private void assertCurrentUser(@UserIdInt int userId) { + assertWithMessage("mediator.getCurrentUserId()").that(mMediator.getCurrentUserId()) + .isEqualTo(userId); + } + + private void assertIsStartedProfile(@UserIdInt int userId) { + assertWithMessage("mediator.isStartedProfile(%s)", userId) + .that(mMediator.isStartedProfile(userId)) + .isTrue(); + } + + private void assertStartedProfileGroupIdOf(@UserIdInt int profileId, @UserIdInt int parentId) { + assertWithMessage("mediator.getStartedProfileGroupId(%s)", profileId) + .that(mMediator.getStartedProfileGroupId(profileId)) + .isEqualTo(parentId); + } + + private void assertIsCurrentUserOrRunningProfileOfCurrentUser(int userId) { + assertWithMessage("mediator.isCurrentUserOrRunningProfileOfCurrentUser(%s)", userId) + .that(mMediator.isCurrentUserOrRunningProfileOfCurrentUser(userId)) + .isTrue(); + } + + private void assertIsNotCurrentUserOrRunningProfileOfCurrentUser(int userId) { + assertWithMessage("mediator.isCurrentUserOrRunningProfileOfCurrentUser(%s)", userId) + .that(mMediator.isCurrentUserOrRunningProfileOfCurrentUser(userId)) + .isFalse(); + } + + private void assertStartUserResult(int actualResult, int expectedResult) { + assertWithMessage("startUser() result (where %s=%s and %s=%s)", + actualResult, startUserResultToString(actualResult), + expectedResult, startUserResultToString(expectedResult)) + .that(actualResult).isEqualTo(expectedResult); } } 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 935d1d880a2a1..ca98150531aca 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -283,7 +283,7 @@ public class UserControllerTest { assertWithMessage("wrong binder message calls").that(mInjector.mHandler.getMessageCodes()) .containsExactly(USER_START_MSG); - verifyUserAssignedToDisplay(TEST_PRE_CREATED_USER_ID, Display.DEFAULT_DISPLAY); + verifyUserNeverAssignedToDisplay(); } private void startUserAssertions( @@ -948,11 +948,13 @@ public class UserControllerTest { } private void verifyUserAssignedToDisplay(@UserIdInt int userId, int displayId) { - verify(mInjector.getUserManagerInternal()).assignUserToDisplay(userId, displayId); + verify(mInjector.getUserManagerInternal()).assignUserToDisplay(eq(userId), anyInt(), + anyBoolean(), eq(displayId)); } private void verifyUserNeverAssignedToDisplay() { - verify(mInjector.getUserManagerInternal(), never()).assignUserToDisplay(anyInt(), anyInt()); + verify(mInjector.getUserManagerInternal(), never()).assignUserToDisplay(anyInt(), anyInt(), + anyBoolean(), anyInt()); } private void verifyUserUnassignedFromDisplay(@UserIdInt int userId) {