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

Commit 5a82b5ae authored by Felipe Leme's avatar Felipe Leme
Browse files

Further improvements on isUserVisibleOnDisplay(userId, displayId). 

Commit d4c41e18ca3b4767907ca6ca57ce8341cdae9f25 changed this method
behavior to always return true to the current user (and their
profiles); that should be the case on most cases, except when
the display is assigned to another user (i.e., a background user
that was started on that display).

This CL partially fixes that issue - it only returns true for the
current / user profile when a display is not assigned to another
user. It's not the ideal solution yet, because it would still fail
in 2 scenarios:

- It would allow the current user to be visible in a secondary
  display that is not being used by a passenger yet.
- It would not handle the case where the a virtual display should
  be visible only to the user that created it.

This CL also:

* Changed it to always return false for INVALID_DISPLAY
* Changed the is-user-visible command to allow INVALID_DISPLAY.
* Fixed startUserInternal() so assignUserToDisplay() is only called
  after checking if the user exists.

Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest UserManagerInternalTest
Test: adb shell am start-user --display 42 13 && \
      adb shell cmd user is-user-visible --display 42 0 &&
      adb shell cmd user is-user-visible --display -1 0

Bug: 244644281
Change-Id: I2803e0b5d49b8a729266e534bbe203e346720417
parent c1527fea

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

