From e8b863318125cf088d7f357ef0f04f2935b869fd Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Fri, 15 Sep 2017 14:48:46 -0700 Subject: [PATCH] Fixing issue with PiP settings not showing apps for other profiles. Bug: 65417722 Test: make -j40 RunSettingsRoboTests Change-Id: If3562dc7c2ddf07e2fc19591238dc3a7b4e9dba7 Merged-In: I3c3fc7a68c172eeccc767cd04b1393b38b36073e --- .../applications/PackageManagerWrapper.java | 6 + .../PackageManagerWrapperImpl.java | 6 + .../PictureInPictureSettings.java | 111 +++++++++--- .../applications/UserManagerWrapper.java | 1 + .../applications/UserManagerWrapperImpl.java | 5 + .../PictureInPictureSettingsTest.java | 168 ++++++++++++++++++ 6 files changed, 272 insertions(+), 25 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/applications/PictureInPictureSettingsTest.java diff --git a/src/com/android/settings/applications/PackageManagerWrapper.java b/src/com/android/settings/applications/PackageManagerWrapper.java index 580b578da73..b0accdc04e6 100644 --- a/src/com/android/settings/applications/PackageManagerWrapper.java +++ b/src/com/android/settings/applications/PackageManagerWrapper.java @@ -21,6 +21,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.IPackageDeleteObserver; +import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.os.UserHandle; @@ -48,6 +49,11 @@ public interface PackageManagerWrapper { */ List getInstalledApplicationsAsUser(int flags, int userId); + /** + * Calls {@code PackageManager.getInstalledPackagesAsUser} + */ + List getInstalledPackagesAsUser(int flags, int userId); + /** * Calls {@code PackageManager.hasSystemFeature()}. * diff --git a/src/com/android/settings/applications/PackageManagerWrapperImpl.java b/src/com/android/settings/applications/PackageManagerWrapperImpl.java index a47137c069f..021a80a3f2e 100644 --- a/src/com/android/settings/applications/PackageManagerWrapperImpl.java +++ b/src/com/android/settings/applications/PackageManagerWrapperImpl.java @@ -21,6 +21,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.IPackageDeleteObserver; +import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.os.UserHandle; @@ -46,6 +47,11 @@ public class PackageManagerWrapperImpl implements PackageManagerWrapper { return mPm.getInstalledApplicationsAsUser(flags, userId); } + @Override + public List getInstalledPackagesAsUser(int flags, int userId) { + return mPm.getInstalledPackagesAsUser(flags, userId); + } + @Override public boolean hasSystemFeature(String name) { return mPm.hasSystemFeature(name); diff --git a/src/com/android/settings/applications/PictureInPictureSettings.java b/src/com/android/settings/applications/PictureInPictureSettings.java index b1c544ad089..79780d6ad50 100644 --- a/src/com/android/settings/applications/PictureInPictureSettings.java +++ b/src/com/android/settings/applications/PictureInPictureSettings.java @@ -22,14 +22,16 @@ import android.content.Context; import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; -import android.content.pm.PackageItemInfo; import android.content.pm.PackageManager; +import android.content.pm.UserInfo; import android.os.Bundle; import android.os.UserHandle; +import android.os.UserManager; import android.support.v7.preference.Preference; import android.support.v7.preference.Preference.OnPreferenceClickListener; import android.support.v7.preference.PreferenceScreen; -import android.util.ArrayMap; +import android.util.IconDrawableFactory; +import android.util.Pair; import android.view.View; import com.android.internal.annotations.VisibleForTesting; @@ -37,8 +39,10 @@ import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; import com.android.settings.notification.EmptyTextSettings; +import java.text.Collator; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; public class PictureInPictureSettings extends EmptyTextSettings { @@ -50,8 +54,38 @@ public class PictureInPictureSettings extends EmptyTextSettings { IGNORE_PACKAGE_LIST.add("com.android.systemui"); } + /** + * Comparator by name, then user id. + * {@see PackageItemInfo#DisplayNameComparator} + */ + static class AppComparator implements Comparator> { + + private final Collator mCollator = Collator.getInstance(); + private final PackageManager mPm; + + public AppComparator(PackageManager pm) { + mPm = pm; + } + + public final int compare(Pair a, + Pair b) { + CharSequence sa = a.first.loadLabel(mPm); + if (sa == null) sa = a.first.name; + CharSequence sb = b.first.loadLabel(mPm); + if (sb == null) sb = b.first.name; + int nameCmp = mCollator.compare(sa.toString(), sb.toString()); + if (nameCmp != 0) { + return nameCmp; + } else { + return a.second - b.second; + } + } + } + private Context mContext; - private PackageManager mPackageManager; + private PackageManagerWrapper mPackageManager; + private UserManagerWrapper mUserManager; + private IconDrawableFactory mIconDrawableFactory; /** * @return true if the package has any activities that declare that they support @@ -93,12 +127,23 @@ public class PictureInPictureSettings extends EmptyTextSettings { return false; } + public PictureInPictureSettings() { + // Do nothing + } + + public PictureInPictureSettings(PackageManagerWrapper pm, UserManagerWrapper um) { + mPackageManager = pm; + mUserManager = um; + } + @Override public void onCreate(Bundle icicle) { super.onCreate(icicle); mContext = getActivity(); - mPackageManager = mContext.getPackageManager(); + mPackageManager = new PackageManagerWrapperImpl(mContext.getPackageManager()); + mUserManager = new UserManagerWrapperImpl(mContext.getSystemService(UserManager.class)); + mIconDrawableFactory = IconDrawableFactory.newInstance(mContext); setPreferenceScreen(getPreferenceManager().createPreferenceScreen(mContext)); } @@ -110,33 +155,25 @@ public class PictureInPictureSettings extends EmptyTextSettings { final PreferenceScreen screen = getPreferenceScreen(); screen.removeAll(); - // Fetch the set of applications which have at least one activity that declare that they - // support picture-in-picture - final ArrayMap packageToState = new ArrayMap<>(); - final ArrayList pipApps = new ArrayList<>(); - final List installedPackages = mPackageManager.getInstalledPackagesAsUser( - GET_ACTIVITIES, UserHandle.myUserId()); - for (PackageInfo packageInfo : installedPackages) { - if (checkPackageHasPictureInPictureActivities(packageInfo.packageName, - packageInfo.activities)) { - final String packageName = packageInfo.applicationInfo.packageName; - final boolean state = PictureInPictureDetails.getEnterPipStateForPackage( - mContext, packageInfo.applicationInfo.uid, packageName); - pipApps.add(packageInfo.applicationInfo); - packageToState.put(packageName, state); - } - } - Collections.sort(pipApps, new PackageItemInfo.DisplayNameComparator(mPackageManager)); + // Fetch the set of applications for each profile which have at least one activity that + // declare that they support picture-in-picture + final PackageManager pm = mPackageManager.getPackageManager(); + final ArrayList> pipApps = + collectPipApps(UserHandle.myUserId()); + Collections.sort(pipApps, new AppComparator(pm)); // Rebuild the list of prefs final Context prefContext = getPrefContext(); - for (final ApplicationInfo appInfo : pipApps) { + for (final Pair appData : pipApps) { + final ApplicationInfo appInfo = appData.first; + final int userId = appData.second; + final UserHandle user = UserHandle.of(userId); final String packageName = appInfo.packageName; - final CharSequence label = appInfo.loadLabel(mPackageManager); + final CharSequence label = appInfo.loadLabel(pm); final Preference pref = new Preference(prefContext); - pref.setIcon(appInfo.loadIcon(mPackageManager)); - pref.setTitle(label); + pref.setIcon(mIconDrawableFactory.getBadgedIcon(appInfo, userId)); + pref.setTitle(pm.getUserBadgedLabel(label, user)); pref.setSummary(PictureInPictureDetails.getPreferenceSummary(prefContext, appInfo.uid, packageName)); pref.setOnPreferenceClickListener(new OnPreferenceClickListener() { @@ -162,4 +199,28 @@ public class PictureInPictureSettings extends EmptyTextSettings { public int getMetricsCategory() { return MetricsEvent.SETTINGS_MANAGE_PICTURE_IN_PICTURE; } + + /** + * @return the list of applications for the given user and all their profiles that have + * activities which support PiP. + */ + ArrayList> collectPipApps(int userId) { + final ArrayList> pipApps = new ArrayList<>(); + final ArrayList userIds = new ArrayList<>(); + for (UserInfo user : mUserManager.getProfiles(userId)) { + userIds.add(user.id); + } + + for (int id : userIds) { + final List installedPackages = mPackageManager.getInstalledPackagesAsUser( + GET_ACTIVITIES, id); + for (PackageInfo packageInfo : installedPackages) { + if (checkPackageHasPictureInPictureActivities(packageInfo.packageName, + packageInfo.activities)) { + pipApps.add(new Pair<>(packageInfo.applicationInfo, id)); + } + } + } + return pipApps; + } } diff --git a/src/com/android/settings/applications/UserManagerWrapper.java b/src/com/android/settings/applications/UserManagerWrapper.java index 5b4ed2aa05f..5a8628531fe 100644 --- a/src/com/android/settings/applications/UserManagerWrapper.java +++ b/src/com/android/settings/applications/UserManagerWrapper.java @@ -29,4 +29,5 @@ import java.util.List; public interface UserManagerWrapper { UserInfo getPrimaryUser(); List getUsers(); + List getProfiles(int userHandle); } diff --git a/src/com/android/settings/applications/UserManagerWrapperImpl.java b/src/com/android/settings/applications/UserManagerWrapperImpl.java index 14ea64ae345..26ffc3e8351 100644 --- a/src/com/android/settings/applications/UserManagerWrapperImpl.java +++ b/src/com/android/settings/applications/UserManagerWrapperImpl.java @@ -37,4 +37,9 @@ public class UserManagerWrapperImpl implements UserManagerWrapper { public List getUsers() { return mUserManager.getUsers(); } + + @Override + public List getProfiles(int userHandle) { + return mUserManager.getProfiles(userHandle); + } } diff --git a/tests/robotests/src/com/android/settings/applications/PictureInPictureSettingsTest.java b/tests/robotests/src/com/android/settings/applications/PictureInPictureSettingsTest.java new file mode 100644 index 00000000000..77acb462462 --- /dev/null +++ b/tests/robotests/src/com/android/settings/applications/PictureInPictureSettingsTest.java @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.settings.applications; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.content.pm.ActivityInfo; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageInfo; +import android.content.pm.UserInfo; +import android.util.Pair; + +import com.android.internal.logging.nano.MetricsProto; +import com.android.settings.TestConfig; +import com.android.settings.testutils.FakeFeatureFactory; +import com.android.settings.testutils.SettingsRobolectricTestRunner; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.annotation.Config; + +import java.util.ArrayList; +import java.util.Collections; + +@RunWith(SettingsRobolectricTestRunner.class) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +public class PictureInPictureSettingsTest { + + private static final int PRIMARY_USER_ID = 0; + private static final int PROFILE_USER_ID = 10; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private Context mContext; + + private FakeFeatureFactory mFeatureFactory; + private PictureInPictureSettings mFragment; + @Mock + private PackageManagerWrapper mPackageManager; + @Mock + private UserManagerWrapper mUserManager; + private ArrayList mPrimaryUserPackages; + private ArrayList mProfileUserPackages; + private ArrayList mUsers; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + FakeFeatureFactory.setupForTest(mContext); + mFeatureFactory = (FakeFeatureFactory) FakeFeatureFactory.getFactory(mContext); + mFragment = new PictureInPictureSettings(mPackageManager, mUserManager); + mPrimaryUserPackages = new ArrayList<>(); + mProfileUserPackages = new ArrayList<>(); + mUsers = new ArrayList<>(); + when(mPackageManager.getInstalledPackagesAsUser(anyInt(), eq(PRIMARY_USER_ID))) + .thenReturn(mPrimaryUserPackages); + when(mPackageManager.getInstalledPackagesAsUser(anyInt(), eq(PROFILE_USER_ID))) + .thenReturn(mProfileUserPackages); + when(mUserManager.getProfiles(anyInt())).thenReturn(mUsers); + } + + @Test + public void testCollectPipApps() { + PackageInfo primaryP1 = createPackage("Calculator", true); + PackageInfo primaryP2 = createPackage("Clock", false); + PackageInfo profileP1 = createPackage("Calculator", false); + PackageInfo profileP2 = createPackage("Clock", true); + + mPrimaryUserPackages.add(primaryP1); + mPrimaryUserPackages.add(primaryP2); + mProfileUserPackages.add(profileP1); + mProfileUserPackages.add(profileP2); + + ArrayList> apps = mFragment.collectPipApps(PRIMARY_USER_ID); + assertThat(containsPackages(apps, primaryP1, profileP2)); + assertThat(!containsPackages(apps, primaryP2, profileP1)); + } + + @Test + public void testAppSort() { + PackageInfo primaryP1 = createPackage("Android", true); + PackageInfo primaryP2 = createPackage("Boop", true); + PackageInfo primaryP3 = createPackage("Deck", true); + PackageInfo profileP1 = createPackage("Android", true); + PackageInfo profileP2 = createPackage("Cool", true); + PackageInfo profileP3 = createPackage("Fast", false); + + mPrimaryUserPackages.add(primaryP1); + mPrimaryUserPackages.add(primaryP2); + mPrimaryUserPackages.add(primaryP3); + mProfileUserPackages.add(profileP1); + mProfileUserPackages.add(profileP2); + mProfileUserPackages.add(profileP3); + + ArrayList> apps = mFragment.collectPipApps(PRIMARY_USER_ID); + Collections.sort(apps, new PictureInPictureSettings.AppComparator(null)); + assertThat(isOrdered(apps, primaryP1, profileP1, primaryP2, profileP2)); + } + + private boolean containsPackages(ArrayList> apps, + PackageInfo... packages) { + for (int i = 0; i < packages.length; i++) { + boolean found = false; + for (int j = 0; j < apps.size(); j++) { + if (apps.get(j).first == packages[i].applicationInfo) { + found = true; + break; + } + } + if (!found) { + return false; + } + } + return true; + } + + private boolean isOrdered(ArrayList> apps, + PackageInfo... packages) { + if (apps.size() != packages.length) { + return false; + } + + for (int i = 0; i < packages.length; i++) { + if (packages[i].applicationInfo != apps.get(i).first) { + return false; + } + } + return true; + } + + private PackageInfo createPackage(String appTitle, boolean supportsPip) { + PackageInfo pi = new PackageInfo(); + ActivityInfo ai = new ActivityInfo(); + if (supportsPip) { + ai.flags |= ActivityInfo.FLAG_SUPPORTS_PICTURE_IN_PICTURE; + } + pi.activities = new ActivityInfo[1]; + pi.activities[0] = ai; + pi.applicationInfo = new ApplicationInfo(); + pi.applicationInfo.name = appTitle; + return pi; + } +}