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

Commit 7a1bfdd0 authored by Felipe Leme's avatar Felipe Leme Committed by Yuncheol Heo
Browse files

Minor adjustements on user visibility internals:

- Added new UserManagerInternal.unassignUserFromDisplay() method
 (instead of calling assignUserToDisplay() passing INVALID_DISPLAY)
- Refactored UserManagerService to:
  - set mUsersOnSecondaryDisplays on constructor (it will be null
    if device doesn't support it)
  - change methods that use mUsersOnSecondaryDisplays to explicitly
    check for the main display case first (which is the most common
    usage) and ignore the rest when it's not supported
- Moved all related logic from UserController to UserManagerService
- Removed extra checks for default display
- Created a DEBUG_MUMD logging guard
- Unassign user from display earlier when stopping it
- Use separate lock for mUsersOnSecondaryDisplays

These changes not only makes the code simpler, but would allow the
logic to be refactored into a helper class (which could be more
unit tested in isolation).

Test: atest CtsMultiUserTestCases:android.multiuser.cts.UserManagerTest\
 CtsMultiUserTestCases:android.multiuser.cts.MultipleUsersOnMultipleDisplaysTest

Bug: 239982558
Fixes: 243869778
Fixes: 239824814
Fixes: 244331360

Change-Id: I8df66f936825813b71a141acdbb6cd3771520fc9
parent 697f48f7
Loading
Loading
Loading
Loading
+11 −25
Original line number Original line Diff line number Diff line
@@ -1036,7 +1036,13 @@ class UserController implements Handler.Callback {
        if (uss.state != UserState.STATE_STOPPING
        if (uss.state != UserState.STATE_STOPPING
                && uss.state != UserState.STATE_SHUTDOWN) {
                && uss.state != UserState.STATE_SHUTDOWN) {
            uss.setState(UserState.STATE_STOPPING);
            uss.setState(UserState.STATE_STOPPING);
            mInjector.getUserManagerInternal().setUserState(userId, uss.state);
            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.unassignUserFromDisplay(userId);

            updateStartedUserArrayLU();
            updateStartedUserArrayLU();


            final boolean allowDelayedLockingCopied = allowDelayedLocking;
            final boolean allowDelayedLockingCopied = allowDelayedLocking;
@@ -1076,12 +1082,6 @@ class UserController implements Handler.Callback {
                        Binder.getCallingPid(), UserHandle.USER_ALL);
                        Binder.getCallingPid(), UserHandle.USER_ALL);
            });
            });
        }
        }

        // 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.
        // TODO(b/240613396) also check if multi-display is supported
        mInjector.getUserManagerInternal().assignUserToDisplay(userId, Display.INVALID_DISPLAY);
    }
    }


    private void finishUserStopping(final int userId, final UserState uss,
    private void finishUserStopping(final int userId, final UserState uss,
@@ -1391,7 +1391,6 @@ class UserController implements Handler.Callback {
                && !user.isQuietModeEnabled();
                && !user.isQuietModeEnabled();
    }
    }


    // TODO(b/239982558): might need to infer the display id based on parent user
    /**
    /**
     * Starts a user only if it's a profile, with a more relaxed permission requirement:
     * Starts a user only if it's a profile, with a more relaxed permission requirement:
     * {@link android.Manifest.permission#MANAGE_USERS} or
     * {@link android.Manifest.permission#MANAGE_USERS} or
@@ -1420,6 +1419,7 @@ class UserController implements Handler.Callback {
            return false;
            return false;
        }
        }


        // TODO(b/239982558): pass proper displayId
        return startUserNoChecks(userId, Display.DEFAULT_DISPLAY, /* foreground= */ false,
        return startUserNoChecks(userId, Display.DEFAULT_DISPLAY, /* foreground= */ false,
                /* unlockListener= */ null);
                /* unlockListener= */ null);
    }
    }
@@ -1477,7 +1477,7 @@ class UserController implements Handler.Callback {
        checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay",
        checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay",
                MANAGE_USERS, CREATE_USERS);
                MANAGE_USERS, CREATE_USERS);


        // DEFAULT_DISPLAY is used for "regular" start user operations
        // DEFAULT_DISPLAY is used for the current foreground user only
        Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY,
        Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY,
                "Cannot use DEFAULT_DISPLAY");
                "Cannot use DEFAULT_DISPLAY");


