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

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

Refactored UMS.getVisibleUsers() to use UserVisibilityMediator. 

Test: atest UserVisibilityMediatorMUMDTest UserVisibilityMediatorSUSDTest # unit
Test: atest MultipleUsersOnMultipleDisplaysTest CtsMultiUserTestCases:android.multiuser.cts.UserManagerTest # CTS

Fixes: 257497221
Change-Id: If3cc5e6d0e56ca3c6e1642e92b18bd9e226bf2e3
parent 7a0ddc07

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

+1 −14
Original line number Diff line number Diff line
@@ -1788,20 +1788,7 @@ public class UserManagerService extends IUserManager.Stub { 
        }

        final long ident = Binder.clearCallingIdentity();

        try {

            // TODO(b/2399825580): refactor into UserDisplayAssigner

            IntArray visibleUsers;

            synchronized (mUsersLock) {

                int usersSize = mUsers.size();

                visibleUsers = new IntArray();

                for (int i = 0; i < usersSize; i++) {

                    UserInfo ui = mUsers.valueAt(i).info;

                    if (!ui.partial && !ui.preCreated && !mRemovingUserIds.get(ui.id)

                            && mUserVisibilityMediator.isUserVisible(ui.id)) {

                        visibleUsers.add(ui.id);

                    }

                }

            }

            return visibleUsers.toArray();

            return mUserVisibilityMediator.getVisibleUsers().toArray();

        } finally {

            Binder.restoreCallingIdentity(ident);

        }

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

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

import android.util.Dumpable;

import android.util.IndentingPrintWriter;

import android.util.IntArray;

import android.util.SparseIntArray;

import android.view.Display;
@@ -117,6 +118,9 @@ public final class UserVisibilityMediator implements Dumpable { 
    UserVisibilityMediator(boolean usersOnSecondaryDisplaysEnabled) {

        mUsersOnSecondaryDisplaysEnabled = usersOnSecondaryDisplaysEnabled;

        mUsersOnSecondaryDisplays = mUsersOnSecondaryDisplaysEnabled ? new SparseIntArray() : null;



        // TODO(b/242195409): might need to change this if boot logic is refactored for HSUM devices

        mStartedProfileGroupIds.put(INITIAL_CURRENT_USER_ID, INITIAL_CURRENT_USER_ID);

    }



    /**
@@ -458,6 +462,25 @@ public final class UserVisibilityMediator implements Dumpable { 
        return currentUserId;

    }



    /**

     * Gets the ids of the visible users.

     */

    public IntArray getVisibleUsers() {

        // TODO(b/258054362): this method's performance is O(n2), as it interacts through all users

        // here, then again on isUserVisible(). We could "fix" it to be O(n), but given that the

        // number of users is too small, the gain is probably not worth the increase on complexity.

        IntArray visibleUsers = new IntArray();

        synchronized (mLock) {

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

                int userId = mStartedProfileGroupIds.keyAt(i);

                if (isUserVisible(userId)) {

                    visibleUsers.add(userId);

                }

            }

        }

        return visibleUsers;

    }



    private void dump(IndentingPrintWriter ipw) {

        ipw.println("UserVisibilityMediator");

        ipw.increaseIndent();
@@ -466,21 +489,27 @@ public final class UserVisibilityMediator implements Dumpable { 
            ipw.print("Current user id: ");

            ipw.println(mCurrentUserId);



            dumpIntArray(ipw, mStartedProfileGroupIds, "started user / profile group", "u", "pg");

            ipw.print("Visible users: ");

            // TODO: merge 2 lines below if/when IntArray implements toString()...

            IntArray visibleUsers = getVisibleUsers();

            ipw.println(java.util.Arrays.toString(visibleUsers.toArray()));



            dumpSparseIntArray(ipw, mStartedProfileGroupIds, "started user / profile group",

                    "u", "pg");



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

            ipw.println(mUsersOnSecondaryDisplaysEnabled);



            if (mUsersOnSecondaryDisplays != null) {

                dumpIntArray(ipw, mUsersOnSecondaryDisplays, "background user / secondary display",

                        "u", "d");

                dumpSparseIntArray(ipw, mUsersOnSecondaryDisplays,

                        "background user / secondary display", "u", "d");

            }

        }



        ipw.decreaseIndent();

    }



    private static void dumpIntArray(IndentingPrintWriter ipw, SparseIntArray array,

    private static void dumpSparseIntArray(IndentingPrintWriter ipw, SparseIntArray array,

            String arrayDescription, String keyName, String valueName) {

        ipw.print("Number of ");

        ipw.print(arrayDescription);

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

+3 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import static android.view.Display.INVALID_DISPLAY; 
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.UserVisibilityMediator.INITIAL_CURRENT_USER_ID;



import org.junit.Test;
@@ -63,6 +64,7 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator 
        expectUserIsVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID);

        expectUserIsNotVisibleOnDisplay(USER_ID, INVALID_DISPLAY);

        expectUserIsNotVisibleOnDisplay(USER_ID, DEFAULT_DISPLAY);

        expectVisibleUsers(INITIAL_CURRENT_USER_ID, USER_ID);



        expectDisplayAssignedToUser(USER_ID, SECONDARY_DISPLAY_ID);

        expectUserAssignedToDisplay(SECONDARY_DISPLAY_ID, USER_ID);
@@ -110,6 +112,7 @@ public final class UserVisibilityMediatorMUMDTest extends UserVisibilityMediator 
        expectUserIsNotVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID);

        expectUserIsNotVisibleOnDisplay(USER_ID, INVALID_DISPLAY);

        expectUserIsNotVisibleOnDisplay(USER_ID, DEFAULT_DISPLAY);

        expectVisibleUsers(INITIAL_CURRENT_USER_ID, USER_ID);



        expectDisplayAssignedToUser(USER_ID, OTHER_SECONDARY_DISPLAY_ID);

        expectUserAssignedToDisplay(OTHER_SECONDARY_DISPLAY_ID, USER_ID);

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

