Merge "Improved UserController so users cannot start before it's ready." into main (c1564dc6) · Commits · e / os / android_frameworks_base

core/proto/android/server/activitymanagerservice.proto

+1 −0

Original line number	Diff line number	Diff line
		@@ -1003,6 +1003,7 @@ message UserControllerProto {
		// both the current user and its profiles (if any)
		optional int32 current_user = 5;
		repeated int32 current_profiles = 6;
		optional bool ready = 7;
		}

		// sync with com.android.server.am.AppTimeTracker.java

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

+26 −3

Original line number	Diff line number	Diff line
		@@ -251,6 +251,10 @@ class UserController implements Handler.Callback {
		*/
		private static final int POSTPONEMENT_TIME_FOR_BACKGROUND_USER_STOP_SECS = 30 * 60; // 30 mins

		@VisibleForTesting
		static final String EXCEPTION_TEMPLATE_CANNOT_START_USER_WHEN_NOT_READY =
		"Trying to start user %d before ready";

		/**
		* Maximum number of users we allow to be running at a time, including system user.
		*
		@@ -469,6 +473,13 @@ class UserController implements Handler.Callback {
		}
		};

		/**
		* Used to prevent external calls (for example, from {@code am start-user}) from crashing the
		* system when it's not ready yet.
		*/
		@GuardedBy("mLock")
		private boolean mReady;

		UserController(ActivityManagerService service) {
		this(new Injector(service));
		}
		@@ -2062,9 +2073,15 @@ class UserController implements Handler.Callback {
		private boolean startUserInternal(@UserIdInt int userId, int displayId,
		@UserStartMode int userStartMode, int autoStopUserInSecs,
		@Nullable IProgressListener unlockListener, TimingsTraceAndSlog t) {
		synchronized (mLock) {
		if (DEBUG_MU) {
		Slogf.i(TAG, "Starting user %d on display %d with mode %s", userId, displayId,
		userStartModeToString(userStartMode));
		Slogf.i(TAG, "Starting user %d on display %d with mode %s (when ready=%b)", userId,
		displayId, userStartModeToString(userStartMode), mReady);
		}
		// NOTE: for now this is the only place that's mReady, but if it's needed in others,
		// this check should be encapsulated into a private helper.
		Preconditions.checkState(mReady, EXCEPTION_TEMPLATE_CANNOT_START_USER_WHEN_NOT_READY,
		userId);
		}
		boolean foreground = userStartMode == USER_START_MODE_FOREGROUND;

		@@ -3411,6 +3428,10 @@ class UserController implements Handler.Callback {

		// IpcDataCache must be invalidated before it starts caching.
		ActivityManager.invalidateGetCurrentUserIdCache();

		synchronized (mLock) {
		mReady = true;
		}
		}

		// TODO(b/266158156): remove this method if initial system user boot logic is refactored?
		@@ -3854,6 +3875,7 @@ class UserController implements Handler.Callback {
		for (int i = 0; i < mCurrentProfileIds.length; i++) {
		proto.write(UserControllerProto.CURRENT_PROFILES, mCurrentProfileIds[i]);
		}
		proto.write(UserControllerProto.READY, mReady);
		proto.end(token);
		}
		}
		@@ -3913,6 +3935,7 @@ class UserController implements Handler.Callback {
		pw.println(" mSwitchingFromUserMessage:" + mSwitchingFromUserMessage);
		pw.println(" mSwitchingToUserMessage:" + mSwitchingToUserMessage);
		pw.println(" mLastUserUnlockingUptime: " + mLastUserUnlockingUptime);
		pw.println(" mReady: " + mReady);
		}
		}

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

+30 −0

Original line number	Diff line number	Diff line
		@@ -141,6 +141,7 @@ import java.util.HashMap;
		import java.util.HashSet;
		import java.util.LinkedHashSet;
		import java.util.List;
		import java.util.Locale;
		import java.util.Set;
		import java.util.stream.Collectors;
		import java.util.stream.Stream;
		@@ -170,6 +171,10 @@ public class UserControllerTest {
		private static final long HANDLER_WAIT_TIME_MS = 100;

		private UserController mUserController;

		// Used by tests that assert state when it's not ready
		private UserController mNotReadyUserController;

		private TestInjector mInjector;
		private final HashMap<Integer, UserState> mUserStates = new HashMap<>();
		private final HashMap<Integer, UserInfo> mUserInfos = new HashMap<>();
		@@ -232,6 +237,11 @@ public class UserControllerTest {

		mUserController = new UserController(mInjector);
		mUserController.setAllowUserUnlocking(true);
		mUserController.onSystemReady();

		mNotReadyUserController = new UserController(mInjector);
		mNotReadyUserController.setAllowUserUnlocking(true);

		setUpUser(TEST_USER_ID, DEFAULT_USER_FLAGS);
		setUpUser(TEST_PRE_CREATED_USER_ID, DEFAULT_USER_FLAGS, /* preCreated= */ true, null);
		mInjector.mRelevantUser = null;
		@@ -244,6 +254,16 @@ public class UserControllerTest {
		validateMockitoUsage();
		}

		@Test
		public void testStartUser_foreground_notReady() {
		var e = assertThrows(IllegalStateException.class,
		() -> mNotReadyUserController.startUser(TEST_USER_ID, USER_START_MODE_FOREGROUND));

		assertThat(e).hasMessageThat().isEqualTo(String.format(Locale.ENGLISH,
		UserController.EXCEPTION_TEMPLATE_CANNOT_START_USER_WHEN_NOT_READY, TEST_USER_ID));
		}


		@Test
		public void testStartUser_foreground() {
		mUserController.startUser(TEST_USER_ID, USER_START_MODE_FOREGROUND);
		@@ -255,6 +275,16 @@ public class UserControllerTest {
		verifyUserAssignedToDisplay(TEST_USER_ID, Display.DEFAULT_DISPLAY);
		}

		@Test
		public void testStartUser_background_notReady() {
		var e = assertThrows(IllegalStateException.class,
		() -> mNotReadyUserController.startUser(TEST_USER_ID, USER_START_MODE_BACKGROUND));

		assertThat(e).hasMessageThat().isEqualTo(String.format(Locale.ENGLISH,
		UserController.EXCEPTION_TEMPLATE_CANNOT_START_USER_WHEN_NOT_READY, TEST_USER_ID));

		}

		@Test
		public void testStartUser_background() {
		mUserController.setInitialConfig(/* mUserSwitchUiEnabled= */ true,