@@ -1510,27 +1510,13 @@ class UserController implements Handler.Callback {
                    foreground ? " in foreground" : "");
                    foreground ? " in foreground" : "");
        }
        }


        // TODO(b/239982558): move logic below to a different class (like DisplayAssignmentManager)
        if (displayId != Display.DEFAULT_DISPLAY) {
        if (displayId != Display.DEFAULT_DISPLAY) {
            // This is called by startUserOnSecondaryDisplay()
            if (!UserManager.isUsersOnSecondaryDisplaysEnabled()) {
                // TODO(b/239824814): add CTS test and/or unit test for all exceptional cases
                throw new UnsupportedOperationException("Not supported by device");
            }

            // TODO(b/239982558): call DisplayManagerInternal to check if display is valid instead
            Preconditions.checkArgument(displayId > 0, "Invalid display id (%d)", displayId);
            Preconditions.checkArgument(userId != UserHandle.USER_SYSTEM, "Cannot start system user"
                    + " on secondary display (%d)", displayId);
            Preconditions.checkArgument(!foreground, "Cannot start user %d in foreground AND "
            Preconditions.checkArgument(!foreground, "Cannot start user %d in foreground AND "
                    + "on secondary display (%d)", userId, displayId);
                    + "on secondary display (%d)", userId, displayId);

            // 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.
            mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId);
        }
        }
        mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId);


        // TODO(b/239982558): log display id (or use a new event)
        EventLog.writeEvent(EventLogTags.UC_START_USER_INTERNAL, userId);
        EventLog.writeEvent(EventLogTags.UC_START_USER_INTERNAL, userId);


        final int callingUid = Binder.getCallingUid();
        final int callingUid = Binder.getCallingUid();
+14 −1
Original line number Original line Diff line number Diff line
@@ -313,9 +313,22 @@ public abstract class UserManagerInternal {
     */
     */
    public abstract @Nullable UserProperties getUserProperties(@UserIdInt int userId);
    public abstract @Nullable UserProperties getUserProperties(@UserIdInt int userId);


    /** TODO(b/239982558): add javadoc / mention invalid_id is used to unassing */
    /**
     * 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).
     */
    public abstract void assignUserToDisplay(@UserIdInt int userId, int displayId);
    public abstract void assignUserToDisplay(@UserIdInt int userId, 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).
     */
    public abstract void unassignUserFromDisplay(@UserIdInt int userId);

    /**
    /**
     * Returns {@code true} if the user is visible (as defined by
     * Returns {@code true} if the user is visible (as defined by
     * {@link UserManager#isUserVisible()} in the given display.
     * {@link UserManager#isUserVisible()} in the given display.
+85 −61
Original line number Original line Diff line number Diff line
@@ -173,6 +173,8 @@ public class UserManagerService extends IUserManager.Stub {


    private static final String LOG_TAG = "UserManagerService";
    private static final String LOG_TAG = "UserManagerService";
    static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE
    static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE
    // For Multiple Users on Multiple Displays
    static final boolean DBG_MUMD = false; // DO NOT SUBMIT WITH TRUE
    private static final boolean DBG_WITH_STACKTRACE = false; // DO NOT SUBMIT WITH TRUE
    private static final boolean DBG_WITH_STACKTRACE = false; // DO NOT SUBMIT WITH TRUE
    // Can be used for manual testing of id recycling
    // Can be used for manual testing of id recycling
    private static final boolean RELEASE_DELETED_USER_ID = false; // DO NOT SUBMIT WITH TRUE
    private static final boolean RELEASE_DELETED_USER_ID = false; // DO NOT SUBMIT WITH TRUE
@@ -624,15 +626,16 @@ public class UserManagerService extends IUserManager.Stub {
    @GuardedBy("mUserStates")
    @GuardedBy("mUserStates")
    private final WatchedUserStates mUserStates = new WatchedUserStates();
    private final WatchedUserStates mUserStates = new WatchedUserStates();




    /**
    /**
     * Used on devices that support background users (key) running on secondary displays (value).
     * Set on on devices that support background users (key) running on secondary displays (value).
     *
     * <p>Is {@code null} by default and instantiated on demand when the users are started on
     * secondary displays.
     */
     */
    // TODO(b/239982558): move such logic to a different class (like UserDisplayAssigner)
    @Nullable
    @Nullable
    @GuardedBy("mUsersLock")
    @GuardedBy("mUsersOnSecondaryDisplays")
    private SparseIntArray mUsersOnSecondaryDisplays;
    private final SparseIntArray mUsersOnSecondaryDisplays;
    private final boolean mUsersOnSecondaryDisplaysEnabled;


    private static UserManagerService sInstance;
    private static UserManagerService sInstance;


@@ -750,6 +753,8 @@ public class UserManagerService extends IUserManager.Stub {
        mUserStates.put(UserHandle.USER_SYSTEM, UserState.STATE_BOOTING);
        mUserStates.put(UserHandle.USER_SYSTEM, UserState.STATE_BOOTING);
        mUser0Allocations = DBG_ALLOCATION ? new AtomicInteger() : null;
        mUser0Allocations = DBG_ALLOCATION ? new AtomicInteger() : null;
        emulateSystemUserModeIfNeeded();
        emulateSystemUserModeIfNeeded();
        mUsersOnSecondaryDisplaysEnabled = UserManager.isUsersOnSecondaryDisplaysEnabled();
        mUsersOnSecondaryDisplays = mUsersOnSecondaryDisplaysEnabled ? new SparseIntArray() : null;
    }
    }


    void systemReady() {
    void systemReady() {
@@ -1720,15 +1725,15 @@ public class UserManagerService extends IUserManager.Stub {
            return true;
            return true;
        }
        }


        // TODO(b/239824814): STOPSHIP - add CTS tests (requires change on testing infra)
        // Device doesn't support multiple users on multiple displays, so only users checked above
        synchronized (mUsersLock) {
        // can be visible
            if (mUsersOnSecondaryDisplays != null) {
        if (!mUsersOnSecondaryDisplaysEnabled) {
                // TODO(b/239824814): make sure it handles profile as well
            return false;
                return (mUsersOnSecondaryDisplays.indexOfKey(userId) >= 0);
            }
        }
        }


        return false;
        synchronized (mUsersOnSecondaryDisplays) {
            return mUsersOnSecondaryDisplays.indexOfKey(userId) >= 0;
        }
    }
    }


    // TODO(b/239982558): add unit test
    // TODO(b/239982558): add unit test
