Don't expose `ChooserTarget` from `TargetInfo` API.
`ChooserTarget` is a deprecated API left over from the (now long-gone)
`ChooserTargetService` system, but internally, `SelectableTargetInfo`
still relies on this type for some basic record-keeping. The
`ChooserTarget`s today come from two sources; they're either passed in
from the caller via `EXTRA_CHOOSER_TARGETS`, or synthesized internally
to shoehorn non-`ChooserTarget` data into a `SelectableTargetInfo`.
Prior to this CL, clients throughout the application might reach
through to the composed-in `ChooserTarget` for some fields stored in
that record type. After this CL, any fields accessed in that way are
instead lifted to the `TargetInfo` API so that clients can access them
directly, without going through a `ChooserTarget`. Thus "consuming"
clients don't need to be aware of the `ChooserTarget`, and from their
perspective it's an internal implementation detail of
`SelectableTargetInfo`.
In a first subsequent CL, these fields can be precomputed from the
`ChooserTarget` during the construction of the `SelectableTargetInfo`
so that we don't retain any instances once we've extracted the
relevant data. In a second subsequent CL, we can expose those fields
as initialization parameters; then instead of synthesizing
intermediate `ChooserTarget`s, we can build our synthetic targets as
`SelectableTargetInfo`s directly. The usage in `EXTRA_CHOOSER_TARGETS`
is regrettably part of our public API that was overlooked in the
`ChooserTargetService` deprecation and won't be going away any time
soon, but if we can adapt them to the `SelectableTargetInfo`
representation immediately when we read them out of the request, we
should be able to cut out any other remaining references to
`ChooserTarget`.
Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: I931234b0912952a4ccf5f94a88694ac82b527ae4
diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index 96f2f9c..76c6fb2 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -1723,12 +1723,7 @@
targetInfo.isSelectableTargetInfo() ? getTargetIntentFilter() : null;
String shortcutTitle = targetInfo.isSelectableTargetInfo()
? targetInfo.getDisplayLabel().toString() : null;
- String shortcutIdKey = targetInfo.isSelectableTargetInfo()
- ? targetInfo
- .getChooserTarget()
- .getIntentExtras()
- .getString(Intent.EXTRA_SHORTCUT_ID)
- : null;
+ String shortcutIdKey = targetInfo.getDirectShareShortcutId();
ChooserTargetActionsDialogFragment.show(
getSupportFragmentManager(),
@@ -1816,15 +1811,7 @@
switch (currentListAdapter.getPositionTargetType(which)) {
case ChooserListAdapter.TARGET_SERVICE:
cat = MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET;
- // Log the package name + target name to answer the question if most users
- // share to mostly the same person or to a bunch of different people.
- ChooserTarget target = targetInfo.getChooserTarget();
- directTargetHashed = HashedStringCache.getInstance().hashString(
- this,
- TAG,
- target.getComponentName().getPackageName()
- + target.getTitle().toString(),
- mMaxHashSaltDays);
+ directTargetHashed = targetInfo.getHashedTargetIdForMetrics(this);
directTargetAlsoRanked = getRankedPosition(targetInfo);
if (mCallerChooserTargets != null) {
@@ -1894,7 +1881,7 @@
private int getRankedPosition(TargetInfo targetInfo) {
String targetPackageName =
- targetInfo.getChooserTarget().getComponentName().getPackageName();
+ targetInfo.getChooserTargetComponentName().getPackageName();
ChooserListAdapter currentListAdapter =
mChooserMultiProfilePagerAdapter.getActiveListAdapter();
int maxRankedResults = Math.min(currentListAdapter.mDisplayList.size(),
@@ -2165,7 +2152,7 @@
ShortcutInfo shortcutInfo = chooserTargetInfo.getDirectShareShortcutInfo();
if (shortcutInfo != null) {
ComponentName componentName =
- chooserTargetInfo.getChooserTarget().getComponentName();
+ chooserTargetInfo.getChooserTargetComponentName();
targetIds.add(new AppTargetId(
String.format(
"%s/%s/%s",
diff --git a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
index 2de901c..8b9bfb3 100644
--- a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
@@ -16,9 +16,6 @@
package com.android.intentresolver.chooser;
-import android.service.chooser.ChooserTarget;
-import android.text.TextUtils;
-
import java.util.ArrayList;
import java.util.Arrays;
@@ -42,24 +39,4 @@
}
return new ArrayList<>(Arrays.asList(getDisplayResolveInfo()));
}
-
- @Override
- public boolean isSimilar(TargetInfo other) {
- if (other == null) return false;
-
- ChooserTarget ct1 = getChooserTarget();
- ChooserTarget ct2 = other.getChooserTarget();
-
- // If either is null, there is not enough info to make an informed decision
- // about equality, so just exit
- if (ct1 == null || ct2 == null) return false;
-
- if (ct1.getComponentName().equals(ct2.getComponentName())
- && TextUtils.equals(getDisplayLabel(), other.getDisplayLabel())
- && TextUtils.equals(getExtendedInfo(), other.getExtendedInfo())) {
- return true;
- }
-
- return false;
- }
}
diff --git a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java
index 66e2022..8ec52c8 100644
--- a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java
@@ -25,7 +25,6 @@
import android.graphics.drawable.Drawable;
import android.os.Bundle;
import android.os.UserHandle;
-import android.service.chooser.ChooserTarget;
import com.android.intentresolver.R;
import com.android.intentresolver.ResolverActivity;
@@ -131,10 +130,6 @@
return -0.1f;
}
- public ChooserTarget getChooserTarget() {
- return null;
- }
-
public boolean isSuspended() {
return false;
}
diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
index 3a3c3e6..7e6e49f 100644
--- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
@@ -34,8 +34,10 @@
import android.graphics.drawable.Icon;
import android.os.Bundle;
import android.os.UserHandle;
+import android.provider.DeviceConfig;
import android.service.chooser.ChooserTarget;
import android.text.SpannableStringBuilder;
+import android.util.HashedStringCache;
import android.util.Log;
import com.android.intentresolver.ChooserActivity;
@@ -43,6 +45,7 @@
import com.android.intentresolver.ResolverListAdapter.ActivityInfoPresentationGetter;
import com.android.intentresolver.SimpleIconFactory;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.config.sysui.SystemUiDeviceConfigFlags;
import java.util.ArrayList;
import java.util.List;
@@ -54,6 +57,14 @@
public final class SelectableTargetInfo extends ChooserTargetInfo {
private static final String TAG = "SelectableTargetInfo";
+ private static final String HASHED_STRING_CACHE_TAG = "ChooserActivity"; // For legacy reasons.
+ private static final int DEFAULT_SALT_EXPIRATION_DAYS = 7;
+
+ private final int mMaxHashSaltDays = DeviceConfig.getInt(
+ DeviceConfig.NAMESPACE_SYSTEMUI,
+ SystemUiDeviceConfigFlags.HASH_SALT_MAX_DAYS,
+ DEFAULT_SALT_EXPIRATION_DAYS);
+
private final Context mContext;
@Nullable
private final DisplayResolveInfo mSourceInfo;
@@ -280,6 +291,11 @@
return null;
}
+ @Override
+ public ComponentName getChooserTargetComponentName() {
+ return mChooserTarget.getComponentName();
+ }
+
private Intent getBaseIntentToSend() {
Intent result = getResolvedIntent();
if (result == null) {
@@ -359,11 +375,6 @@
}
@Override
- public ChooserTarget getChooserTarget() {
- return mChooserTarget;
- }
-
- @Override
@Nullable
public ShortcutInfo getDirectShareShortcutInfo() {
return mShortcutInfo;
@@ -395,6 +406,18 @@
return mIsPinned;
}
+ @Override
+ public HashedStringCache.HashResult getHashedTargetIdForMetrics(Context context) {
+ final String plaintext =
+ mChooserTarget.getComponentName().getPackageName()
+ + mChooserTarget.getTitle().toString();
+ return HashedStringCache.getInstance().hashString(
+ context,
+ HASHED_STRING_CACHE_TAG,
+ plaintext,
+ mMaxHashSaltDays);
+ }
+
@Nullable
private static ApplicationInfo getApplicationInfoFromSource(
@Nullable DisplayResolveInfo sourceInfo) {
diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java
index 29c4cfc..46cd53c 100644
--- a/java/src/com/android/intentresolver/chooser/TargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java
@@ -29,6 +29,8 @@
import android.os.Bundle;
import android.os.UserHandle;
import android.service.chooser.ChooserTarget;
+import android.text.TextUtils;
+import android.util.HashedStringCache;
import com.android.intentresolver.ResolverActivity;
@@ -52,13 +54,34 @@
/**
* Get the resolved component name that represents this target. Note that this may not
* be the component that will be directly launched by calling one of the <code>start</code>
- * methods provided; this is the component that will be credited with the launch.
+ * methods provided; this is the component that will be credited with the launch. This may be
+ * null if the target was specified by a caller-provided {@link ChooserTarget} that we failed to
+ * resolve to a component on the system.
*
* @return the resolved ComponentName for this target
*/
+ @Nullable
ComponentName getResolvedComponentName();
/**
+ * If this target was historically built from a (now-deprecated) {@link ChooserTarget} record,
+ * get the {@link ComponentName} that would've been provided by that record.
+ *
+ * TODO: for (historical) {@link ChooserTargetInfo} targets, this differs from the result of
+ * {@link #getResolvedComponentName()} only for caller-provided targets that we fail to resolve;
+ * then this returns the name of the component that was requested, and the other returns null.
+ * At the time of writing, this method is only called in contexts where the client knows that
+ * the target was a historical {@link ChooserTargetInfo}. Thus this method could be removed and
+ * all clients consolidated on the other, if we have some alternate mechanism of tracking this
+ * discrepancy; or if we know that the distinction won't apply in the conditions when we call
+ * this method; or if we determine that tracking the distinction isn't a requirement for us.
+ */
+ @Nullable
+ default ComponentName getChooserTargetComponentName() {
+ return null;
+ }
+
+ /**
* Start the activity referenced by this target.
*
* @param activity calling Activity performing the launch
@@ -172,7 +195,25 @@
* presumably resolved by converting {@code TargetInfo} from an interface to an abstract class.
*/
default boolean isSimilar(TargetInfo other) {
- return Objects.equals(this, other);
+ if (other == null) {
+ return false;
+ }
+
+ // TODO: audit usage and try to reconcile a behavior that doesn't depend on the legacy
+ // subclass type. Note that the `isSimilar()` method was pulled up from the legacy
+ // `ChooserTargetInfo`, so no legacy behavior currently depends on calling `isSimilar()` on
+ // an instance where `isChooserTargetInfo()` would return false (although technically it may
+ // have been possible for the `other` target to be of a different type). Thus we have
+ // flexibility in defining the similarity conditions between pairs of non "chooser" targets.
+ if (isChooserTargetInfo()) {
+ return other.isChooserTargetInfo()
+ && Objects.equals(
+ getChooserTargetComponentName(), other.getChooserTargetComponentName())
+ && TextUtils.equals(getDisplayLabel(), other.getDisplayLabel())
+ && TextUtils.equals(getExtendedInfo(), other.getExtendedInfo());
+ } else {
+ return !other.isChooserTargetInfo() && Objects.equals(this, other);
+ }
}
/**
@@ -186,24 +227,6 @@
}
/**
- * @return the {@link ChooserTarget} record that contains additional data about this target, if
- * any. This is only non-null for selectable targets (and probably only Direct Share targets?).
- *
- * @deprecated {@link ChooserTarget} (and any other related {@code ChooserTargetService} APIs)
- * got deprecated as part of sunsetting that old system design, but for historical reasons
- * Chooser continues to shoehorn data from other sources into this representation to maintain
- * compatibility with legacy internal APIs. New clients should avoid taking any further
- * dependencies on the {@link ChooserTarget} type; any data they want to query from those
- * records should instead be pulled up to new query methods directly on this class (or on the
- * root {@link TargetInfo}).
- */
- @Deprecated
- @Nullable
- default ChooserTarget getChooserTarget() {
- return null;
- }
-
- /**
* @return the {@link ShortcutManager} data for any shortcut associated with this target.
*/
@Nullable
@@ -212,6 +235,19 @@
}
/**
+ * @return the ID of the shortcut represented by this target, or null if the target didn't come
+ * from a {@link ShortcutManager} shortcut.
+ */
+ @Nullable
+ default String getDirectShareShortcutId() {
+ ShortcutInfo shortcut = getDirectShareShortcutInfo();
+ if (shortcut == null) {
+ return null;
+ }
+ return shortcut.getId();
+ }
+
+ /**
* @return the {@link AppTarget} metadata if this target was sourced from App Prediction
* service, or null otherwise.
*/
@@ -255,11 +291,11 @@
* @return true if this target represents a legacy {@code ChooserTargetInfo}. These objects were
* historically documented as representing "[a] TargetInfo for Direct Share." However, not all
* of these targets are actually *valid* for direct share; e.g. some represent "empty" items
- * (although perhaps only for display in the Direct Share UI?). {@link #getChooserTarget()} will
- * return null for any of these "invalid" items. In even earlier versions, these targets may
- * also have been results from (now-deprecated/unsupported) {@code ChooserTargetService} peers;
- * even though we no longer use these services, we're still shoehorning other target data into
- * the deprecated {@link ChooserTarget} structure for compatibility with some internal APIs.
+ * (although perhaps only for display in the Direct Share UI?). In even earlier versions, these
+ * targets may also have been results from peers in the (now-deprecated/unsupported)
+ * {@code ChooserTargetService} ecosystem; even though we no longer use these services, we're
+ * still shoehorning other target data into the deprecated {@link ChooserTarget} structure for
+ * compatibility with some internal APIs.
* TODO: refactor to clarify the semantics of any target for which this method returns true
* (e.g., are they characterized by their application in the Direct Share UI?), and to remove
* the scaffolding that adapts to and from the {@link ChooserTarget} structure. Eventually, we
@@ -352,6 +388,19 @@
}
/**
+ * @param context caller's context, to provide the {@link SharedPreferences} for use by the
+ * {@link HashedStringCache}.
+ * @return a hashed ID that should be logged along with our target-selection metrics, or null.
+ * The contents of the plaintext are defined for historical reasons, "the package name + target
+ * name to answer the question if most users share to mostly the same person
+ * or to a bunch of different people." Clients should consider this as opaque data for logging
+ * only; they should not rely on any particular semantics about the value.
+ */
+ default HashedStringCache.HashResult getHashedTargetIdForMetrics(Context context) {
+ return null;
+ }
+
+ /**
* Fix the URIs in {@code intent} if cross-profile sharing is required. This should be called
* before launching the intent as another user.
*/
diff --git a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
index f45d592..8581ed0 100644
--- a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
+++ b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
@@ -261,6 +261,8 @@
)
}
+ // TODO: consider renaming. Not all `ChooserTarget`s are "shortcuts" and many of our test cases
+ // add results with `isShortcutResult = false` and `directShareToShortcutInfos = emptyMap()`.
private fun assertShortcutsInOrder(
expected: List<ChooserTarget>, actual: List<TargetInfo>, msg: String? = ""
) {
@@ -268,8 +270,13 @@
for (i in expected.indices) {
assertEquals(
"Unexpected item at position $i",
- expected[i],
- actual[i].chooserTarget
+ expected[i].componentName,
+ actual[i].chooserTargetComponentName
+ )
+ assertEquals(
+ "Unexpected item at position $i",
+ expected[i].title,
+ actual[i].displayLabel
)
}
}
diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
index b6d0962..ae9c0f8 100644
--- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
+++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
@@ -76,7 +76,9 @@
assertThat(targetInfo.isSelectableTargetInfo()).isTrue()
assertThat(targetInfo.isChooserTargetInfo()).isTrue() // From legacy inheritance model.
assertThat(targetInfo.getDisplayResolveInfo()).isSameInstanceAs(displayInfo)
- assertThat(targetInfo.getChooserTarget()).isSameInstanceAs(chooserTarget)
+ assertThat(targetInfo.getChooserTargetComponentName())
+ .isEqualTo(chooserTarget.getComponentName())
+ assertThat(targetInfo.getDirectShareShortcutId()).isEqualTo(shortcutInfo.getId())
assertThat(targetInfo.getDirectShareShortcutInfo()).isSameInstanceAs(shortcutInfo)
assertThat(targetInfo.getDirectShareAppTarget()).isSameInstanceAs(appTarget)
// TODO: make more meaningful assertions about the behavior of a selectable target.
@@ -147,4 +149,4 @@
// TODO: consider exercising activity-start behavior.
// TODO: consider exercising DisplayResolveInfo base class behavior.
}
-}
\ No newline at end of file
+}
