Merge "Allow refinement of more non-filter-equals fields." into udc-dev am: 3374bdb665 am: 3afed5b83e

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/IntentResolver/+/22748083

Change-Id: Ic02b50165dfba6b24252f5fa2a3909d1569b5d7d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

4 files changed, 54 insertions, 8 deletions

diff --git a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
index 29be6dc6..09cf319f 100644
--- a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
+++ b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
@@ -184,9 +184,10 @@ public class DisplayResolveInfo implements TargetInfo {
             return null;
         }
 
-        Intent merged = new Intent(matchingBase);
-        merged.fillIn(proposedRefinement, 0);
-        return new DisplayResolveInfo(this, merged, mPresentationGetter);
+        return new DisplayResolveInfo(
+                this,
+                TargetInfo.mergeRefinementIntoMatchingBaseIntent(matchingBase, proposedRefinement),
+                mPresentationGetter);
     }
 
     @Override
diff --git a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java
index 2d9683e1..10d4415a 100644
--- a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java
@@ -427,8 +427,8 @@ public final class ImmutableTargetInfo implements TargetInfo {
             return null;
         }
 
-        Intent merged = new Intent(matchingBase);
-        merged.fillIn(proposedRefinement, 0);
+        Intent merged = TargetInfo.mergeRefinementIntoMatchingBaseIntent(
+                matchingBase, proposedRefinement);
         return toBuilder().setBaseIntentToSend(merged).build();
     }
 
diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
index 74c19e67..5766db0e 100644
--- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
@@ -279,9 +279,9 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
             return null;
         }
 
-        Intent merged = new Intent(matchingBase);
-        merged.fillIn(proposedRefinement, 0);
-        return new SelectableTargetInfo(this, merged);
+        return new SelectableTargetInfo(
+                this,
+                TargetInfo.mergeRefinementIntoMatchingBaseIntent(matchingBase, proposedRefinement));
     }
 
     @Override
diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java
index 2f48704c..9d793994 100644
--- a/java/src/com/android/intentresolver/chooser/TargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java
@@ -454,4 +454,49 @@ public interface TargetInfo {
             intent.fixUris(currentUserId);
         }
     }
+
+    /**
+     * Derive a "complete" intent from a proposed `refinement` intent by merging it into a matching
+     * `base` intent, without modifying the filter-equality properties of the `base` intent, while
+     * still allowing the `refinement` to replace Share "payload" fields.
+     * Note! Callers are responsible for ensuring that the `base` is a suitable match for the given
+     * `refinement`, such that the two can be merged without modifying filter-equality properties.
+     */
+    static Intent mergeRefinementIntoMatchingBaseIntent(Intent base, Intent refinement) {
+        Intent mergedIntent = new Intent(base);
+
+        /* Copy over any fields from the `refinement` that weren't already specified by the `base`,
+         * along with the refined ClipData (if present, even if that overwrites data given in the
+         * `base` intent).
+         *
+         * Refinement may have modified the payload content stored in the ClipData; such changes
+         * are permitted in refinement since ClipData isn't a factor in the determination of
+         * `Intent.filterEquals()` (which must be preserved as an invariant of refinement). */
+        mergedIntent.fillIn(refinement, Intent.FILL_IN_CLIP_DATA);
+
+        /* Refinement may also modify payload content held in the 'extras' representation, as again
+         * those attributes aren't a factor in determining filter-equality. There is no `FILL_IN_*`
+         * flag that would allow the refinement to overwrite existing keys in the `base` extras, so
+         * here we have to implement the logic ourselves.
+         *
+         * Note this still doesn't imply that the refined intent is the final authority on extras;
+         * in particular, `SelectableTargetInfo.mActivityStarter` uses `Intent.putExtras(Bundle)` to
+         * merge in the `mChooserTargetIntentExtras` (i.e., the `EXTRA_SHORTCUT_ID`), which will
+         * overwrite any existing value.
+         *
+         * TODO: formalize the precedence and make sure extras are set in the appropriate stages,
+         * instead of relying on maintainers to know that (e.g.) authoritative changes belong in the
+         * `TargetActivityStarter`. Otherwise, any extras-based data that Sharesheet adds internally
+         * might be susceptible to "spoofing" from the refinement activity. */
+        mergedIntent.putExtras(refinement);  // Re-merge extras to favor refinement.
+
+        // TODO(b/279067078): consider how to populate the "merged" ClipData. The `base`
+        // already has non-null ClipData due to the implicit migration in Intent, so if the 
+        // refinement modified any of the payload extras, they *must* also provide a modified
+        // ClipData, or else the updated "extras" payload will be inconsistent with the
+        // pre-refinement ClipData when they're merged together. We may be able to do better,
+        // but there are complicated tradeoffs.
+
+        return mergedIntent;
+    }
 }