@@ -1736,10 +1741,13 @@ public class UserManagerService extends IUserManager.Stub {
        if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) {
        if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) {
            return Display.DEFAULT_DISPLAY;
            return Display.DEFAULT_DISPLAY;
        }
        }
        synchronized (mUsersLock) {

            return mUsersOnSecondaryDisplays == null
        if (!mUsersOnSecondaryDisplaysEnabled) {
                    ? Display.INVALID_DISPLAY
            return Display.INVALID_DISPLAY;
                    : mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY);
        }

        synchronized (mUsersOnSecondaryDisplays) {
            return mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY);
        }
        }
    }
    }


@@ -1760,18 +1768,20 @@ public class UserManagerService extends IUserManager.Stub {
        return false;
        return false;
    }
    }


    // TODO(b/239982558): currently used just by shell command, might need to move to
    // TODO(b/239982558): try to merge with isUserVisibleUnchecked() (once both are unit tested)
    // UserManagerInternal if needed by other components (like WindowManagerService)
    // TODO(b/239982558): add unit test
    // TODO(b/239982558): try to merge with isUserVisibleUnchecked()
    boolean isUserVisibleOnDisplay(@UserIdInt int userId, int displayId) {
    boolean isUserVisibleOnDisplay(@UserIdInt int userId, int displayId) {
        if (displayId == Display.DEFAULT_DISPLAY) {
        if (displayId == Display.DEFAULT_DISPLAY) {
            return isCurrentUserOrRunningProfileOfCurrentUser(userId);
            return isCurrentUserOrRunningProfileOfCurrentUser(userId);
        }
        }
        synchronized (mUsersLock) {

            // TODO(b/239824814): make sure it handles profile as well
        // Device doesn't support multiple users on multiple displays, so only users checked above
            return mUsersOnSecondaryDisplays != null && mUsersOnSecondaryDisplays.get(userId,
        // can be visible
                    Display.INVALID_DISPLAY) == displayId;
        if (!mUsersOnSecondaryDisplaysEnabled) {
            return false;
        }

        synchronized (mUsersOnSecondaryDisplays) {
            return mUsersOnSecondaryDisplays.get(userId, Display.INVALID_DISPLAY) == displayId;
        }
        }
    }
    }


@@ -6054,11 +6064,14 @@ public class UserManagerService extends IUserManager.Stub {
        } // synchronized (mPackagesLock)
        } // synchronized (mPackagesLock)


        // Multiple Users on Multiple Display info
        // Multiple Users on Multiple Display info
        pw.println("  Supports users on secondary displays: "
        pw.println("  Supports users on secondary displays: " + mUsersOnSecondaryDisplaysEnabled);
        // mUsersOnSecondaryDisplaysEnabled is set on constructor, while the UserManager API is
        // set dynamically, so print both to help cases where the developer changed it on the fly
        pw.println("  UM.isUsersOnSecondaryDisplaysEnabled(): "
                + UserManager.isUsersOnSecondaryDisplaysEnabled());
                + UserManager.isUsersOnSecondaryDisplaysEnabled());
        synchronized (mUsersLock) {
        if (mUsersOnSecondaryDisplaysEnabled) {
            if (mUsersOnSecondaryDisplays != null) {
            pw.print("  Users on secondary displays: ");
            pw.print("  Users on secondary displays: ");
            synchronized (mUsersOnSecondaryDisplays) {
                pw.println(mUsersOnSecondaryDisplays);
                pw.println(mUsersOnSecondaryDisplays);
            }
            }
        }
        }
