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

Commit dcca5d8b authored by Adam Bookatz's avatar Adam Bookatz
Browse files

Don't stop current user's profile even if excess 

In Ie08342bb77544117cab4423b543af74cbeb109e2 we fixed some bugs relating
to stopping profiles' parents. However, this could cause the "force"
parameter to not be handled properly. We almost never actually want it,
but:

The case of stopping users when exceeding the maximum running users
is one of the few cases where we want "force==false" to truly not
stop a profile if its parent is the current user. As a result, we cannot
simply ignore the notion of "force".
We need to really refactor this notion of "force", but in the meantime,
here's a quick fix and test.

Test: atest UserControllerTest#testStoppingExcessRunningUsersAfterSwitch_currentProfileNotStopped
Bug: 324647580
Bug: 310249114
Change-Id: I4f9c5a11908fd9f7ad381d337d11af5c3058bc3d
parent 3c1f216e

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

+7 −0
Original line number Diff line number Diff line
@@ -991,6 +991,13 @@ class UserController implements Handler.Callback { 
        if (isCurrentUserLU(userId)) {

            return USER_OP_IS_CURRENT;

        }

        // TODO(b/324647580): Refactor the idea of "force" and clean up. In the meantime...

        final int parentId = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID);

        if (parentId != UserInfo.NO_PROFILE_GROUP_ID && parentId != userId) {

            if ((UserHandle.USER_SYSTEM == parentId || isCurrentUserLU(parentId)) && !force) {

                return USER_OP_ERROR_RELATED_USERS_CANNOT_STOP;

            }

        }

        TimingsTraceAndSlog t = new TimingsTraceAndSlog();

        int[] usersToStop = getUsersToStopLU(userId);

        // If one of related users is system or current, no related users should be stopped

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

+82 −0
Original line number Diff line number Diff line
@@ -123,6 +123,7 @@ import java.util.ArrayList; 
import java.util.Arrays;

import java.util.Collections;

import java.util.HashMap;

import java.util.HashSet;

import java.util.LinkedHashSet;

import java.util.List;

import java.util.Set;
@@ -677,6 +678,87 @@ public class UserControllerTest { 
                .lockCeStorage(TEST_USER_ID);

    }



    /**

     * Test that, when exceeding the maximum number of running users, a profile of the current user

     * is not stopped.

     */

    @Test

    public void testStoppingExcessRunningUsersAfterSwitch_currentProfileNotStopped()

            throws Exception {

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

                /* maxRunningUsers= */ 5, /* delayUserDataLocking= */ false);



        final int PARENT_ID = 200;

        final int PROFILE1_ID = 201;

        final int PROFILE2_ID = 202;

        final int FG_USER_ID = 300;

        final int BG_USER_ID = 400;



        setUpUser(PARENT_ID, 0).profileGroupId = PARENT_ID;

        setUpUser(PROFILE1_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID;

        setUpUser(PROFILE2_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID;

        setUpUser(FG_USER_ID, 0).profileGroupId = FG_USER_ID;

        setUpUser(BG_USER_ID, 0).profileGroupId = UserInfo.NO_PROFILE_GROUP_ID;

        mUserController.onSystemReady(); // To set the profileGroupIds in UserController.



        assertEquals(newHashSet(

                SYSTEM_USER_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));



        int numberOfUserSwitches = 1;

        addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,

                numberOfUserSwitches, false);

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

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);

        assertTrue(mUserController.canStartMoreUsers());

        assertEquals(newHashSet(

                SYSTEM_USER_ID, PARENT_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));



        assertThat(mUserController.startProfile(PROFILE1_ID, true, null)).isTrue();

        assertEquals(newHashSet(

                SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));



        numberOfUserSwitches++;

        addForegroundUserAndContinueUserSwitch(FG_USER_ID, PARENT_ID,

                numberOfUserSwitches, false);

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

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);

        assertTrue(mUserController.canStartMoreUsers());

        assertEquals(newHashSet(

                SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, FG_USER_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));



        mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND);

        assertEquals(newHashSet(

                SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, BG_USER_ID, FG_USER_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));



        // Now we exceed the maxRunningUsers parameter (of 5):

        assertThat(mUserController.startProfile(PROFILE2_ID, true, null)).isTrue();

        // Currently, starting a profile doesn't trigger evaluating whether we've exceeded max, so

        // we expect no users to be stopped. This policy may change in the future. Log but no fail.

        if (!newHashSet(SYSTEM_USER_ID, PROFILE1_ID, BG_USER_ID, PROFILE2_ID, PARENT_ID, FG_USER_ID)

                .equals(new HashSet<>(mUserController.getRunningUsersLU()))) {

            Log.w(TAG, "Starting a profile that exceeded max running users didn't lead to "

                    + "expectations: " + mUserController.getRunningUsersLU());

        }



        numberOfUserSwitches++;

        addForegroundUserAndContinueUserSwitch(PARENT_ID, FG_USER_ID,

                numberOfUserSwitches, false);

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

        waitForHandlerToComplete(mInjector.mHandler, HANDLER_WAIT_TIME_MS);

        // We've now done a user switch and should notice that we've exceeded the maximum number of

        // users. The oldest background user should be stopped (BG_USER); even though PROFILE1 was

        // older, it should not be stopped since it's a profile of the (new) current user.

        assertFalse(mUserController.canStartMoreUsers());

        assertEquals(newHashSet(

                SYSTEM_USER_ID, PROFILE1_ID, PROFILE2_ID, FG_USER_ID, PARENT_ID),

                new HashSet<>(mUserController.getRunningUsersLU()));

    }



    /**

     * Test that, in getRunningUsersLU, parents come after their profile, even if the profile was

     * started afterwards.