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

Commit 537b09dd authored by Adam Bookatz's avatar Adam Bookatz
Browse files

LogoutUser relies on SYSTEM.supportsSwitchTo 

logoutUser always switches to the system user. All that therefore
matters is that the device can, in fact, switch to the system user. This
is irrespective of whether it is HSUM or not, so we don't need any
special-casing; we can just request whether we can switch to user 0.
This is a general policy: we avoid checking modes and instead check
properties - in this case, whether SYSTEM_USER has the property allowing
us to switch to it.

Also fixes some UserControllerTest logoutUser tests that weren't working
properly since they weren't ever actually switching users.

Test: UserControllerTest
Bug: 411696141
Bug: 428046912
Flag: EXEMPT trivial equivalent code
Change-Id: I14bd7e92679c51f799737ca22faecf77fc9039b7
parent e3ef8bea

core/java/android/content/pm/UserInfo.java

+2 −2
Original line number Diff line number Diff line
@@ -468,7 +468,7 @@ public class UserInfo implements Parcelable { 
            // Don't support switching to pre-created users until they become "real" users.

            return false;

        }

        return isFull() || canSwitchToHeadlessSystemUser();

        return isFull() || isSwitchableHeadlessSystemUser();

    }



    /**
@@ -476,7 +476,7 @@ public class UserInfo implements Parcelable { 
     * {@link com.android.internal.R.bool#config_canSwitchToHeadlessSystemUser} is true.

     */

    @android.ravenwood.annotation.RavenwoodThrow

    private boolean canSwitchToHeadlessSystemUser() {

    private boolean isSwitchableHeadlessSystemUser() {

        return UserManager.USER_TYPE_SYSTEM_HEADLESS.equals(userType) && Resources.getSystem()

                .getBoolean(com.android.internal.R.bool.config_canSwitchToHeadlessSystemUser);

    }

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

+6 −5
Original line number Diff line number Diff line
@@ -1134,10 +1134,7 @@ class UserController implements Handler.Callback { 
        // system user (not the previous foreground user). Thus we cannot support HSUM devices

        // without interactive headless system user. To solve it for the long term, a refactor might

        // be needed, see more details in the bug.

        // TODO(b/411696141): Use the more proper API to check if headless system user is

        // interactive, once it's ready.

        if (mInjector.isHeadlessSystemUserMode()

                && !mInjector.getUserManager().canSwitchToHeadlessSystemUser()) {

        if (!mInjector.doesUserSupportSwitchTo(getUserInfo(UserHandle.USER_SYSTEM))) {

            throw new UnsupportedOperationException("device does not support logoutUser");

        }

        boolean shouldSwitchUser = false;
@@ -2508,7 +2505,7 @@ class UserController implements Handler.Callback { 
                Slogf.w(TAG, "No user info for user #" + targetUserId);

                return false;

            }

            if (!targetUserInfo.supportsSwitchTo()) {

            if (!mInjector.doesUserSupportSwitchTo(targetUserInfo)) {

                Slogf.w(TAG, "Cannot switch to User #" + targetUserId + ": not supported");

                return false;

            }
@@ -4493,6 +4490,10 @@ class UserController implements Handler.Callback { 
            return UserManager.isHeadlessSystemUserMode();

        }



        boolean doesUserSupportSwitchTo(UserInfo user) {

            return user.supportsSwitchTo();

        }



        boolean isUsersOnSecondaryDisplaysEnabled() {

            return UserManager.isVisibleBackgroundUsersEnabled();

        }

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

+0 −1
Original line number Diff line number Diff line
@@ -8957,7 +8957,6 @@ public class UserManagerService extends IUserManager.Stub { 
            writeUserLP(userData);

            return true;

        }



    }



    /**

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

+61 −42
Original line number Diff line number Diff line
@@ -294,8 +294,6 @@ public class UserControllerTest { 


    @Test

    public void testStartUser_sendsNoBroadcastsForSystemUserInNonHeadlessMode() {

        setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,

                UserManager.USER_TYPE_FULL_SYSTEM);

        mockIsHeadlessSystemUserMode(false);



        mUserController.startUser(SYSTEM_USER_ID, USER_START_MODE_FOREGROUND);
@@ -306,8 +304,6 @@ public class UserControllerTest { 


    @Test

    public void testStartUser_sendsBroadcastsForSystemUserInHeadlessMode() {

        setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,

                UserManager.USER_TYPE_SYSTEM_HEADLESS);

        mockIsHeadlessSystemUserMode(true);



        mUserController.startUser(SYSTEM_USER_ID, USER_START_MODE_FOREGROUND);
@@ -536,14 +532,11 @@ public class UserControllerTest { 
        continueUserSwitchAssertions(oldUserId, TEST_USER_ID, false, false);

    }



    private void mockCanSwitchToHeadlessSystemUser(boolean canSwitch) {

        doReturn(canSwitch).when(mInjector.mUserManagerMock)

                .canSwitchToHeadlessSystemUser();

    }



    @Test

    public void testLogoutUserDuringSwitchToSameUser_nonHsum()

            throws InterruptedException {

        // TODO(b/428046912): This test doesn't actually test anything. The switch isn't sufficient,

        //  so the list of running users will never contain the test user anyway.

        mockIsHeadlessSystemUserMode(false);



        // Start user -- this will update state of mUserController
@@ -562,8 +555,9 @@ public class UserControllerTest { 
    @Test

    public void testLogoutUserDuringSwitchToSameUser_hsumAndInteractiveSystemUser()

            throws InterruptedException {

        mockIsHeadlessSystemUserMode(true);

        mockCanSwitchToHeadlessSystemUser(true);

        // TODO(b/428046912): This test doesn't actually test anything. The switch isn't sufficient,

        //  so the list of running users will never contain the test user anyway.

        mockIsSwitchableHeadlessSystemUserMode();



        // Start user -- this will update state of mUserController

        mUserController.startUser(TEST_USER_ID1, USER_START_MODE_FOREGROUND);
@@ -581,38 +575,47 @@ public class UserControllerTest { 
    @Test

    public void testLogoutUser_nonHsum() throws InterruptedException {

        mockIsHeadlessSystemUserMode(false);

        mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,

                /* maxRunningUsers= */ 3, /* delayUserDataLocking= */ false,

                /* backgroundUserScheduledStopTimeSecs= */ -1);



        // Start user -- this will update state of mUserController

        mUserController.switchUser(UserHandle.USER_SYSTEM);

        mUserController.switchUser(TEST_USER_ID);

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);

        // Switch to the test user.

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID, 1,

                /* expectOldUserStopping= */false,

                /* expectScheduleBackgroundUserStopping= */ false);

        assertTrue(mUserController.getRunningUsersLU().contains(TEST_USER_ID));



        // Logout user.

        // Logout the test user.

        mUserController.logoutUser(TEST_USER_ID);

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);



        // Verify that TEST_USER_ID is not running.

        List<Integer> runningUserIds = mUserController.getRunningUsersLU();

        assertFalse(runningUserIds.contains(TEST_USER_ID));

        // Verify that TEST_USER_ID is no longer running.

        assertFalse(mUserController.getRunningUsersLU().contains(TEST_USER_ID));

    }



    @Test

    public void testLogoutUser_hsumAndInteractiveSystemUser() throws InterruptedException {

        mockIsHeadlessSystemUserMode(true);

        mockCanSwitchToHeadlessSystemUser(true);

        mockIsSwitchableHeadlessSystemUserMode();

        mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,

                /* maxRunningUsers= */ 3, /* delayUserDataLocking= */ false,

                /* backgroundUserScheduledStopTimeSecs= */ -1);



        // Start user -- this will update state of mUserController

        mUserController.switchUser(UserHandle.USER_SYSTEM);

        mUserController.switchUser(TEST_USER_ID);

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);

        // Switch to the test user.

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID, 1,

                /* expectOldUserStopping= */false,

                /* expectScheduleBackgroundUserStopping= */ false);

        assertRunningUsersIgnoreOrder(SYSTEM_USER_ID, TEST_USER_ID);

        assertEquals("Unexpected current user", TEST_USER_ID, mUserController.getCurrentUserId());



        // Logout user.

        // Logout the test user.

        mUserController.logoutUser(TEST_USER_ID);

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);



        // Verify that TEST_USER_ID is not running.

        List<Integer> runningUserIds = mUserController.getRunningUsersLU();

        assertFalse(runningUserIds.contains(TEST_USER_ID));

        // Verify that TEST_USER_ID is no longer running.

        assertRunningUsersIgnoreOrder(SYSTEM_USER_ID);

        // Current policy is that interactive HSUM should specifically log out to the system user.

        assertEquals("Logout should always be to the system user in iHSUM",

                SYSTEM_USER_ID, mUserController.getCurrentOrTargetUserId());

    }



    private void continueUserSwitchAssertions(int expectedOldUserId, int expectedNewUserId,
@@ -677,7 +680,7 @@ public class UserControllerTest { 


        // Switch to TEST_USER_ID from user 0

        int numberOfUserSwitches = 0;

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,

                ++numberOfUserSwitches,

                /* expectOldUserStopping= */false,

                /* expectScheduleBackgroundUserStopping= */ false);
@@ -922,7 +925,7 @@ public class UserControllerTest { 


        // Switch to TEST_USER_ID from user 0

        int numberOfUserSwitches = 0;

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,

                ++numberOfUserSwitches, false,

                /* expectScheduleBackgroundUserStopping= */ false);

        assertRunningUsersInOrder(SYSTEM_USER_ID, TEST_USER_ID);
@@ -1057,7 +1060,7 @@ public class UserControllerTest { 
        setUpUser(TEST_USER_ID1, 0);

        setUpUser(TEST_USER_ID2, 0);

        int numberOfUserSwitches = 1;

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,

                numberOfUserSwitches, false, false);

        // running: user 0, USER_ID

        assertTrue(mUserController.canStartMoreUsers());
@@ -1099,7 +1102,7 @@ public class UserControllerTest { 
        setUpUser(TEST_USER_ID1, 0);

        setUpUser(TEST_USER_ID2, 0);

        int numberOfUserSwitches = 1;

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID, SYSTEM_USER_ID,

                numberOfUserSwitches, false, false);

        // running: user 0, USER_ID

        assertTrue(mUserController.canStartMoreUsers());
@@ -1197,7 +1200,7 @@ public class UserControllerTest { 
        assertRunningUsersInOrder(TEST_USER_ID1, SYSTEM_USER_ID);



        // Start a user in the foreground. This exceeds max. Make sure we cut down to 2 users.

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, SYSTEM_USER_ID,

                1, false, false);

        mUserController.finishUserSwitch(mUserStates.get(TEST_USER_ID2));

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1245,7 +1248,7 @@ public class UserControllerTest { 


        assertRunningUsersInOrder(SYSTEM_USER_ID);



        addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, SYSTEM_USER_ID,

                1, false, false);

        mUserController.finishUserSwitch(mUserStates.get(TEST_USER_ID1));

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1301,7 +1304,7 @@ public class UserControllerTest { 
        assertRunningUsersIgnoreOrder(BG_USER_ID, SYSTEM_USER_ID);



        // Start PARENT_ID

        addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, 1, false, false);

        addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID, 1, false, false);

        // We call startProfiles(), but the profiles were configured to not auto-start.

        mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); // Calls startProfiles()

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1355,7 +1358,7 @@ public class UserControllerTest { 
        assertRunningUsersIgnoreOrder(BG_USER_ID, SYSTEM_USER_ID);



        // Start PARENT_ID

        addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM, 1, false, false);

        addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID, 1, false, false);

        // We call startProfiles(), which auto-starts the profiles.

        mUserController.finishUserSwitch(mUserStates.get(PARENT_ID)); // Calls startProfiles()

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1393,7 +1396,7 @@ public class UserControllerTest { 
        assertRunningUsersInOrder(SYSTEM_USER_ID);



        int numberOfUserSwitches = 1;

        addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID,

                numberOfUserSwitches, false, false);

        mUserController.finishUserSwitch(mUserStates.get(PARENT_ID));

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);
@@ -1515,7 +1518,7 @@ public class UserControllerTest { 
        assertRunningUsersInOrder(SYSTEM_USER_ID);



        int numberOfUserSwitches = 1;

        addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,

        addForegroundUserAndContinueUserSwitch(PARENT_ID, SYSTEM_USER_ID,

                numberOfUserSwitches, false, false);

        assertRunningUsersInOrder(SYSTEM_USER_ID, PARENT_ID);
@@ -1561,7 +1564,7 @@ public class UserControllerTest { 


        assertRunningUsersInOrder(SYSTEM_USER_ID);



        addForegroundUserAndContinueUserSwitch(CURRENT_ID, UserHandle.USER_SYSTEM, 1, false, false);

        addForegroundUserAndContinueUserSwitch(CURRENT_ID, SYSTEM_USER_ID, 1, false, false);

        assertRunningUsersInOrder(SYSTEM_USER_ID, CURRENT_ID);



        mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND);
@@ -2178,8 +2181,24 @@ public class UserControllerTest { 
        }

    }



    private void mockIsHeadlessSystemUserMode(boolean value) {

        when(mInjector.isHeadlessSystemUserMode()).thenReturn(value);

    /** Specify whether to mock the device as being in regular (non-switchable) HSUM. */

    private void mockIsHeadlessSystemUserMode(boolean isHsum) {

        mockIsHeadlessSystemUserMode(isHsum, false);

    }



    /** Mocks the device as being in interactive (switchable) HSUM. */

    private void mockIsSwitchableHeadlessSystemUserMode() {

        mockIsHeadlessSystemUserMode(true, true);

    }



    private void mockIsHeadlessSystemUserMode(boolean isHsum, boolean canSwitch) {

        when(mInjector.isHeadlessSystemUserMode()).thenReturn(isHsum);

        UserInfo sysInfo = setUpUser(SYSTEM_USER_ID, UserInfo.FLAG_SYSTEM, /* preCreated= */ false,

                isHsum ? UserManager.USER_TYPE_SYSTEM_HEADLESS : UserManager.USER_TYPE_FULL_SYSTEM);

        if (isHsum) {

            doReturn(canSwitch).when(mInjector.mUserManagerMock).canSwitchToHeadlessSystemUser();

            doReturn(canSwitch).when(mInjector).doesUserSupportSwitchTo(eq(sysInfo));

        }

    }



    private void mockIsUsersOnSecondaryDisplaysEnabled(boolean value) {