+2 −2
Original line number Diff line number Diff line
@@ -1514,8 +1514,6 @@ class UserController implements Handler.Callback { 
            Preconditions.checkArgument(!foreground, "Cannot start user %d in foreground AND "

                    + "on secondary display (%d)", 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);
@@ -1571,6 +1569,8 @@ class UserController implements Handler.Callback { 
                return false;

            }



            mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId);



            // TODO(b/239982558): might need something similar for bg users on secondary display

            if (foreground && isUserSwitchUiEnabled()) {

                t.traceBegin("startFreezingScreen");

services/core/java/com/android/server/pm/UserManagerService.java

+37 −11
Original line number Diff line number Diff line
@@ -1862,22 +1862,48 @@ public class UserManagerService extends IUserManager.Stub { 
    }



    // TODO(b/239982558): try to merge with isUserVisibleUnchecked() (once both are unit tested)

    /**

     * See {@link UserManagerInternal#isUserVisible(int, int)}.

     */

    boolean isUserVisibleOnDisplay(@UserIdInt int userId, int displayId) {

        // TODO(b/244644281): temporary workaround to let WM use this API without breaking current

        // behavior (otherwise current user / profiles wouldn't be able to launch activities on

        // other non-passenger displays, like cluster, display, or virtual displays)

        if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) {

            return true;

        if (displayId == Display.INVALID_DISPLAY) {

            return false;

        }



        if (displayId == Display.DEFAULT_DISPLAY) {

        if (!mUsersOnSecondaryDisplaysEnabled) {

            return isCurrentUserOrRunningProfileOfCurrentUser(userId);

        }



        // Device doesn't support multiple users on multiple displays, so only users checked above

        // can be visible

        if (!mUsersOnSecondaryDisplaysEnabled) {

            return false;

        // TODO(b/244644281): temporary workaround to let WM use this API without breaking current

        // behavior - return true for current user / profile for any display (other than those

        // explicitly assigned to another users), otherwise they wouldn't be able to launch

        // activities on other non-passenger displays, like cluster, display, or virtual displays).

        // In the long-term, it should rely just on mUsersOnSecondaryDisplays, which

        // would be updated by DisplayManagerService when displays are created / initialized.

        if (isCurrentUserOrRunningProfileOfCurrentUser(userId)) {

            synchronized (mUsersOnSecondaryDisplays) {

                boolean assignedToUser = false;

                boolean assignedToAnotherUser = false;

                for (int i = 0; i < mUsersOnSecondaryDisplays.size(); i++) {

                    if (mUsersOnSecondaryDisplays.valueAt(i) == displayId) {

                        if (mUsersOnSecondaryDisplays.keyAt(i) == userId) {

                            assignedToUser = true;

                            break;

                        } else {

                            assignedToAnotherUser = true;

                            // Cannot break because it could be assigned to a profile of the user

                            // (and we better not assume that the iteration will check for the

                            // parent user before its profiles)

                        }

                    }

                }

                if (DBG_MUMD) {

                    Slogf.d(LOG_TAG, "isUserVisibleOnDisplay(%d, %d): assignedToUser=%b, "

                            + "assignedToAnotherUser=%b, mUsersOnSecondaryDisplays=%s",

                            userId, displayId, assignedToUser, assignedToAnotherUser,

                            mUsersOnSecondaryDisplays);

                }

                return assignedToUser || !assignedToAnotherUser;

            }

        }



        synchronized (mUsersOnSecondaryDisplays) {

services/core/java/com/android/server/pm/UserManagerServiceShellCommand.java

+4 −10
Original line number Diff line number Diff line
@@ -35,7 +35,6 @@ import android.os.UserHandle; 
import android.os.UserManager;

import android.util.IndentingPrintWriter;

import android.util.Slog;

import android.view.Display;



import com.android.internal.widget.LockPatternUtils;

import com.android.server.LocalServices;
@@ -374,19 +373,14 @@ public class UserManagerServiceShellCommand extends ShellCommand { 
    })

    private int runIsUserVisible() {

        PrintWriter pw = getOutPrintWriter();

        int displayId = Display.INVALID_DISPLAY;

        Integer displayId = null;

        String opt;

        while ((opt = getNextOption()) != null) {

            boolean invalidOption = false;

            switch (opt) {

                case "--display":

                    displayId = Integer.parseInt(getNextArgRequired());

                    invalidOption = displayId == Display.INVALID_DISPLAY;

                    break;

                default:

                    invalidOption = true;

            }

            if (invalidOption) {

                    pw.println("Invalid option: " + opt);

                    return -1;

            }
@@ -404,7 +398,7 @@ public class UserManagerServiceShellCommand extends ShellCommand { 
        }



        boolean isVisible;

        if (displayId != Display.INVALID_DISPLAY) {

        if (displayId != null) {

            isVisible = mService.isUserVisibleOnDisplay(userId, displayId);

        } else {

            isVisible = getUserManagerForUser(userId).isUserVisible();

services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceOrInternalTestCase.java

+61 −2
Original line number Diff line number Diff line
@@ -101,6 +101,11 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC 
     */

    protected static final int OTHER_SECONDARY_DISPLAY_ID = 108;



    /**

     * Id of another secondary display (i.e, not {@link android.view.Display.DEFAULT_DISPLAY}).

     */

    private static final int ANOTHER_SECONDARY_DISPLAY_ID = 108;



    private final Object mPackagesLock = new Object();

    private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation()

            .getTargetContext();
@@ -267,7 +272,7 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC 
        mockCurrentUser(USER_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", USER_ID, INVALID_DISPLAY)

                .that(isUserVisibleOnDisplay(USER_ID, INVALID_DISPLAY)).isTrue();

                .that(isUserVisibleOnDisplay(USER_ID, INVALID_DISPLAY)).isFalse();

    }



    @Test
@@ -286,6 +291,49 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC 
                .that(isUserVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID)).isTrue();

    }



    @Test

    public void testIsUserVisibleOnDisplay_mumd_currentUserUnassignedSecondaryDisplay() {

        enableUsersOnSecondaryDisplays();

        mockCurrentUser(USER_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", USER_ID, SECONDARY_DISPLAY_ID)

                .that(isUserVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID)).isTrue();

    }



    @Test

    public void testIsUserVisibleOnDisplay_mumd_currentUserSecondaryDisplayAssignedToAnotherUser() {

        enableUsersOnSecondaryDisplays();

        mockCurrentUser(USER_ID);

        assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", USER_ID, SECONDARY_DISPLAY_ID)

                .that(isUserVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID)).isFalse();

    }



    @Test

    public void testIsUserVisibleOnDisplay_mumd_startedProfileOfCurrentUserSecondaryDisplayAssignedToAnotherUser() {

        enableUsersOnSecondaryDisplays();

        addDefaultProfileAndParent();

        startDefaultProfile();

        mockCurrentUser(PARENT_USER_ID);

        assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", PROFILE_USER_ID, SECONDARY_DISPLAY_ID)

                .that(isUserVisibleOnDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)).isFalse();

    }



    @Test

    public void testIsUserVisibleOnDisplay_mumd_stoppedProfileOfCurrentUserSecondaryDisplayAssignedToAnotherUser() {

        enableUsersOnSecondaryDisplays();

        addDefaultProfileAndParent();

        stopDefaultProfile();

        mockCurrentUser(PARENT_USER_ID);

        assignUserToDisplay(OTHER_USER_ID, SECONDARY_DISPLAY_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", PROFILE_USER_ID, SECONDARY_DISPLAY_ID)

                .that(isUserVisibleOnDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID)).isFalse();

    }



    @Test

    public void testIsUserVisibleOnDisplay_nonCurrentUserDefaultDisplay() {

        mockCurrentUser(OTHER_USER_ID);
@@ -358,7 +406,7 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC 
    }



    @Test

    public void testIsUserVisibleOnDisplay_bgUserOnSecondaryDisplay() {

    public void testIsUserVisibleOnDisplay_mumd_bgUserOnSecondaryDisplay() {

        enableUsersOnSecondaryDisplays();

        mockCurrentUser(OTHER_USER_ID);

        assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID);
@@ -367,6 +415,17 @@ abstract class UserManagerServiceOrInternalTestCase extends ExtendedMockitoTestC 
                .that(isUserVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID)).isTrue();

    }



    @Test

    public void testIsUserVisibleOnDisplay_mumd_bgUserOnAnotherSecondaryDisplay() {

        enableUsersOnSecondaryDisplays();

        mockCurrentUser(OTHER_USER_ID);

        assignUserToDisplay(USER_ID, SECONDARY_DISPLAY_ID);



        assertWithMessage("isUserVisibleOnDisplay(%s, %s)", USER_ID, SECONDARY_DISPLAY_ID)

                .that(isUserVisibleOnDisplay(USER_ID, ANOTHER_SECONDARY_DISPLAY_ID)).isFalse();

    }





    // NOTE: we don't need to add tests for profiles (started / stopped profiles of bg user), as

    // isUserVisibleOnDisplay() for bg users relies only on the user / display assignments