Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d16bc3ee authored by Felipe Leme's avatar Felipe Leme
Browse files

UserVisibilityMediator refactoring, step 2.

The previous refactoring introduced a startUser() on
UserVisibilityMediator and changed UserManagerInternal to call both
startUser() and assignUserToDisplay(); this CL "merges" these two
methods into just startUser(), and changed UserController.startUser()
workflow so the logic of whether the user is visible is handled by
UserVisibilityMediator.

Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest UserVisibilityMediatorMUMDTest UserVisibilityMediatorSUSDTest UserControllerTest # unit
Test: atest CtsMultiUserTestCases:android.multiuser.cts.MultipleUsersOnMultipleDisplaysTest # CTS
Test: adb shell dumpsys user --visibility-mediator

Bug: 244644281

Change-Id: Ide7b62c1ab3bb119dde7dc2790919ad3c864b3e9
parent 90e81227
Loading
Loading
Loading
Loading
+25 −21
Original line number Diff line number Diff line
@@ -1090,7 +1090,7 @@ class UserController implements Handler.Callback {
            // TODO(b/239982558): for now we're just updating the user's visibility, but most likely
            // we'll need to remove this call and handle that as part of the user state workflow
            // instead.
            userManagerInternal.unassignUserFromDisplay(userId);
            userManagerInternal.unassignUserFromDisplayOnStop(userId);

            final boolean visibilityChanged;
            boolean visibleBefore;
@@ -1650,13 +1650,30 @@ class UserController implements Handler.Callback {
                return false;
            }

            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,
            t.traceBegin("assignUserToDisplayOnStart");
            int result = mInjector.getUserManagerInternal().assignUserToDisplayOnStart(userId,
                    userInfo.profileGroupId, foreground, displayId);
            t.traceEnd();

            boolean visible;
            switch (result) {
                case UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE:
                    visible = true;
                    break;
                case UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE:
                    visible = false;
                    break;
                default:
                    Slogf.wtf(TAG, "Wrong result from assignUserToDisplayOnStart(): %d", result);
                    // Fall through
                case UserManagerInternal.USER_ASSIGNMENT_RESULT_FAILURE:
                    Slogf.e(TAG, "%s user(%d) / display (%d) assignment failed: %s",
                            (foreground ? "fg" : "bg"), userId, displayId,
                            UserManagerInternal.userAssignmentResultToString(result));
                    return false;
            }


            // TODO(b/239982558): might need something similar for bg users on secondary display
            if (foreground && isUserSwitchUiEnabled()) {
                t.traceBegin("startFreezingScreen");
@@ -1751,19 +1768,6 @@ class UserController implements Handler.Callback {
            }
            t.traceEnd();

            // Need to call UM when user is on background, as there are some cases where the user
            // cannot be started in background on a secondary display (for example, if user is a
            // profile).
            // TODO(b/253103846): it's also explicitly checking if the user is the USER_SYSTEM, as
            // the UM call would return true during boot (when CarService / BootUserInitializer
            // calls AM.startUserInBackground() because the system user is still the current user.
            // TODO(b/244644281): another fragility of this check is that it must wait to call
            // UMI.isUserVisible() until the user state is check, as that method checks if the
            // profile of the current user is started. We should fix that dependency so the logic
            // belongs to just one place (like UserDisplayAssigner)
            boolean visible = foreground
                    || userId != UserHandle.USER_SYSTEM
                            && mInjector.getUserManagerInternal().isUserVisible(userId);
            if (visible) {
                synchronized (mLock) {
                    addVisibleUserLocked(userId);
@@ -1816,8 +1820,8 @@ class UserController implements Handler.Callback {
                // user that was started in the background before), so it's necessary to explicitly
                // notify the services (while when the user starts from BOOTING, USER_START_MSG
                // takes care of that.
                mHandler.sendMessage(mHandler.obtainMessage(USER_VISIBILITY_CHANGED_MSG, userId,
                        visible ? 1 : 0));
                mHandler.sendMessage(
                        mHandler.obtainMessage(USER_VISIBILITY_CHANGED_MSG, userId, 1));
            }

            t.traceBegin("sendMessages");
+33 −17
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.content.pm.UserProperties;
import android.graphics.Bitmap;
import android.os.Bundle;
import android.os.UserManager;
import android.util.DebugUtils;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -46,6 +47,18 @@ public abstract class UserManagerInternal {
    public @interface OwnerType {
    }

    public static final int USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE = 1;
    public static final int USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE = 2;
    public static final int USER_ASSIGNMENT_RESULT_FAILURE = -1;

    private static final String PREFIX_USER_ASSIGNMENT_RESULT = "USER_ASSIGNMENT_RESULT";
    @IntDef(flag = false, prefix = {PREFIX_USER_ASSIGNMENT_RESULT}, value = {
            USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE,
            USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE,
            USER_ASSIGNMENT_RESULT_FAILURE
    })
    public @interface UserAssignmentResult {}

    public interface UserRestrictionsListener {
        /**
         * Called when a user restriction changes.
@@ -343,34 +356,28 @@ public abstract class UserManagerInternal {
    public abstract @Nullable UserProperties getUserProperties(@UserIdInt int userId);

    /**
     * Assigns a user to a display.
     *
     * <p>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).
     * Assigns a user to a display when it's starting, returning whether the assignment succeeded
     * and the user is {@link UserManager#isUserVisible() visible}.
     *
     * <p><b>NOTE: </b>this method is meant to be used only by {@code UserController} (when a user
     * is started)
     * is started). If other clients (like {@code CarService} need to explicitly change the user /
     * display assignment, we'll need to provide other APIs.
     *
     * <p><b>NOTE: </b>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).
     * pass a valid display id.
     */
    // 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,
    public abstract @UserAssignmentResult int assignUserToDisplayOnStart(@UserIdInt int userId,
            @UserIdInt int profileGroupId,
            boolean foreground, int displayId);

    /**
     * Unassigns a user from its current display.
     *
     * <p>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).
     * Unassigns a user from its current display when it's stopping.
     *
     * <p><b>NOTE: </b>this method is meant to be used only by {@code UserController} (when a user
     * is stopped).
     * is stopped). If other clients (like {@code CarService} need to explicitly change the user /
     * display assignment, we'll need to provide other APIs.
     */
    public abstract void unassignUserFromDisplay(@UserIdInt int userId);
    public abstract void unassignUserFromDisplayOnStop(@UserIdInt int userId);

    /**
     * Returns {@code true} if the user is visible (as defined by
@@ -413,6 +420,15 @@ public abstract class UserManagerInternal {
     */
    public abstract @UserIdInt int getUserAssignedToDisplay(int displayId);

    /**
     * Gets the user-friendly representation of the {@code result} of a
     * {@link #assignUserToDisplayOnStart(int, int, boolean, int)} call.
     */
    public static String userAssignmentResultToString(@UserAssignmentResult int result) {
        return DebugUtils.constantToString(UserManagerInternal.class, PREFIX_USER_ASSIGNMENT_RESULT,
                result);
    }

    /** Adds a {@link UserVisibilityListener}. */
    public abstract void addUserVisibilityListener(UserVisibilityListener listener);

+4 −5
Original line number Diff line number Diff line
@@ -6799,15 +6799,14 @@ public class UserManagerService extends IUserManager.Stub {
        }

        @Override
        public void assignUserToDisplay(@UserIdInt int userId, @UserIdInt int profileGroupId,
        public int assignUserToDisplayOnStart(@UserIdInt int userId, @UserIdInt int profileGroupId,
                boolean foreground, int displayId) {
            mUserVisibilityMediator.startUser(userId, profileGroupId, foreground, displayId);
            mUserVisibilityMediator.assignUserToDisplay(userId, profileGroupId, displayId);
            return mUserVisibilityMediator.startUser(userId, profileGroupId, foreground,
                    displayId);
        }

        @Override
        public void unassignUserFromDisplay(@UserIdInt int userId) {
            mUserVisibilityMediator.unassignUserFromDisplay(userId);
        public void unassignUserFromDisplayOnStop(@UserIdInt int userId) {
            mUserVisibilityMediator.stopUser(userId);
        }

+93 −76
Original line number Diff line number Diff line
@@ -20,12 +20,15 @@ import static android.os.UserHandle.USER_NULL;
import static android.os.UserHandle.USER_SYSTEM;
import static android.view.Display.DEFAULT_DISPLAY;

import android.annotation.IntDef;
import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_FAILURE;
import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
import static com.android.server.pm.UserManagerInternal.USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE;
import static com.android.server.pm.UserManagerInternal.userAssignmentResultToString;

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;
@@ -34,6 +37,7 @@ import android.view.Display;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
import com.android.server.pm.UserManagerInternal.UserAssignmentResult;
import com.android.server.utils.Slogf;

import java.io.PrintWriter;
@@ -52,23 +56,10 @@ public final class UserVisibilityMediator implements Dumpable {

    private static final String TAG = UserVisibilityMediator.class.getSimpleName();

    private static final String PREFIX_START_USER_RESULT = "START_USER_";

    // TODO(b/242195409): might need to change this if boot logic is refactored for HSUM devices
    @VisibleForTesting
    static final int INITIAL_CURRENT_USER_ID = USER_SYSTEM;

    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;
@@ -97,10 +88,37 @@ public final class UserVisibilityMediator implements Dumpable {
    }

    /**
     * TODO(b/244644281): merge with assignUserToDisplay() or add javadoc.
     * See {@link UserManagerInternal#assignUserToDisplayOnStart(int, int, boolean, int)}.
     */
    public @StartUserResult int startUser(@UserIdInt int userId, @UserIdInt int profileGroupId,
    public @UserAssignmentResult int startUser(@UserIdInt int userId, @UserIdInt int profileGroupId,
            boolean foreground, int displayId) {
        // TODO(b/244644281): this method need to perform 4 actions:
        //
        // 1. Check if the user can be started given the provided arguments
        // 2. If it can, decide whether it's visible or not (which is the return value)
        // 3. Update the current user / profiles state
        // 4. Update the users on secondary display state (if applicable)
        //
        // Ideally, they should be done "atomically" (i.e, only changing state while holding the
        // mLock), but the initial implementation is just calling the existing methods, as the
        // focus is to change the UserController startUser() workflow (so it relies on this class
        // for the logic above).
        //
        // The next CL will refactor it (and the unit tests) to achieve that atomicity.
        int result = startOnly(userId, profileGroupId, foreground, displayId);
        if (result != USER_ASSIGNMENT_RESULT_FAILURE) {
            assignUserToDisplay(userId, profileGroupId, displayId);
        }
        return result;
    }

    /**
     * @deprecated - see comment inside {@link #startUser(int, int, boolean, int)}
     */
    @Deprecated
    @VisibleForTesting
    @UserAssignmentResult int startOnly(@UserIdInt int userId,
            @UserIdInt int profileGroupId, boolean foreground, int displayId) {
        int actualProfileGroupId = profileGroupId == NO_PROFILE_GROUP_ID
                ? userId
                : profileGroupId;
@@ -111,7 +129,7 @@ public final class UserVisibilityMediator implements Dumpable {
        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;
            return USER_ASSIGNMENT_RESULT_FAILURE;
        }

        int visibility;
@@ -121,13 +139,12 @@ public final class UserVisibilityMediator implements Dumpable {
                    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;
                    return USER_ASSIGNMENT_RESULT_FAILURE;
                }
                if (foreground) {
                    Slogf.w(TAG, "startUser(%d, %d, %b, %d) failed: cannot start profile user in "
                            + "foreground", userId, actualProfileGroupId, foreground,
                            displayId);
                    return START_USER_RESULT_FAILURE;
                            + "foreground", userId, actualProfileGroupId, foreground, displayId);
                    return USER_ASSIGNMENT_RESULT_FAILURE;
                } else {
                    boolean isParentRunning = mStartedProfileGroupIds
                            .get(actualProfileGroupId) == actualProfileGroupId;
@@ -135,18 +152,18 @@ public final class UserVisibilityMediator implements Dumpable {
                        Slogf.d(TAG, "profile parent running: %b", isParentRunning);
                    }
                    visibility = isParentRunning
                            ? START_USER_RESULT_SUCCESS_VISIBLE
                            : START_USER_RESULT_SUCCESS_INVISIBLE;
                            ? USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE
                            : USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
                }
            } else if (foreground) {
                mCurrentUserId = userId;
                visibility = START_USER_RESULT_SUCCESS_VISIBLE;
                visibility = USER_ASSIGNMENT_RESULT_SUCCESS_VISIBLE;
            } else {
                visibility = START_USER_RESULT_SUCCESS_INVISIBLE;
                visibility = USER_ASSIGNMENT_RESULT_SUCCESS_INVISIBLE;
            }
            if (DBG) {
                Slogf.d(TAG, "adding user / profile mapping (%d -> %d) and returning %s",
                        userId, actualProfileGroupId, startUserResultToString(visibility));
                        userId, actualProfileGroupId, userAssignmentResultToString(visibility));
            }
            mStartedProfileGroupIds.put(userId, actualProfileGroupId);
        }
@@ -154,21 +171,11 @@ public final class UserVisibilityMediator implements Dumpable {
    }

    /**
     * TODO(b/244644281): merge with unassignUserFromDisplay() or add javadoc (and unit tests)
     * @deprecated - see comment inside {@link #startUser(int, int, boolean, int)}
     */
    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 profileGroupId, int displayId) {
    @Deprecated
    @VisibleForTesting
    void assignUserToDisplay(int userId, int profileGroupId, int displayId) {
        if (DBG) {
            Slogf.d(TAG, "assignUserToDisplay(%d, %d): mUsersOnSecondaryDisplaysEnabled=%b",
                    userId, displayId, mUsersOnSecondaryDisplaysEnabled);
@@ -246,22 +253,25 @@ public final class UserVisibilityMediator implements Dumpable {
    }

    /**
     * See {@link UserManagerInternal#unassignUserFromDisplay(int)}.
     * See {@link UserManagerInternal#unassignUserFromDisplayOnStop(int)}.
     */
    public void unassignUserFromDisplay(int userId) {
    public void stopUser(int userId) {
        if (DBG) {
            Slogf.d(TAG, "unassignUserFromDisplay(%d)", userId);
            Slogf.d(TAG, "stopUser(%d)", userId);
        }
        if (!mUsersOnSecondaryDisplaysEnabled) {
            // 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.
        synchronized (mLock) {
            if (DBG) {
                Slogf.d(TAG, "ignoring when device doesn't support MUMD");
                Slogf.d(TAG, "Removing %d from mStartedProfileGroupIds (%s)", userId,
                        mStartedProfileGroupIds);
            }
            mStartedProfileGroupIds.delete(userId);

            if (!mUsersOnSecondaryDisplaysEnabled) {
                // Don't need to do update mUsersOnSecondaryDisplays because methods (such as
                // isUserVisible()) already know that the current user (and their profiles) is
                // assigned to the default display.
                return;
            }

        synchronized (mLock) {
            if (DBG) {
                Slogf.d(TAG, "Removing %d from mUsersOnSecondaryDisplays (%s)", userId,
                        mUsersOnSecondaryDisplays);
@@ -395,30 +405,37 @@ public final class UserVisibilityMediator implements Dumpable {
            ipw.print("Current user id: ");
            ipw.println(mCurrentUserId);

            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();
            }
            dumpIntArray(ipw, mStartedProfileGroupIds, "started user / profile group", "u", "pg");

            ipw.print("Supports users on secondary displays: ");
            ipw.print("Supports background users on secondary displays: ");
            ipw.println(mUsersOnSecondaryDisplaysEnabled);

            if (mUsersOnSecondaryDisplaysEnabled) {
                ipw.print("Users on secondary displays: ");
                synchronized (mLock) {
                    ipw.println(mUsersOnSecondaryDisplays);
                dumpIntArray(ipw, mUsersOnSecondaryDisplays, "background user / secondary display",
                        "u", "d");
            }
        }

        ipw.decreaseIndent();
    }

    private static void dumpIntArray(IndentingPrintWriter ipw, SparseIntArray array,
            String arrayDescription, String keyName, String valueName) {
        ipw.print("Number of ");
        ipw.print(arrayDescription);
        ipw.print(" mappings: ");
        ipw.println(array.size());
        if (array.size() <= 0) {
            return;
        }
        ipw.increaseIndent();
        for (int i = 0; i < array.size(); i++) {
            ipw.print(keyName); ipw.print(':');
            ipw.print(array.keyAt(i));
            ipw.print(" -> ");
            ipw.print(valueName); ipw.print(':');
            ipw.println(array.valueAt(i));
        }
        ipw.decreaseIndent();
    }

@@ -445,14 +462,6 @@ public final class UserVisibilityMediator implements Dumpable {
        return map;
    }

    /**
     * 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);
    }

    // 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.
@@ -482,6 +491,14 @@ public final class UserVisibilityMediator implements Dumpable {
        return profileGroupId != NO_PROFILE_GROUP_ID && profileGroupId != userId;
    }

    @VisibleForTesting
    boolean isStartedUser(@UserIdInt int userId) {
        synchronized (mLock) {
            return mStartedProfileGroupIds.get(userId,
                    INITIAL_CURRENT_USER_ID) != INITIAL_CURRENT_USER_ID;
        }
    }

    @VisibleForTesting
    boolean isStartedProfile(@UserIdInt int userId) {
        int profileGroupId;
+2 −8
Original line number Diff line number Diff line
@@ -153,14 +153,8 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator
        assertUserAssignedToDisplay(PARENT_USER_ID, SECONDARY_DISPLAY_ID);
    }

    @Test
    public void testUnassignUserFromDisplay() {
        testAssignUserToDisplay_displayAvailable();

        mMediator.unassignUserFromDisplay(USER_ID);

        assertNoUserAssignedToDisplay();
    }
    // TODO(b/244644281): when start & assign are merged, rename tests above and also call
    // stopUserAndAssertState() at the end of them

    @Test
    public void testIsUserVisible_bgUserOnSecondaryDisplay() {
Loading