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

Commit 6495efaa authored by Ben Reich's avatar Ben Reich
Browse files

Don't use isSystemUser for UserItemsCombiner 

For devices which don't have an actual user as the system user, this
re-orders the user items incorrectly. Check the managed profile instead
of using the isSystemUser.

Bug: 408066117
Test: atest com.android.documentsui.sidebar.UserItemsCombinerTest
Flag: EXEMPT but fix
Change-Id: Iab80ccad4f7ba5218a23eccb38e75e2834f22bae
parent 3c82e697

src/com/android/documentsui/sidebar/RootsFragment.java

+11 −4
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.graphics.Color; 
import android.graphics.drawable.ColorDrawable;

import android.os.Build;

import android.os.Bundle;

import android.os.UserManager;

import android.os.ext.SdkExtensions;

import android.provider.DocumentsContract;

import android.provider.MediaStore;
@@ -571,16 +572,22 @@ public class RootsFragment extends Fragment { 
    private List<Item> getPresentableListPrivateSpaceEnabled(Context context, State state,

            List<List<Item>> rootListAllUsers, List<UserId> userIds,

            UserManagerState userManagerState) {

        return new UserItemsCombiner(context.getResources(),

                context.getSystemService(DevicePolicyManager.class), state)

        return new UserItemsCombiner(

                        context.getResources(),

                        context.getSystemService(UserManager.class),

                        context.getSystemService(DevicePolicyManager.class),

                        state)

                .setRootListForAllUsers(rootListAllUsers)

                .createPresentableListForAllUsers(userIds, userManagerState.getUserIdToLabelMap());

    }



    private List<Item> getPresentableListPrivateSpaceDisabled(Context context, State state,

            List<Item> rootList, List<Item> rootListOtherUser) {

        return new UserItemsCombiner(context.getResources(),

                context.getSystemService(DevicePolicyManager.class), state)

        return new UserItemsCombiner(

                        context.getResources(),

                        context.getSystemService(UserManager.class),

                        context.getSystemService(DevicePolicyManager.class),

                        state)

                .setRootListForCurrentUser(rootList)

                .setRootListForOtherUser(rootListOtherUser)

                .createPresentableList();

src/com/android/documentsui/sidebar/UserItemsCombiner.java

+10 −5
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import static com.android.documentsui.DevicePolicyResources.Strings.WORK_TAB; 
import android.app.admin.DevicePolicyManager;

import android.content.res.Resources;

import android.os.Build;

import android.os.UserManager;



import androidx.annotation.RequiresApi;

import androidx.annotation.VisibleForTesting;
@@ -46,14 +47,17 @@ class UserItemsCombiner { 


    private UserId mCurrentUser;

    private final Resources mResources;

    private final UserManager mUserManager;

    private final DevicePolicyManager mDpm;

    private final State mState;

    private List<Item> mRootList;

    private List<Item> mRootListOtherUser;

    private List<List<Item>> mRootListAllUsers;



    UserItemsCombiner(Resources resources, DevicePolicyManager dpm, State state) {

    UserItemsCombiner(

            Resources resources, UserManager userManager, DevicePolicyManager dpm, State state) {

        mCurrentUser = UserId.CURRENT_USER;

        mUserManager = userManager;

        mResources = checkNotNull(resources);

        mDpm = dpm;

        mState = checkNotNull(state);
@@ -94,12 +98,13 @@ class UserItemsCombiner { 
                // Identify personal and work root list.

                final List<Item> personalRootList;

                final List<Item> workRootList;

                if (mCurrentUser.isSystem()) {

                    personalRootList = mRootList;

                    workRootList = mRootListOtherUser;

                } else {



                if (mCurrentUser.isManagedProfile(mUserManager)) {

                    personalRootList = mRootListOtherUser;

                    workRootList = mRootList;

                } else {

                    personalRootList = mRootList;

                    workRootList = mRootListOtherUser;

                }

                result.add(new HeaderItem(getEnterpriseString(

                        PERSONAL_TAB, R.string.personal_tab)));

tests/unit/com/android/documentsui/sidebar/UserItemsCombinerTest.java

+91 −63
Original line number Diff line number Diff line
@@ -18,8 +18,12 @@ package com.android.documentsui.sidebar; 


import static com.google.common.truth.Truth.assertThat;



import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;



import android.app.admin.DevicePolicyManager;

import android.content.res.Resources;

import android.os.UserManager;

import android.view.View;



import androidx.test.filters.MediumTest;
@@ -80,6 +84,7 @@ public class UserItemsCombinerTest { 
    private final State mState = new State();

    private final Resources mResources =

            InstrumentationRegistry.getInstrumentation().getTargetContext().getResources();

    private final UserManager mMockUserManager = mock(UserManager.class);

    private final DevicePolicyManager mDpm =

            InstrumentationRegistry.getInstrumentation().getTargetContext().getSystemService(

                    DevicePolicyManager.class);
@@ -106,11 +111,14 @@ public class UserItemsCombinerTest { 
            mState.canForwardToProfileIdMap.put(PRIVATE_USER, true);

            mTestConfigStore.enablePrivateSpaceInPhotoPicker();

        }



        when(mMockUserManager.isManagedProfile(WORK_USER.getIdentifier())).thenReturn(true);

    }



    @Test

    public void testCreatePresentableList_empty() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForCurrentUser(Collections.emptyList())

                        .setRootListForOtherUser(Collections.emptyList());

        assertThat(mCombiner.createPresentableList()).isEmpty();
@@ -124,7 +132,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(Collections.emptyList());

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(

                mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap)).isEmpty();
@@ -132,7 +141,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsPersonal_personalItemsOnly() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForCurrentUser(PERSONAL_ITEMS)

                        .setRootListForOtherUser(Collections.emptyList());

        assertThat(mCombiner.createPresentableList())
@@ -143,7 +153,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsWork_personalItemsOnly() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForCurrentUser(Collections.emptyList())

                        .setRootListForOtherUser(PERSONAL_ITEMS);
@@ -161,7 +172,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(Collections.emptyList());

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))

                .comparingElementsUsing(ITEM_CORRESPONDENCE)
@@ -177,7 +189,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(Collections.emptyList());

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))
@@ -193,7 +206,8 @@ public class UserItemsCombinerTest { 
        rootListAllUsers.add(Lists.newArrayList(PERSONAL_ITEMS));

        rootListAllUsers.add(Collections.emptyList());

        rootListAllUsers.add(Collections.emptyList());

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(PRIVATE_USER)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))
@@ -204,7 +218,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsPersonal_workItemsOnly() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForCurrentUser(Collections.emptyList())

                        .setRootListForOtherUser(WORK_ITEMS);

        assertThat(mCombiner.createPresentableList())
@@ -215,7 +230,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsWork_workItemsOnly() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForCurrentUser(WORK_ITEMS)

                        .setRootListForOtherUser(Collections.emptyList());
@@ -233,7 +249,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(Collections.emptyList());

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(PERSONAL_USER)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))
@@ -250,7 +267,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(Collections.emptyList());

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))
@@ -266,7 +284,8 @@ public class UserItemsCombinerTest { 
        rootListAllUsers.add(Collections.emptyList());

        rootListAllUsers.add(Lists.newArrayList(WORK_ITEMS));

        rootListAllUsers.add(Collections.emptyList());

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(PRIVATE_USER)

                        .setRootListForAllUsers(rootListAllUsers);

        assertThat(mCombiner.createPresentableListForAllUsers(mUserIds, mUserIdToLabelMap))
@@ -277,7 +296,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsPersonal_personalAndWorkItems() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForCurrentUser(PERSONAL_ITEMS)

                        .setRootListForOtherUser(WORK_ITEMS);
@@ -295,7 +315,8 @@ public class UserItemsCombinerTest { 


    @Test

    public void testCreatePresentableList_currentIsWork_personalAndWorkItems() {

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForCurrentUser(WORK_ITEMS)

                        .setRootListForOtherUser(PERSONAL_ITEMS);
@@ -320,7 +341,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(PRIVATE_ITEMS);

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForAllUsers(rootListAllUsers);



        List<Item> expected = Lists.newArrayList();
@@ -347,7 +369,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(PRIVATE_ITEMS);

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForAllUsers(rootListAllUsers);
@@ -375,7 +398,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(PRIVATE_ITEMS);

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(PRIVATE_USER)

                        .setRootListForAllUsers(rootListAllUsers);
@@ -398,7 +422,8 @@ public class UserItemsCombinerTest { 
    @Test

    public void testCreatePresentableList_currentIsPersonal_personalAndWorkItems_cannotShare() {

        mState.canShareAcrossProfile = false;

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForCurrentUser(PERSONAL_ITEMS)

                        .setRootListForOtherUser(WORK_ITEMS);
@@ -411,7 +436,8 @@ public class UserItemsCombinerTest { 
    @Test

    public void testCreatePresentableList_currentIsWork_personalItemsOnly_cannotShare() {

        mState.canShareAcrossProfile = false;

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForCurrentUser(Collections.emptyList())

                        .setRootListForOtherUser(PERSONAL_ITEMS);
@@ -429,7 +455,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(PRIVATE_ITEMS);

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .setRootListForAllUsers(rootListAllUsers);



        List<Item> expected = Lists.newArrayList();
@@ -460,7 +487,8 @@ public class UserItemsCombinerTest { 
        if (SdkLevel.isAtLeastV()) {

            rootListAllUsers.add(PRIVATE_ITEMS);

        }

        mCombiner = new UserItemsCombiner(mResources, mDpm, mState)

        mCombiner =

                new UserItemsCombiner(mResources, mMockUserManager, mDpm, mState)

                        .overrideCurrentUserForTest(WORK_USER)

                        .setRootListForAllUsers(rootListAllUsers);