+13 −0
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import static com.google.common.truth.Truth.assertWithMessage; 
import static org.junit.Assert.assertThrows;



import android.annotation.UserIdInt;

import android.util.IntArray;

import android.util.Log;



import com.android.internal.util.Preconditions;
@@ -42,6 +43,8 @@ import com.android.server.ExtendedMockitoTestCase; 
import org.junit.Before;

import org.junit.Test;



import java.util.Arrays;



/**

 * Base class for {@link UserVisibilityMediator} tests.

 *
@@ -133,6 +136,7 @@ abstract class UserVisibilityMediatorTestCase extends ExtendedMockitoTestCase { 
        // TODO(b/244644281): once isUserVisible() is fixed (see note there), this assertion will

        // fail on MUMD, so we'll need to refactor / split this test (and possibly others)

        expectUserIsVisibleOnDisplay(USER_ID, SECONDARY_DISPLAY_ID);

        expectVisibleUsers(USER_ID);



        expectDisplayAssignedToUser(USER_ID, DEFAULT_DISPLAY);

        expectUserAssignedToDisplay(DEFAULT_DISPLAY, USER_ID);
@@ -156,6 +160,7 @@ abstract class UserVisibilityMediatorTestCase extends ExtendedMockitoTestCase { 
        expectUserIsNotVisibleOnDisplay(currentUserId, INVALID_DISPLAY);

        expectUserIsVisibleOnDisplay(currentUserId, DEFAULT_DISPLAY);

        expectUserIsVisibleOnDisplay(currentUserId, SECONDARY_DISPLAY_ID);

        expectVisibleUsers(currentUserId);



        expectDisplayAssignedToUser(currentUserId, DEFAULT_DISPLAY);

        expectUserAssignedToDisplay(DEFAULT_DISPLAY, currentUserId);
@@ -216,6 +221,7 @@ abstract class UserVisibilityMediatorTestCase extends ExtendedMockitoTestCase { 
        expectUserIsNotVisibleOnDisplay(PROFILE_USER_ID, INVALID_DISPLAY);

        expectUserIsVisibleOnDisplay(PROFILE_USER_ID, DEFAULT_DISPLAY);

        expectUserIsVisibleOnDisplay(PROFILE_USER_ID, SECONDARY_DISPLAY_ID);

        expectVisibleUsers(PARENT_USER_ID, PROFILE_USER_ID);



        expectDisplayAssignedToUser(PROFILE_USER_ID, DEFAULT_DISPLAY);

        expectUserAssignedToDisplay(DEFAULT_DISPLAY, PARENT_USER_ID);
@@ -404,6 +410,13 @@ abstract class UserVisibilityMediatorTestCase extends ExtendedMockitoTestCase { 
                .isTrue();

    }



    protected void expectVisibleUsers(@UserIdInt Integer... userIds) {

        IntArray visibleUsers = mMediator.getVisibleUsers();

        expectWithMessage("getVisibleUsers()").that(visibleUsers).isNotNull();

        expectWithMessage("getVisibleUsers()").that(visibleUsers.toArray()).asList()

                .containsExactlyElementsIn(Arrays.asList(userIds));

    }



    protected void expectUserIsVisibleOnDisplay(@UserIdInt int userId, int displayId) {

        expectWithMessage("mediator.isUserVisible(%s, %s)", userId, displayId)

                .that(mMediator.isUserVisible(userId, displayId))