From 218a9903375ccd7cafd9c29ed84daf0d07ca4c2d Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Thu, 23 Feb 2023 22:39:10 +0000 Subject: Allow refinement of any matching source intent. We believe it's merely a bug that the legacy implementation always merged the "refinement" into the "primary match" (among those that matched the selected target -- so it's sort of arbitrary that this didn't even necessarily have to be the "primary intent" of the chooser request). Thus we believe the loosened restrictions keep with the spirit of the original guardrails on refinement -- but this seems to be a necessary "fix" before the refinement flow could fit its billing as a mechanism to let users select among alternate formats. I believe this won't currently work for `SelectableTargetInfo` (i.e. app share/shortcut targets) which for some reason doesn't seem to keep track of its "alternates" -- I'll look into that outside of the scope of this current CL. Patchset #5 changes: * Make sure that component name is set for DisplayResolveInfo after a refinement intent is provided; * Update MultiDisplayResolveInfo intent refinement logic -- it is now delegated to a selected target. Bug: 262805893 Test: `atest IntentResolverUnitTests` Change-Id: Ie56b14b9a7beaa6bde8fe476d1ff140280abc51a --- .../intentresolver/ChooserRefinementManager.java | 33 +--- .../intentresolver/chooser/DisplayResolveInfo.java | 42 +++-- .../chooser/ImmutableTargetInfo.java | 139 ++++++++++----- .../chooser/MultiDisplayResolveInfo.java | 27 ++- .../chooser/SelectableTargetInfo.java | 56 +++--- .../android/intentresolver/chooser/TargetInfo.java | 19 ++- .../chooser/ImmutableTargetInfoTest.kt | 117 +++++++++---- .../intentresolver/chooser/TargetInfoTest.kt | 189 ++++++++++++++++++++- 8 files changed, 462 insertions(+), 160 deletions(-) diff --git a/java/src/com/android/intentresolver/ChooserRefinementManager.java b/java/src/com/android/intentresolver/ChooserRefinementManager.java index 98c6bddc..90447c5e 100644 --- a/java/src/com/android/intentresolver/ChooserRefinementManager.java +++ b/java/src/com/android/intentresolver/ChooserRefinementManager.java @@ -40,12 +40,6 @@ import java.util.function.Consumer; * additional extras and other refinements (subject to {@link Intent#filterEquals()}), e.g., to * convert the format of the payload, or lazy-download some data that was deferred in the original * call). - * - * TODO(b/262805893): this currently requires the result to be a refinement of the best - * match for the user's selected target among the initially-provided source intents (according to - * their originally-provided priority order). In order to support alternate formats/actions, we - * should instead require it to refine any of the source intents -- presumably, the first - * in priority order that matches according to {@link Intent#filterEquals()}. */ public final class ChooserRefinementManager { private static final String TAG = "ChooserRefinement"; @@ -88,10 +82,12 @@ public final class ChooserRefinementManager { mRefinementResultReceiver = new RefinementResultReceiver( refinedIntent -> { destroy(); - TargetInfo refinedTarget = getValidRefinedTarget(selectedTarget, refinedIntent); + TargetInfo refinedTarget = + selectedTarget.tryToCloneWithAppliedRefinement(refinedIntent); if (refinedTarget != null) { mOnSelectionRefined.accept(refinedTarget); } else { + Log.e(TAG, "Failed to apply refinement to any matching source intent"); mOnRefinementCancelled.run(); } }, @@ -192,27 +188,4 @@ public final class ChooserRefinementManager { return receiverForSending; } } - - private static TargetInfo getValidRefinedTarget( - TargetInfo originalTarget, Intent proposedRefinement) { - if (originalTarget == null) { - // TODO: this legacy log message doesn't seem to describe the real condition we just - // checked; probably this method should never be invoked with a null target. - Log.e(TAG, "Refinement result intent did not match any known targets; canceling"); - return null; - } - if (!checkProposalRefinesSourceIntent(originalTarget, proposedRefinement)) { - Log.e(TAG, "Refinement " + proposedRefinement + " has no match in " + originalTarget); - return null; - } - return originalTarget.cloneFilledIn(proposedRefinement, 0); // TODO: select the right base. - } - - // TODO: return the actual match, to use as the base that we fill in? Or, if that's handled by - // `TargetInfo.cloneFilledIn()`, just let it be nullable (it already is?) and don't bother doing - // this pre-check. - private static boolean checkProposalRefinesSourceIntent( - TargetInfo originalTarget, Intent proposedMatch) { - return originalTarget.getAllSourceIntents().stream().anyMatch(proposedMatch::filterEquals); - } } diff --git a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java index 0bbd6901..29be6dc6 100644 --- a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java +++ b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java @@ -96,25 +96,22 @@ public class DisplayResolveInfo implements TargetInfo { final ActivityInfo ai = mResolveInfo.activityInfo; mIsSuspended = (ai.applicationInfo.flags & ApplicationInfo.FLAG_SUSPENDED) != 0; - final Intent intent = new Intent(resolvedIntent); - intent.addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT - | Intent.FLAG_ACTIVITY_PREVIOUS_IS_TOP); - intent.setComponent(new ComponentName(ai.applicationInfo.packageName, ai.name)); - mResolvedIntent = intent; + mResolvedIntent = createResolvedIntent(resolvedIntent, ai); } private DisplayResolveInfo( DisplayResolveInfo other, - Intent fillInIntent, - int flags, + @Nullable Intent baseIntentToSend, TargetPresentationGetter presentationGetter) { mSourceIntents.addAll(other.getAllSourceIntents()); mResolveInfo = other.mResolveInfo; mIsSuspended = other.mIsSuspended; mDisplayLabel = other.mDisplayLabel; mExtendedInfo = other.mExtendedInfo; - mResolvedIntent = new Intent(other.mResolvedIntent); - mResolvedIntent.fillIn(fillInIntent, flags); + + mResolvedIntent = createResolvedIntent( + baseIntentToSend == null ? other.mResolvedIntent : baseIntentToSend, + mResolveInfo.activityInfo); mPresentationGetter = presentationGetter; mDisplayIconHolder.setDisplayIcon(other.mDisplayIconHolder.getDisplayIcon()); @@ -132,6 +129,14 @@ public class DisplayResolveInfo implements TargetInfo { mDisplayIconHolder.setDisplayIcon(other.mDisplayIconHolder.getDisplayIcon()); } + private static Intent createResolvedIntent(Intent resolvedIntent, ActivityInfo ai) { + final Intent result = new Intent(resolvedIntent); + result.addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT + | Intent.FLAG_ACTIVITY_PREVIOUS_IS_TOP); + result.setComponent(new ComponentName(ai.applicationInfo.packageName, ai.name)); + return result; + } + @Override public final boolean isDisplayResolveInfo() { return true; @@ -167,12 +172,21 @@ public class DisplayResolveInfo implements TargetInfo { } @Override - public TargetInfo cloneFilledIn(Intent fillInIntent, int flags) { - return cloneFilledInInternal(fillInIntent, flags); - } + @Nullable + public DisplayResolveInfo tryToCloneWithAppliedRefinement(Intent proposedRefinement) { + Intent matchingBase = + getAllSourceIntents() + .stream() + .filter(i -> i.filterEquals(proposedRefinement)) + .findFirst() + .orElse(null); + if (matchingBase == null) { + return null; + } - protected final DisplayResolveInfo cloneFilledInInternal(Intent fillInIntent, int flags) { - return new DisplayResolveInfo(this, fillInIntent, flags, mPresentationGetter); + Intent merged = new Intent(matchingBase); + merged.fillIn(proposedRefinement, 0); + return new DisplayResolveInfo(this, merged, mPresentationGetter); } @Override diff --git a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java index 38991c78..2d9683e1 100644 --- a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java @@ -16,6 +16,7 @@ package com.android.intentresolver.chooser; +import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Activity; import android.app.prediction.AppTarget; @@ -27,7 +28,6 @@ import android.content.pm.ShortcutInfo; import android.os.Bundle; import android.os.UserHandle; import android.util.HashedStringCache; -import android.util.Log; import androidx.annotation.VisibleForTesting; @@ -83,6 +83,15 @@ public final class ImmutableTargetInfo implements TargetInfo { @Nullable private ComponentName mResolvedComponentName; + @Nullable + private Intent mResolvedIntent; + + @Nullable + private Intent mBaseIntentToSend; + + @Nullable + private Intent mTargetIntent; + @Nullable private ComponentName mChooserTargetComponentName; @@ -101,20 +110,29 @@ public final class ImmutableTargetInfo implements TargetInfo { @Nullable private Intent mReferrerFillInIntent; - private Intent mResolvedIntent; - private Intent mTargetIntent; + @Nullable private TargetActivityStarter mActivityStarter; + + @Nullable private ResolveInfo mResolveInfo; + + @Nullable private CharSequence mDisplayLabel; + + @Nullable private CharSequence mExtendedInfo; + + @Nullable private IconHolder mDisplayIconHolder; - private List mSourceIntents; - private List mAllDisplayTargets; + private boolean mIsSuspended; private boolean mIsPinned; private float mModifiedScore = -0.1f; private LegacyTargetType mLegacyType = LegacyTargetType.NOT_LEGACY_TARGET; + private ImmutableList mAlternateSourceIntents = ImmutableList.of(); + private ImmutableList mAllDisplayTargets = ImmutableList.of(); + /** * Configure an {@link Intent} to be built in to the output target as the resolution for the * requested target data. @@ -124,6 +142,17 @@ public final class ImmutableTargetInfo implements TargetInfo { return this; } + /** + * Configure an {@link Intent} to be built in to the output target as the "base intent to + * send," which may be a refinement of any of our source targets. This is private because + * it's only used internally by {@link #tryToCloneWithAppliedRefinement()}; if it's ever + * expanded, the builder should probably be responsible for enforcing the refinement check. + */ + private Builder setBaseIntentToSend(Intent baseIntent) { + mBaseIntentToSend = baseIntent; + return this; + } + /** * Configure an {@link Intent} to be built in to the output as the "target intent." */ @@ -192,15 +221,33 @@ public final class ImmutableTargetInfo implements TargetInfo { return this; } - /** Configure the list of source intents to be built in to the output target. */ + /** Configure the list of alternate source intents we could resolve for this target. */ + public Builder setAlternateSourceIntents(List sourceIntents) { + mAlternateSourceIntents = immutableCopyOrEmpty(sourceIntents); + return this; + } + + /** + * Configure the full list of source intents we could resolve for this target. This is + * effectively the same as calling {@link #setResolvedIntent()} with the first element of + * the list, and {@link #setAlternateSourceIntents()} with the remainder (or clearing those + * fields on the builder if there are no corresponding elements in the list). + */ public Builder setAllSourceIntents(List sourceIntents) { - mSourceIntents = sourceIntents; + if ((sourceIntents == null) || sourceIntents.isEmpty()) { + setResolvedIntent(null); + setAlternateSourceIntents(null); + return this; + } + + setResolvedIntent(sourceIntents.get(0)); + setAlternateSourceIntents(sourceIntents.subList(1, sourceIntents.size())); return this; } /** Configure the list of display targets to be built in to the output target. */ public Builder setAllDisplayTargets(List targets) { - mAllDisplayTargets = targets; + mAllDisplayTargets = immutableCopyOrEmpty(targets); return this; } @@ -246,28 +293,27 @@ public final class ImmutableTargetInfo implements TargetInfo { return this; } - Builder setLegacyType(LegacyTargetType legacyType) { + Builder setLegacyType(@NonNull LegacyTargetType legacyType) { mLegacyType = legacyType; return this; } - /** - * Construct an {@code ImmutableTargetInfo} with the current builder data, where the - * provided intent is used to fill in missing values from the resolved intent before the - * target is (potentially) ever launched. - * - * @see android.content.Intent#fillIn(Intent, int) - */ - public ImmutableTargetInfo buildWithFillInIntent( - @Nullable Intent fillInIntent, int fillInFlags) { - Intent baseIntentToSend = mResolvedIntent; - if (baseIntentToSend == null) { - Log.w(TAG, "No base intent to send"); - } else { + /** Construct an {@code ImmutableTargetInfo} with the current builder data. */ + public ImmutableTargetInfo build() { + List sourceIntents = new ArrayList<>(); + if (mResolvedIntent != null) { + sourceIntents.add(mResolvedIntent); + } + if (mAlternateSourceIntents != null) { + sourceIntents.addAll(mAlternateSourceIntents); + } + + Intent baseIntentToSend = mBaseIntentToSend; + if ((baseIntentToSend == null) && !sourceIntents.isEmpty()) { + baseIntentToSend = sourceIntents.get(0); + } + if (baseIntentToSend != null) { baseIntentToSend = new Intent(baseIntentToSend); - if (fillInIntent != null) { - baseIntentToSend.fillIn(fillInIntent, fillInFlags); - } if (mReferrerFillInIntent != null) { baseIntentToSend.fillIn(mReferrerFillInIntent, 0); } @@ -275,7 +321,7 @@ public final class ImmutableTargetInfo implements TargetInfo { return new ImmutableTargetInfo( baseIntentToSend, - mResolvedIntent, + ImmutableList.copyOf(sourceIntents), mTargetIntent, mReferrerFillInIntent, mResolvedComponentName, @@ -285,7 +331,6 @@ public final class ImmutableTargetInfo implements TargetInfo { mDisplayLabel, mExtendedInfo, mDisplayIconHolder, - mSourceIntents, mAllDisplayTargets, mIsSuspended, mIsPinned, @@ -296,11 +341,6 @@ public final class ImmutableTargetInfo implements TargetInfo { mHashProvider, mLegacyType); } - - /** Construct an {@code ImmutableTargetInfo} with the current builder data. */ - public ImmutableTargetInfo build() { - return buildWithFillInIntent(null, 0); - } } @Nullable @@ -325,14 +365,13 @@ public final class ImmutableTargetInfo implements TargetInfo { private final TargetHashProvider mHashProvider; private final Intent mBaseIntentToSend; - private final Intent mResolvedIntent; + private final ImmutableList mSourceIntents; private final Intent mTargetIntent; private final TargetActivityStarter mActivityStarter; private final ResolveInfo mResolveInfo; private final CharSequence mDisplayLabel; private final CharSequence mExtendedInfo; private final IconHolder mDisplayIconHolder; - private final ImmutableList mSourceIntents; private final ImmutableList mAllDisplayTargets; private final boolean mIsSuspended; private final boolean mIsPinned; @@ -347,6 +386,7 @@ public final class ImmutableTargetInfo implements TargetInfo { /** Construct a {@link Builder} pre-initialized to match this target. */ public Builder toBuilder() { return newBuilder() + .setBaseIntentToSend(getBaseIntentToSend()) .setResolvedIntent(getResolvedIntent()) .setTargetIntent(getTargetIntent()) .setReferrerFillInIntent(getReferrerFillInIntent()) @@ -375,13 +415,26 @@ public final class ImmutableTargetInfo implements TargetInfo { } @Override - public ImmutableTargetInfo cloneFilledIn(Intent fillInIntent, int flags) { - return toBuilder().buildWithFillInIntent(fillInIntent, flags); + @Nullable + public ImmutableTargetInfo tryToCloneWithAppliedRefinement(Intent proposedRefinement) { + Intent matchingBase = + getAllSourceIntents() + .stream() + .filter(i -> i.filterEquals(proposedRefinement)) + .findFirst() + .orElse(null); + if (matchingBase == null) { + return null; + } + + Intent merged = new Intent(matchingBase); + merged.fillIn(proposedRefinement, 0); + return toBuilder().setBaseIntentToSend(merged).build(); } @Override public Intent getResolvedIntent() { - return mResolvedIntent; + return (mSourceIntents.isEmpty() ? null : mSourceIntents.get(0)); } @Override @@ -408,11 +461,13 @@ public final class ImmutableTargetInfo implements TargetInfo { @Override public boolean startAsCaller(Activity activity, Bundle options, int userId) { + // TODO: make sure that the component name is set in all cases return mActivityStarter.startAsCaller(this, activity, options, userId); } @Override public boolean startAsUser(Activity activity, Bundle options, UserHandle user) { + // TODO: make sure that the component name is set in all cases return mActivityStarter.startAsUser(this, activity, options, user); } @@ -531,7 +586,7 @@ public final class ImmutableTargetInfo implements TargetInfo { private ImmutableTargetInfo( Intent baseIntentToSend, - Intent resolvedIntent, + ImmutableList sourceIntents, Intent targetIntent, @Nullable Intent referrerFillInIntent, @Nullable ComponentName resolvedComponentName, @@ -541,8 +596,7 @@ public final class ImmutableTargetInfo implements TargetInfo { CharSequence displayLabel, CharSequence extendedInfo, IconHolder iconHolder, - @Nullable List sourceIntents, - @Nullable List allDisplayTargets, + ImmutableList allDisplayTargets, boolean isSuspended, boolean isPinned, float modifiedScore, @@ -552,7 +606,7 @@ public final class ImmutableTargetInfo implements TargetInfo { @Nullable TargetHashProvider hashProvider, LegacyTargetType legacyType) { mBaseIntentToSend = baseIntentToSend; - mResolvedIntent = resolvedIntent; + mSourceIntents = sourceIntents; mTargetIntent = targetIntent; mReferrerFillInIntent = referrerFillInIntent; mResolvedComponentName = resolvedComponentName; @@ -562,8 +616,7 @@ public final class ImmutableTargetInfo implements TargetInfo { mDisplayLabel = displayLabel; mExtendedInfo = extendedInfo; mDisplayIconHolder = iconHolder; - mSourceIntents = immutableCopyOrEmpty(sourceIntents); - mAllDisplayTargets = immutableCopyOrEmpty(allDisplayTargets); + mAllDisplayTargets = allDisplayTargets; mIsSuspended = isSuspended; mIsPinned = isPinned; mModifiedScore = modifiedScore; diff --git a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java index 0938c55e..b97e6b45 100644 --- a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java +++ b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java @@ -21,7 +21,10 @@ import android.content.Intent; import android.os.Bundle; import android.os.UserHandle; +import androidx.annotation.Nullable; + import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -93,10 +96,19 @@ public class MultiDisplayResolveInfo extends DisplayResolveInfo { } @Override - public TargetInfo cloneFilledIn(Intent fillInIntent, int flags) { - ArrayList targetInfos = new ArrayList<>(mTargetInfos.size()); - for (int i = 0, size = mTargetInfos.size(); i < size; i++) { - targetInfos.add(mTargetInfos.get(i).cloneFilledInInternal(fillInIntent, flags)); + @Nullable + public MultiDisplayResolveInfo tryToCloneWithAppliedRefinement(Intent proposedRefinement) { + final int size = mTargetInfos.size(); + ArrayList targetInfos = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + DisplayResolveInfo target = mTargetInfos.get(i); + DisplayResolveInfo targetClone = (i == mSelected) + ? target.tryToCloneWithAppliedRefinement(proposedRefinement) + : new DisplayResolveInfo(target); + if (targetClone == null) { + return null; + } + targetInfos.add(targetClone); } MultiDisplayResolveInfo clone = new MultiDisplayResolveInfo(targetInfos); clone.mSelected = mSelected; @@ -117,4 +129,11 @@ public class MultiDisplayResolveInfo extends DisplayResolveInfo { public Intent getTargetIntent() { return mTargetInfos.get(mSelected).getTargetIntent(); } + + @Override + public List getAllSourceIntents() { + return hasSelected() + ? mTargetInfos.get(mSelected).getAllSourceIntents() + : Collections.emptyList(); + } } diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java index df27c2b0..1fbe2da7 100644 --- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java @@ -78,7 +78,6 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { private final CharSequence mChooserTargetUnsanitizedTitle; private final Icon mChooserTargetIcon; private final Bundle mChooserTargetIntentExtras; - private final int mFillInFlags; private final boolean mIsPinned; private final float mModifiedScore; private final boolean mIsSuspended; @@ -90,12 +89,6 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { private final TargetHashProvider mHashProvider; private final TargetActivityStarter mActivityStarter; - /** - * A refinement intent from the caller, if any (see - * {@link Intent#EXTRA_CHOOSER_REFINEMENT_INTENT_SENDER}) - */ - private final Intent mFillInIntent; - /** * An intent containing referrer URI (see {@link Activity#getReferrer()} (possibly {@code null}) * in its extended data under the key {@link Intent#EXTRA_REFERRER}. @@ -159,6 +152,7 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { sourceInfo, backupResolveInfo, resolvedIntent, + null, chooserTargetComponentName, chooserTargetUnsanitizedTitle, chooserTargetIcon, @@ -166,15 +160,14 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { modifiedScore, shortcutInfo, appTarget, - referrerFillInIntent, - /* fillInIntent = */ null, - /* fillInFlags = */ 0); + referrerFillInIntent); } private SelectableTargetInfo( @Nullable DisplayResolveInfo sourceInfo, @Nullable ResolveInfo backupResolveInfo, Intent resolvedIntent, + @Nullable Intent baseIntentToSend, ComponentName chooserTargetComponentName, CharSequence chooserTargetUnsanitizedTitle, Icon chooserTargetIcon, @@ -182,9 +175,7 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { float modifiedScore, @Nullable ShortcutInfo shortcutInfo, @Nullable AppTarget appTarget, - Intent referrerFillInIntent, - @Nullable Intent fillInIntent, - int fillInFlags) { + Intent referrerFillInIntent) { mSourceInfo = sourceInfo; mBackupResolveInfo = backupResolveInfo; mResolvedIntent = resolvedIntent; @@ -192,8 +183,6 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { mShortcutInfo = shortcutInfo; mAppTarget = appTarget; mReferrerFillInIntent = referrerFillInIntent; - mFillInIntent = fillInIntent; - mFillInFlags = fillInFlags; mChooserTargetComponentName = chooserTargetComponentName; mChooserTargetUnsanitizedTitle = chooserTargetUnsanitizedTitle; mChooserTargetIcon = chooserTargetIcon; @@ -209,9 +198,8 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { mAllSourceIntents = getAllSourceIntents(sourceInfo); mBaseIntentToSend = getBaseIntentToSend( + baseIntentToSend, mResolvedIntent, - mFillInIntent, - mFillInFlags, mReferrerFillInIntent); mHashProvider = context -> { @@ -262,11 +250,12 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { }; } - private SelectableTargetInfo(SelectableTargetInfo other, Intent fillInIntent, int flags) { + private SelectableTargetInfo(SelectableTargetInfo other, Intent baseIntentToSend) { this( other.mSourceInfo, other.mBackupResolveInfo, other.mResolvedIntent, + baseIntentToSend, other.mChooserTargetComponentName, other.mChooserTargetUnsanitizedTitle, other.mChooserTargetIcon, @@ -274,14 +263,25 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { other.mModifiedScore, other.mShortcutInfo, other.mAppTarget, - other.mReferrerFillInIntent, - fillInIntent, - flags); + other.mReferrerFillInIntent); } @Override - public TargetInfo cloneFilledIn(Intent fillInIntent, int flags) { - return new SelectableTargetInfo(this, fillInIntent, flags); + @Nullable + public TargetInfo tryToCloneWithAppliedRefinement(Intent proposedRefinement) { + Intent matchingBase = + getAllSourceIntents() + .stream() + .filter(i -> i.filterEquals(proposedRefinement)) + .findFirst() + .orElse(null); + if (matchingBase == null) { + return null; + } + + Intent merged = new Intent(matchingBase); + merged.fillIn(proposedRefinement, 0); + return new SelectableTargetInfo(this, merged); } @Override @@ -418,18 +418,14 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { @Nullable private static Intent getBaseIntentToSend( - @Nullable Intent resolvedIntent, - Intent fillInIntent, - int fillInFlags, + @Nullable Intent providedBase, + @Nullable Intent fallbackBase, Intent referrerFillInIntent) { - Intent result = resolvedIntent; + Intent result = (providedBase != null) ? providedBase : fallbackBase; if (result == null) { Log.e(TAG, "ChooserTargetInfo: no base intent available to send"); } else { result = new Intent(result); - if (fillInIntent != null) { - result.fillIn(fillInIntent, fillInFlags); - } result.fillIn(referrerFillInIntent, 0); } return result; diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java index 69f58a7b..2f48704c 100644 --- a/java/src/com/android/intentresolver/chooser/TargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java @@ -182,10 +182,25 @@ public interface TargetInfo { default boolean hasDisplayIcon() { return getDisplayIconHolder().getDisplayIcon() != null; } + /** - * Clone this target with the given fill-in information. + * Attempt to apply a {@code proposedRefinement} that the {@link ChooserRefinementManager} + * received from the caller's refinement flow. This may succeed only if the target has a source + * intent that matches the filtering parameters of the proposed refinement (according to + * {@link Intent#filterEquals()}). Then the first such match is the "base intent," and the + * proposed refinement is merged into that base (via {@link Intent#fillIn()}; this can never + * result in a change to the {@link Intent#filterEquals()} status of the base, but may e.g. add + * new "extras" that weren't previously given in the base intent). + * + * @return a copy of this {@link TargetInfo} where the "base intent to send" is the result of + * merging the refinement into the best-matching source intent, if possible. If there is no + * suitable match for the proposed refinement, or if merging fails for any other reason, this + * returns null. + * + * @see android.content.Intent#fillIn(Intent, int) */ - TargetInfo cloneFilledIn(Intent fillInIntent, int flags); + @Nullable + TargetInfo tryToCloneWithAppliedRefinement(Intent proposedRefinement); /** * @return the list of supported source intents deduped against this single target diff --git a/java/tests/src/com/android/intentresolver/chooser/ImmutableTargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/ImmutableTargetInfoTest.kt index 4989a3f1..e9c755d3 100644 --- a/java/tests/src/com/android/intentresolver/chooser/ImmutableTargetInfoTest.kt +++ b/java/tests/src/com/android/intentresolver/chooser/ImmutableTargetInfoTest.kt @@ -88,7 +88,7 @@ class ImmutableTargetInfoTest { .setDisplayLabel(displayLabel) .setExtendedInfo(extendedInfo) .setDisplayIconHolder(displayIconHolder) - .setAllSourceIntents(listOf(sourceIntent1, sourceIntent2)) + .setAlternateSourceIntents(listOf(sourceIntent1, sourceIntent2)) .setAllDisplayTargets(listOf(displayTarget1, displayTarget2)) .setIsSuspended(true) .setIsPinned(true) @@ -108,7 +108,8 @@ class ImmutableTargetInfoTest { assertThat(info.displayLabel).isEqualTo(displayLabel) assertThat(info.extendedInfo).isEqualTo(extendedInfo) assertThat(info.displayIconHolder).isEqualTo(displayIconHolder) - assertThat(info.allSourceIntents).containsExactly(sourceIntent1, sourceIntent2) + assertThat(info.allSourceIntents).containsExactly( + resolvedIntent, sourceIntent1, sourceIntent2) assertThat(info.allDisplayTargets).containsExactly(displayTarget1, displayTarget2) assertThat(info.isSuspended).isTrue() assertThat(info.isPinned).isTrue() @@ -140,7 +141,7 @@ class ImmutableTargetInfoTest { .setDisplayLabel(displayLabel) .setExtendedInfo(extendedInfo) .setDisplayIconHolder(displayIconHolder) - .setAllSourceIntents(listOf(sourceIntent1, sourceIntent2)) + .setAlternateSourceIntents(listOf(sourceIntent1, sourceIntent2)) .setAllDisplayTargets(listOf(displayTarget1, displayTarget2)) .setIsSuspended(true) .setIsPinned(true) @@ -162,7 +163,8 @@ class ImmutableTargetInfoTest { assertThat(info.displayLabel).isEqualTo(displayLabel) assertThat(info.extendedInfo).isEqualTo(extendedInfo) assertThat(info.displayIconHolder).isEqualTo(displayIconHolder) - assertThat(info.allSourceIntents).containsExactly(sourceIntent1, sourceIntent2) + assertThat(info.allSourceIntents).containsExactly( + resolvedIntent, sourceIntent1, sourceIntent2) assertThat(info.allDisplayTargets).containsExactly(displayTarget1, displayTarget2) assertThat(info.isSuspended).isTrue() assertThat(info.isPinned).isTrue() @@ -204,25 +206,25 @@ class ImmutableTargetInfoTest { } @Test - fun testBaseIntentToSend_fillsInFromCloneRequestIntent() { + fun testBaseIntentToSend_fillsInFromRefinementIntent() { val originalIntent = Intent() - originalIntent.setPackage("original") + originalIntent.putExtra("ORIGINAL", true) - val cloneFillInIntent = Intent("CLONE_FILL_IN") - cloneFillInIntent.setPackage("clone") + val refinementIntent = Intent() + refinementIntent.putExtra("REFINEMENT", true) val originalInfo = ImmutableTargetInfo.newBuilder() .setResolvedIntent(originalIntent) .build() - val info = originalInfo.cloneFilledIn(cloneFillInIntent, 0) + val info = originalInfo.tryToCloneWithAppliedRefinement(refinementIntent) - assertThat(info.baseIntentToSend.getPackage()).isEqualTo("original") // Only fill if empty. - assertThat(info.baseIntentToSend.action).isEqualTo("CLONE_FILL_IN") + assertThat(info.baseIntentToSend.getBooleanExtra("ORIGINAL", false)).isTrue() + assertThat(info.baseIntentToSend.getBooleanExtra("REFINEMENT", false)).isTrue() } @Test - fun testBaseIntentToSend_twoFillInSourcesFavorsCloneRequest() { - val originalIntent = Intent() + fun testBaseIntentToSend_twoFillInSourcesFavorsRefinementRequest() { + val originalIntent = Intent("REFINE_ME") originalIntent.setPackage("original") val referrerFillInIntent = Intent("REFERRER_FILL_IN") @@ -234,43 +236,92 @@ class ImmutableTargetInfoTest { .setReferrerFillInIntent(referrerFillInIntent) .build() - val cloneFillInIntent = Intent("CLONE_FILL_IN") - cloneFillInIntent.setPackage("clone") + val refinementIntent = Intent("REFINE_ME") + refinementIntent.setPackage("original") // Has to match for refinement. - val info = infoWithReferrerFillIn.cloneFilledIn(cloneFillInIntent, 0) + val info = infoWithReferrerFillIn.tryToCloneWithAppliedRefinement(refinementIntent) assertThat(info.baseIntentToSend.getPackage()).isEqualTo("original") // Set all along. - assertThat(info.baseIntentToSend.action).isEqualTo("CLONE_FILL_IN") // Clone wins. + assertThat(info.baseIntentToSend.action).isEqualTo("REFINE_ME") // Refinement wins. assertThat(info.baseIntentToSend.type).isEqualTo("test/referrer") // Left for referrer. } @Test - fun testBaseIntentToSend_doubleCloningPreservesReferrerFillInButNotOriginalCloneFillIn() { - val originalIntent = Intent() + fun testBaseIntentToSend_doubleRefinementPreservesReferrerFillInButNotOriginalRefinement() { + val originalIntent = Intent("REFINE_ME") val referrerFillInIntent = Intent("REFERRER_FILL_IN") - val cloneFillInIntent1 = Intent() - cloneFillInIntent1.setPackage("clone1") - val cloneFillInIntent2 = Intent() - cloneFillInIntent2.setType("test/clone2") + referrerFillInIntent.putExtra("TEST", "REFERRER") + val refinementIntent1 = Intent("REFINE_ME") + refinementIntent1.putExtra("TEST1", "1") + val refinementIntent2 = Intent("REFINE_ME") + refinementIntent2.putExtra("TEST2", "2") val originalInfo = ImmutableTargetInfo.newBuilder() .setResolvedIntent(originalIntent) .setReferrerFillInIntent(referrerFillInIntent) .build() - val clone1 = originalInfo.cloneFilledIn(cloneFillInIntent1, 0) - val clone2 = clone1.cloneFilledIn(cloneFillInIntent2, 0) // Clone-of-clone. + val refined1 = originalInfo.tryToCloneWithAppliedRefinement(refinementIntent1) + val refined2 = refined1.tryToCloneWithAppliedRefinement(refinementIntent2) // Cloned clone. // Both clones get the same values filled in from the referrer intent. - assertThat(clone1.baseIntentToSend.action).isEqualTo("REFERRER_FILL_IN") - assertThat(clone2.baseIntentToSend.action).isEqualTo("REFERRER_FILL_IN") - // Each clone has the respective value that was set in the fill-in request. - assertThat(clone1.baseIntentToSend.getPackage()).isEqualTo("clone1") - assertThat(clone2.baseIntentToSend.type).isEqualTo("test/clone2") - // The clones don't have the data from each other's fill-in requests, even though the intent + assertThat(refined1.baseIntentToSend.getStringExtra("TEST")).isEqualTo("REFERRER") + assertThat(refined2.baseIntentToSend.getStringExtra("TEST")).isEqualTo("REFERRER") + // Each clone has the respective value that was set in their own refinement request. + assertThat(refined1.baseIntentToSend.getStringExtra("TEST1")).isEqualTo("1") + assertThat(refined2.baseIntentToSend.getStringExtra("TEST2")).isEqualTo("2") + // The clones don't have the data from each other's refinements, even though the intent // field is empty (thus able to be populated by filling-in). - assertThat(clone1.baseIntentToSend.type).isNull() - assertThat(clone2.baseIntentToSend.getPackage()).isNull() + assertThat(refined1.baseIntentToSend.getStringExtra("TEST2")).isNull() + assertThat(refined2.baseIntentToSend.getStringExtra("TEST1")).isNull() + } + + @Test + fun testBaseIntentToSend_refinementToAlternateSourceIntent() { + val originalIntent = Intent("DONT_REFINE_ME") + originalIntent.putExtra("originalIntent", true) + val mismatchedAlternate = Intent("DOESNT_MATCH") + mismatchedAlternate.putExtra("mismatchedAlternate", true) + val targetAlternate = Intent("REFINE_ME") + targetAlternate.putExtra("targetAlternate", true) + val extraMatch = Intent("REFINE_ME") + extraMatch.putExtra("extraMatch", true) + + val originalInfo = ImmutableTargetInfo.newBuilder() + .setResolvedIntent(originalIntent) + .setAllSourceIntents(listOf( + originalIntent, mismatchedAlternate, targetAlternate, extraMatch)) + .build() + + val refinement = Intent("REFINE_ME") // First match is `targetAlternate` + refinement.putExtra("refinement", true) + + val refinedResult = originalInfo.tryToCloneWithAppliedRefinement(refinement) + assertThat(refinedResult.baseIntentToSend.getBooleanExtra("refinement", false)).isTrue() + assertThat(refinedResult.baseIntentToSend.getBooleanExtra("targetAlternate", false)) + .isTrue() + // None of the other source intents got merged in (not even the later one that matched): + assertThat(refinedResult.baseIntentToSend.getBooleanExtra("originalIntent", false)) + .isFalse() + assertThat(refinedResult.baseIntentToSend.getBooleanExtra("mismatchedAlternate", false)) + .isFalse() + assertThat(refinedResult.baseIntentToSend.getBooleanExtra("extraMatch", false)).isFalse() + } + + @Test + fun testBaseIntentToSend_noSourceIntentMatchingProposedRefinement() { + val originalIntent = Intent("DONT_REFINE_ME") + originalIntent.putExtra("originalIntent", true) + val mismatchedAlternate = Intent("DOESNT_MATCH") + mismatchedAlternate.putExtra("mismatchedAlternate", true) + + val originalInfo = ImmutableTargetInfo.newBuilder() + .setResolvedIntent(originalIntent) + .setAllSourceIntents(listOf(originalIntent, mismatchedAlternate)) + .build() + + val refinement = Intent("PROPOSED_REFINEMENT") + assertThat(originalInfo.tryToCloneWithAppliedRefinement(refinement)).isNull() } @Test diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt index e9dbe00e..dddbcccb 100644 --- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt +++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt @@ -26,13 +26,19 @@ import android.graphics.drawable.AnimatedVectorDrawable import android.os.UserHandle import android.test.UiThreadTest import androidx.test.platform.app.InstrumentationRegistry +import com.android.intentresolver.ResolverDataProvider import com.android.intentresolver.createChooserTarget import com.android.intentresolver.createShortcutInfo import com.android.intentresolver.mock -import com.android.intentresolver.ResolverDataProvider +import com.android.intentresolver.whenever import com.google.common.truth.Truth.assertThat import org.junit.Before import org.junit.Test +import org.mockito.Mockito.any +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify class TargetInfoTest { private val context = InstrumentationRegistry.getInstrumentation().getContext() @@ -142,6 +148,42 @@ class TargetInfoTest { assertThat(targetInfo.resolvedComponentName).isEqualTo(ComponentName(pkgName, className)) } + @Test + fun testSelectableTargetInfo_noSourceIntentMatchingProposedRefinement() { + val resolvedIntent = Intent("DONT_REFINE_ME") + resolvedIntent.putExtra("resolvedIntent", true) + + val baseDisplayInfo = DisplayResolveInfo.newDisplayResolveInfo( + resolvedIntent, + ResolverDataProvider.createResolveInfo(1, 0), + "label", + "extended info", + resolvedIntent, + /* resolveInfoPresentationGetter= */ null) + val chooserTarget = createChooserTarget( + "title", 0.3f, ResolverDataProvider.createComponentName(2), "test_shortcut_id") + val shortcutInfo = createShortcutInfo("id", ResolverDataProvider.createComponentName(3), 3) + val appTarget = AppTarget( + AppTargetId("id"), + chooserTarget.componentName.packageName, + chooserTarget.componentName.className, + UserHandle.CURRENT) + + val targetInfo = SelectableTargetInfo.newSelectableTargetInfo( + baseDisplayInfo, + mock(), + resolvedIntent, + chooserTarget, + 0.1f, + shortcutInfo, + appTarget, + mock(), + ) + + val refinement = Intent("PROPOSED_REFINEMENT") + assertThat(targetInfo.tryToCloneWithAppliedRefinement(refinement)).isNull() + } + @Test fun testNewDisplayResolveInfo() { val intent = Intent(Intent.ACTION_SEND) @@ -162,6 +204,64 @@ class TargetInfoTest { assertThat(targetInfo.isChooserTargetInfo()).isFalse() } + @Test + fun test_DisplayResolveInfo_refinementToAlternateSourceIntent() { + val originalIntent = Intent("DONT_REFINE_ME") + originalIntent.putExtra("originalIntent", true) + val mismatchedAlternate = Intent("DOESNT_MATCH") + mismatchedAlternate.putExtra("mismatchedAlternate", true) + val targetAlternate = Intent("REFINE_ME") + targetAlternate.putExtra("targetAlternate", true) + val extraMatch = Intent("REFINE_ME") + extraMatch.putExtra("extraMatch", true) + + val originalInfo = DisplayResolveInfo.newDisplayResolveInfo( + originalIntent, + ResolverDataProvider.createResolveInfo(3, 0), + "label", + "extended info", + originalIntent, + /* resolveInfoPresentationGetter= */ null) + originalInfo.addAlternateSourceIntent(mismatchedAlternate) + originalInfo.addAlternateSourceIntent(targetAlternate) + originalInfo.addAlternateSourceIntent(extraMatch) + + val refinement = Intent("REFINE_ME") // First match is `targetAlternate` + refinement.putExtra("refinement", true) + + val refinedResult = originalInfo.tryToCloneWithAppliedRefinement(refinement) + // Note `DisplayResolveInfo` targets merge refinements directly into their `resolvedIntent`. + assertThat(refinedResult.resolvedIntent.getBooleanExtra("refinement", false)).isTrue() + assertThat(refinedResult.resolvedIntent.getBooleanExtra("targetAlternate", false)) + .isTrue() + // None of the other source intents got merged in (not even the later one that matched): + assertThat(refinedResult.resolvedIntent.getBooleanExtra("originalIntent", false)) + .isFalse() + assertThat(refinedResult.resolvedIntent.getBooleanExtra("mismatchedAlternate", false)) + .isFalse() + assertThat(refinedResult.resolvedIntent.getBooleanExtra("extraMatch", false)).isFalse() + } + + @Test + fun testDisplayResolveInfo_noSourceIntentMatchingProposedRefinement() { + val originalIntent = Intent("DONT_REFINE_ME") + originalIntent.putExtra("originalIntent", true) + val mismatchedAlternate = Intent("DOESNT_MATCH") + mismatchedAlternate.putExtra("mismatchedAlternate", true) + + val originalInfo = DisplayResolveInfo.newDisplayResolveInfo( + originalIntent, + ResolverDataProvider.createResolveInfo(3, 0), + "label", + "extended info", + originalIntent, + /* resolveInfoPresentationGetter= */ null) + originalInfo.addAlternateSourceIntent(mismatchedAlternate) + + val refinement = Intent("PROPOSED_REFINEMENT") + assertThat(originalInfo.tryToCloneWithAppliedRefinement(refinement)).isNull() + } + @Test fun testNewMultiDisplayResolveInfo() { val intent = Intent(Intent.ACTION_SEND) @@ -204,12 +304,93 @@ class TargetInfoTest { assertThat(multiTargetInfo.hasSelected()).isTrue() assertThat(multiTargetInfo.getSelectedTarget()).isEqualTo(secondTargetInfo) - val multiTargetInfoClone = multiTargetInfo.cloneFilledIn(Intent(), 0) - assertThat(multiTargetInfoClone).isInstanceOf(MultiDisplayResolveInfo::class.java) - assertThat((multiTargetInfoClone as MultiDisplayResolveInfo).hasSelected()) + val refined = multiTargetInfo.tryToCloneWithAppliedRefinement(intent) + assertThat(refined).isInstanceOf(MultiDisplayResolveInfo::class.java) + assertThat((refined as MultiDisplayResolveInfo).hasSelected()) .isEqualTo(multiTargetInfo.hasSelected()) // TODO: consider exercising activity-start behavior. // TODO: consider exercising DisplayResolveInfo base class behavior. } + + @Test + fun testNewMultiDisplayResolveInfo_getAllSourceIntents_fromSelectedTarget() { + val sendImage = Intent("SEND").apply { type = "image/png" } + val sendUri = Intent("SEND").apply { type = "text/uri" } + + val resolveInfo = ResolverDataProvider.createResolveInfo(1, 0) + + val imageOnlyTarget = DisplayResolveInfo.newDisplayResolveInfo( + sendImage, + resolveInfo, + "Send Image", + "Sends only images", + sendImage, + /* resolveInfoPresentationGetter= */ null) + + val textOnlyTarget = DisplayResolveInfo.newDisplayResolveInfo( + sendUri, + resolveInfo, + "Send Text", + "Sends only text", + sendUri, + /* resolveInfoPresentationGetter= */ null) + + val imageOrTextTarget = DisplayResolveInfo.newDisplayResolveInfo( + sendImage, + resolveInfo, + "Send Image or Text", + "Sends images or text", + sendImage, + /* resolveInfoPresentationGetter= */ null + ).apply { + addAlternateSourceIntent(sendUri) + } + + val multiTargetInfo = MultiDisplayResolveInfo.newMultiDisplayResolveInfo( + listOf(imageOnlyTarget, textOnlyTarget, imageOrTextTarget) + ) + + multiTargetInfo.setSelected(0) + assertThat(multiTargetInfo.selectedTarget).isEqualTo(imageOnlyTarget) + assertThat(multiTargetInfo.allSourceIntents).isEqualTo(imageOnlyTarget.allSourceIntents) + + multiTargetInfo.setSelected(1) + assertThat(multiTargetInfo.selectedTarget).isEqualTo(textOnlyTarget) + assertThat(multiTargetInfo.allSourceIntents).isEqualTo(textOnlyTarget.allSourceIntents) + + multiTargetInfo.setSelected(2) + assertThat(multiTargetInfo.selectedTarget).isEqualTo(imageOrTextTarget) + assertThat(multiTargetInfo.allSourceIntents).isEqualTo(imageOrTextTarget.allSourceIntents) + } + + @Test + fun testNewMultiDisplayResolveInfo_tryToCloneWithAppliedRefinement_delegatedToSelectedTarget() { + val refined = Intent("SEND") + val sendImage = Intent("SEND") + val targetOne = spy( + DisplayResolveInfo.newDisplayResolveInfo( + sendImage, + ResolverDataProvider.createResolveInfo(1, 0), + "Target One", + "Target One", + sendImage, + /* resolveInfoPresentationGetter= */ null + ) + ) + val targetTwo = mock { + whenever(tryToCloneWithAppliedRefinement(any())).thenReturn(this) + } + + val multiTargetInfo = MultiDisplayResolveInfo.newMultiDisplayResolveInfo( + listOf(targetOne, targetTwo) + ) + + multiTargetInfo.setSelected(1) + assertThat(multiTargetInfo.selectedTarget).isEqualTo(targetTwo) + + multiTargetInfo.tryToCloneWithAppliedRefinement(refined) + verify(targetTwo, times(1)).tryToCloneWithAppliedRefinement(refined) + verify(targetOne, never()).tryToCloneWithAppliedRefinement(any()) + } } -- cgit v1.2.3-59-g8ed1b