@@ -6628,44 +6641,32 @@ public class UserManagerService extends IUserManager.Stub {


        @Override
        @Override
        public void assignUserToDisplay(int userId, int displayId) {
        public void assignUserToDisplay(int userId, int displayId) {
            if (DBG) {
            if (DBG_MUMD) {
                Slogf.d(LOG_TAG, "assignUserToDisplay(%d, %d): mUsersOnSecondaryDisplays=%s",
                Slogf.d(LOG_TAG, "assignUserToDisplay(%d, %d)", userId, displayId);
                        userId, displayId, mUsersOnSecondaryDisplays);
            }
            }
            // TODO(b/240613396) throw exception if feature not supported


            if (displayId == Display.INVALID_DISPLAY) {
            if (displayId == Display.DEFAULT_DISPLAY) {
                synchronized (mUsersLock) {
                // Don't need to do anything because methods (such as isUserVisible()) already know
                    if (mUsersOnSecondaryDisplays == null) {
                // that the current user (and their profiles) is assigned to the default display.
                        if (false) {
                if (DBG_MUMD) {
                            // TODO(b/240613396): remove this if once we check for support above
                    Slogf.d(LOG_TAG, "ignoring on default display");
                            Slogf.wtf(LOG_TAG, "assignUserToDisplay(%d, %d): no "
                                    + "mUsersOnSecondaryDisplays", userId, displayId);
                        }
                        return;
                    }
                    if (DBG) {
                        Slogf.d(LOG_TAG, "Removing %d from mUsersOnSecondaryDisplays", userId);
                    }
                    mUsersOnSecondaryDisplays.delete(userId);
                    if (mUsersOnSecondaryDisplays.size() == 0) {
                        if (DBG) {
                            Slogf.d(LOG_TAG, "Removing mUsersOnSecondaryDisplays");
                        }
                        mUsersOnSecondaryDisplays = null;
                    }
                }
                }
                return;
                return;
            }
            }


            synchronized (mUsersLock) {
            if (!mUsersOnSecondaryDisplaysEnabled) {
                if (mUsersOnSecondaryDisplays == null) {
                throw new UnsupportedOperationException("assignUserToDisplay(" + userId + ", "
                    if (DBG) {
                        + displayId + ") called on device that doesn't support multiple "
                        Slogf.d(LOG_TAG, "Creating mUsersOnSecondaryDisplays");
                        + "users on multiple displays");
            }
            }
                    mUsersOnSecondaryDisplays = new SparseIntArray();

                }
            Preconditions.checkArgument(userId != UserHandle.USER_SYSTEM, "Cannot start system user"
                if (DBG) {
                    + " on secondary display (%d)", displayId);
            // TODO(b/239982558): call DisplayManagerInternal to check if display is valid instead
            Preconditions.checkArgument(displayId > 0, "Invalid display id (%d)", displayId);

            synchronized (mUsersOnSecondaryDisplays) {
                if (DBG_MUMD) {
                    Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays",
                    Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays",
                            userId, displayId);
                            userId, displayId);
                }
                }
@@ -6703,6 +6704,29 @@ public class UserManagerService extends IUserManager.Stub {
            }
            }
        }
        }


        @Override
        public void unassignUserFromDisplay(@UserIdInt int userId) {
            if (DBG_MUMD) {
                Slogf.d(LOG_TAG, "unassignUserFromDisplay(%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.
                if (DBG_MUMD) {
                    Slogf.d(LOG_TAG, "ignoring when device doesn't support MUMD");
                }
                return;
            }

            synchronized (mUsersOnSecondaryDisplays) {
                if (DBG_MUMD) {
                    Slogf.d(LOG_TAG, "Removing %d from mUsersOnSecondaryDisplays (%s)", userId,
                            mUsersOnSecondaryDisplays);
                }
                mUsersOnSecondaryDisplays.delete(userId);
            }
        }

        @Override
        @Override
        public boolean isUserVisible(int userId, int displayId) {
        public boolean isUserVisible(int userId, int displayId) {
            return isUserVisibleOnDisplay(userId, displayId);
            return isUserVisibleOnDisplay(userId, displayId);