diff options
4 files changed, 152 insertions, 37 deletions
diff --git a/core/java/com/android/internal/app/ChooserActivity.java b/core/java/com/android/internal/app/ChooserActivity.java index 9f58fee539f0..862c9001e380 100644 --- a/core/java/com/android/internal/app/ChooserActivity.java +++ b/core/java/com/android/internal/app/ChooserActivity.java @@ -850,9 +850,7 @@ public class ChooserActivity extends ResolverActivity implements final List<ShortcutManager.ShareShortcutInfo> shareShortcutInfos = new ArrayList<>(); - // Separate ChooserTargets ranking scores and ranked Shortcuts. List<AppTarget> shortcutResults = new ArrayList<>(); - List<AppTarget> chooserTargetScores = new ArrayList<>(); for (AppTarget appTarget : resultList) { if (appTarget.getShortcutInfo() == null) { continue; diff --git a/core/java/com/android/internal/app/ChooserListAdapter.java b/core/java/com/android/internal/app/ChooserListAdapter.java index d153107d77c7..cc2b12a99d79 100644 --- a/core/java/com/android/internal/app/ChooserListAdapter.java +++ b/core/java/com/android/internal/app/ChooserListAdapter.java @@ -33,9 +33,9 @@ import android.graphics.drawable.Drawable; import android.os.AsyncTask; import android.os.UserHandle; import android.os.UserManager; +import android.provider.DeviceConfig; import android.service.chooser.ChooserTarget; import android.util.Log; -import android.util.Pair; import android.view.View; import android.view.ViewGroup; @@ -46,14 +46,13 @@ import com.android.internal.app.chooser.DisplayResolveInfo; import com.android.internal.app.chooser.MultiDisplayResolveInfo; import com.android.internal.app.chooser.SelectableTargetInfo; import com.android.internal.app.chooser.TargetInfo; +import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; public class ChooserListAdapter extends ResolverListAdapter { private static final String TAG = "ChooserListAdapter"; @@ -70,8 +69,6 @@ public class ChooserListAdapter extends ResolverListAdapter { private static final int MAX_SUGGESTED_APP_TARGETS = 4; private static final int MAX_CHOOSER_TARGETS_PER_APP = 2; - private static final int MAX_SERVICE_TARGET_APP = 8; - private static final int DEFAULT_DIRECT_SHARE_RANKING_SCORE = 1000; /** {@link #getBaseScore} */ public static final float CALLER_TARGET_SCORE_BOOST = 900.f; @@ -85,17 +82,11 @@ public class ChooserListAdapter extends ResolverListAdapter { private int mNumShortcutResults = 0; private Map<DisplayResolveInfo, LoadIconTask> mIconLoaders = new HashMap<>(); + private boolean mApplySharingAppLimits; // Reserve spots for incoming direct share targets by adding placeholders private ChooserTargetInfo mPlaceHolderTargetInfo = new ChooserActivity.PlaceHolderTargetInfo(); - private int mValidServiceTargetsNum = 0; - private int mAvailableServiceTargetsNum = 0; - private final Map<ComponentName, Pair<List<ChooserTargetInfo>, Integer>> - mParkingDirectShareTargets = new HashMap<>(); - private final Map<ComponentName, Map<String, Integer>> mChooserTargetScores = new HashMap<>(); - private Set<ComponentName> mPendingChooserTargetService = new HashSet<>(); - private Set<ComponentName> mShortcutComponents = new HashSet<>(); private final List<ChooserTargetInfo> mServiceTargets = new ArrayList<>(); private final List<DisplayResolveInfo> mCallerTargets = new ArrayList<>(); @@ -174,6 +165,10 @@ public class ChooserListAdapter extends ResolverListAdapter { if (mCallerTargets.size() == MAX_SUGGESTED_APP_TARGETS) break; } } + mApplySharingAppLimits = DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.APPLY_SHARING_APP_LIMITS_IN_SYSUI, + true); } AppPredictor getAppPredictor() { @@ -209,10 +204,6 @@ public class ChooserListAdapter extends ResolverListAdapter { private void createPlaceHolders() { mNumShortcutResults = 0; mServiceTargets.clear(); - mValidServiceTargetsNum = 0; - mParkingDirectShareTargets.clear(); - mPendingChooserTargetService.clear(); - mShortcutComponents.clear(); for (int i = 0; i < mChooserListCommunicator.getMaxRankedTargets(); i++) { mServiceTargets.add(mPlaceHolderTargetInfo); } @@ -508,25 +499,27 @@ public class ChooserListAdapter extends ResolverListAdapter { if (targets.size() == 0) { return; } - final float baseScore = getBaseScore(origTarget, targetType); Collections.sort(targets, mBaseTargetComparator); - final boolean isShortcutResult = (targetType == TARGET_TYPE_SHORTCUTS_FROM_SHORTCUT_MANAGER || targetType == TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE); final int maxTargets = isShortcutResult ? mMaxShortcutTargetsPerApp : MAX_CHOOSER_TARGETS_PER_APP; + final int targetsLimit = mApplySharingAppLimits ? Math.min(targets.size(), maxTargets) + : targets.size(); float lastScore = 0; boolean shouldNotify = false; - for (int i = 0, count = Math.min(targets.size(), maxTargets); i < count; i++) { + for (int i = 0, count = targetsLimit; i < count; i++) { final ChooserTarget target = targets.get(i); float targetScore = target.getScore(); - targetScore *= baseScore; - if (i > 0 && targetScore >= lastScore) { - // Apply a decay so that the top app can't crowd out everything else. - // This incents ChooserTargetServices to define what's truly better. - targetScore = lastScore * 0.95f; + if (mApplySharingAppLimits) { + targetScore *= baseScore; + if (i > 0 && targetScore >= lastScore) { + // Apply a decay so that the top app can't crowd out everything else. + // This incents ChooserTargetServices to define what's truly better. + targetScore = lastScore * 0.95f; + } } UserHandle userHandle = getUserHandle(); Context contextAsUser = mContext.createContextAsUser(userHandle, 0 /* flags */); @@ -544,7 +537,8 @@ public class ChooserListAdapter extends ResolverListAdapter { Log.d(TAG, " => " + target.toString() + " score=" + targetScore + " base=" + target.getScore() + " lastScore=" + lastScore - + " baseScore=" + baseScore); + + " baseScore=" + baseScore + + " applyAppLimit=" + mApplySharingAppLimits); } lastScore = targetScore; @@ -580,16 +574,11 @@ public class ChooserListAdapter extends ResolverListAdapter { if (target == null) { return CALLER_TARGET_SCORE_BOOST; } - - if (targetType == TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE) { - return SHORTCUT_TARGET_SCORE_BOOST; - } - float score = super.getScore(target); - if (targetType == TARGET_TYPE_SHORTCUTS_FROM_SHORTCUT_MANAGER) { + if (targetType == TARGET_TYPE_SHORTCUTS_FROM_SHORTCUT_MANAGER + || targetType == TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE) { return score * SHORTCUT_TARGET_SCORE_BOOST; } - return score; } diff --git a/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java b/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java index 52801faf9c36..3729899f819d 100644 --- a/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java +++ b/core/java/com/android/internal/config/sysui/SystemUiDeviceConfigFlags.java @@ -473,6 +473,14 @@ public final class SystemUiDeviceConfigFlags { */ public static final String SHARE_USE_SERVICE_TARGETS = "share_use_service_targets"; + /** + * (boolean) If true, SysUI provides guardrails for app usage of Direct Share by enforcing + * limits on number of targets per app & adjusting scores for apps providing many targets. If + * false, this step is skipped. This should be true unless the ranking provider configured by + * [some other flag] is expected to manage these incentives. + */ + public static final String APPLY_SHARING_APP_LIMITS_IN_SYSUI = + "apply_sharing_app_limits_in_sysui"; /* * (long) The duration that the home button must be pressed before triggering Assist diff --git a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java index d9012f649dd3..4c58ad3d7f8d 100644 --- a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java +++ b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java @@ -154,8 +154,8 @@ public class ChooserActivityTest { sOverrides.reset(); sOverrides.createPackageManager = mPackageManagerOverride; DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, - SystemUiDeviceConfigFlags.APPEND_DIRECT_SHARE_ENABLED, - Boolean.toString(false), + SystemUiDeviceConfigFlags.APPLY_SHARING_APP_LIMITS_IN_SYSUI, + Boolean.toString(true), true /* makeDefault*/); } @@ -1017,7 +1017,7 @@ public class ChooserActivityTest { assertThat(adapter.getBaseScore(testDri, TARGET_TYPE_DEFAULT), is(testBaseScore)); assertThat(adapter.getBaseScore(testDri, TARGET_TYPE_CHOOSER_TARGET), is(testBaseScore)); assertThat(adapter.getBaseScore(testDri, TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE), - is(SHORTCUT_TARGET_SCORE_BOOST)); + is(testBaseScore * SHORTCUT_TARGET_SCORE_BOOST)); assertThat(adapter.getBaseScore(testDri, TARGET_TYPE_SHORTCUTS_FROM_SHORTCUT_MANAGER), is(testBaseScore * SHORTCUT_TARGET_SCORE_BOOST)); } @@ -1262,6 +1262,126 @@ public class ChooserActivityTest { .getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_RANKED_POSITION), is(0)); } + @Test + public void testShortcutTargetWithApplyAppLimits() throws InterruptedException { + // Set up resources + sOverrides.resources = Mockito.spy( + InstrumentationRegistry.getInstrumentation().getContext().getResources()); + when(sOverrides.resources.getInteger(R.integer.config_maxShortcutTargetsPerApp)) + .thenReturn(1); + Intent sendIntent = createSendTextIntent(); + // We need app targets for direct targets to get displayed + List<ResolvedComponentInfo> resolvedComponentInfos = createResolvedComponentsForTest(2); + when(sOverrides.resolverListController.getResolversForIntent(Mockito.anyBoolean(), + Mockito.anyBoolean(), + Mockito.isA(List.class))).thenReturn(resolvedComponentInfos); + // Create direct share target + List<ChooserTarget> serviceTargets = createDirectShareTargets(2, + resolvedComponentInfos.get(0).getResolveInfoAt(0).activityInfo.packageName); + ResolveInfo ri = ResolverDataProvider.createResolveInfo(3, 0); + + // Start activity + final ChooserWrapperActivity activity = mActivityRule + .launchActivity(Intent.createChooser(sendIntent, null)); + + // Insert the direct share target + Map<ChooserTarget, ShortcutInfo> directShareToShortcutInfos = new HashMap<>(); + List<ShareShortcutInfo> shortcutInfos = createShortcuts(activity); + directShareToShortcutInfos.put(serviceTargets.get(0), + shortcutInfos.get(0).getShortcutInfo()); + directShareToShortcutInfos.put(serviceTargets.get(1), + shortcutInfos.get(1).getShortcutInfo()); + InstrumentationRegistry.getInstrumentation().runOnMainSync( + () -> activity.getAdapter().addServiceResults( + activity.createTestDisplayResolveInfo(sendIntent, + ri, + "testLabel", + "testInfo", + sendIntent, + /* resolveInfoPresentationGetter */ null), + serviceTargets, + TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE, + directShareToShortcutInfos, + List.of()) + ); + // Thread.sleep shouldn't be a thing in an integration test but it's + // necessary here because of the way the code is structured + // TODO: restructure the tests b/129870719 + Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS); + + assertThat("Chooser should have 3 targets (2 apps, 1 direct)", + activity.getAdapter().getCount(), is(3)); + assertThat("Chooser should have exactly one selectable direct target", + activity.getAdapter().getSelectableServiceTargetCount(), is(1)); + assertThat("The resolver info must match the resolver info used to create the target", + activity.getAdapter().getItem(0).getResolveInfo(), is(ri)); + assertThat("The display label must match", + activity.getAdapter().getItem(0).getDisplayLabel(), is("testTitle0")); + } + + @Test + public void testShortcutTargetWithoutApplyAppLimits() throws InterruptedException { + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_SYSTEMUI, + SystemUiDeviceConfigFlags.APPLY_SHARING_APP_LIMITS_IN_SYSUI, + Boolean.toString(false), + true /* makeDefault*/); + // Set up resources + sOverrides.resources = Mockito.spy( + InstrumentationRegistry.getInstrumentation().getContext().getResources()); + when(sOverrides.resources.getInteger(R.integer.config_maxShortcutTargetsPerApp)) + .thenReturn(1); + Intent sendIntent = createSendTextIntent(); + // We need app targets for direct targets to get displayed + List<ResolvedComponentInfo> resolvedComponentInfos = createResolvedComponentsForTest(2); + when(sOverrides.resolverListController.getResolversForIntent(Mockito.anyBoolean(), + Mockito.anyBoolean(), + Mockito.isA(List.class))).thenReturn(resolvedComponentInfos); + // Create direct share target + List<ChooserTarget> serviceTargets = createDirectShareTargets(2, + resolvedComponentInfos.get(0).getResolveInfoAt(0).activityInfo.packageName); + ResolveInfo ri = ResolverDataProvider.createResolveInfo(3, 0); + + // Start activity + final ChooserWrapperActivity activity = mActivityRule + .launchActivity(Intent.createChooser(sendIntent, null)); + + // Insert the direct share target + Map<ChooserTarget, ShortcutInfo> directShareToShortcutInfos = new HashMap<>(); + List<ShareShortcutInfo> shortcutInfos = createShortcuts(activity); + directShareToShortcutInfos.put(serviceTargets.get(0), + shortcutInfos.get(0).getShortcutInfo()); + directShareToShortcutInfos.put(serviceTargets.get(1), + shortcutInfos.get(1).getShortcutInfo()); + InstrumentationRegistry.getInstrumentation().runOnMainSync( + () -> activity.getAdapter().addServiceResults( + activity.createTestDisplayResolveInfo(sendIntent, + ri, + "testLabel", + "testInfo", + sendIntent, + /* resolveInfoPresentationGetter */ null), + serviceTargets, + TARGET_TYPE_SHORTCUTS_FROM_PREDICTION_SERVICE, + directShareToShortcutInfos, + List.of()) + ); + // Thread.sleep shouldn't be a thing in an integration test but it's + // necessary here because of the way the code is structured + // TODO: restructure the tests b/129870719 + Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS); + + assertThat("Chooser should have 4 targets (2 apps, 2 direct)", + activity.getAdapter().getCount(), is(4)); + assertThat("Chooser should have exactly two selectable direct target", + activity.getAdapter().getSelectableServiceTargetCount(), is(2)); + assertThat("The resolver info must match the resolver info used to create the target", + activity.getAdapter().getItem(0).getResolveInfo(), is(ri)); + assertThat("The display label must match", + activity.getAdapter().getItem(0).getDisplayLabel(), is("testTitle0")); + assertThat("The display label must match", + activity.getAdapter().getItem(1).getDisplayLabel(), is("testTitle1")); + } + // This test is too long and too slow and should not be taken as an example for future tests. @Test public void testDirectTargetLoggingWithAppTargetNotRankedPortrait() |