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 --- .../chooser/ImmutableTargetInfoTest.kt | 117 +++++++++---- .../intentresolver/chooser/TargetInfoTest.kt | 189 ++++++++++++++++++++- 2 files changed, 269 insertions(+), 37 deletions(-) (limited to 'java/tests') 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