From fb433f9d327ee66e880d7b93bca74c9fd81f0ad2 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Thu, 13 Oct 2022 14:28:12 -0400 Subject: Pull ChooserTargetInfo API up to base TargetInfo. As in ag/20189669, this makes the concrete type into an implementation detail while allowing clients to operate in terms of the more general base interface (with an eye, long-term, towards potentially "flattening" this class hierarchy, per go/chooser-targetinfo-cleanup). After this CL, clients that only "access" but don't "create" target objects shouldn't need to know the concrete type of any instance on ChooserTargetInfo side of the tree (i.e., clients only potentially need to care about the runtime type of targets that descend from DisplayResolveInfo -- until that side too can be cleaned up in a future CL). Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: I47d54c64971e1c0ca2a4953d347eb4323a598f5c --- .../android/intentresolver/ChooserActivity.java | 31 ++++----- .../android/intentresolver/ChooserListAdapter.java | 21 +++--- .../intentresolver/ShortcutSelectionLogic.java | 12 ++-- .../intentresolver/chooser/ChooserTargetInfo.java | 50 +-------------- .../chooser/NotSelectableTargetInfo.java | 4 +- .../android/intentresolver/chooser/TargetInfo.java | 74 ++++++++++++++++++++++ 6 files changed, 108 insertions(+), 84 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 9d4515fa..63ab20cd 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -116,7 +116,6 @@ import androidx.viewpager.widget.ViewPager; import com.android.intentresolver.ResolverListAdapter.ActivityInfoPresentationGetter; import com.android.intentresolver.ResolverListAdapter.ViewHolder; -import com.android.intentresolver.chooser.ChooserTargetInfo; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.MultiDisplayResolveInfo; import com.android.intentresolver.chooser.SelectableTargetInfo.SelectableTargetInfoCommunicator; @@ -1711,24 +1710,23 @@ public class ChooserActivity extends ResolverActivity implements Bundle bundle = new Bundle(); if (targetInfo.isSelectableTargetInfo()) { - ChooserTargetInfo selectableTargetInfo = (ChooserTargetInfo) targetInfo; - if (selectableTargetInfo.getDisplayResolveInfo() == null - || selectableTargetInfo.getChooserTarget() == null) { + if (targetInfo.getDisplayResolveInfo() == null + || targetInfo.getChooserTarget() == null) { Log.e(TAG, "displayResolveInfo or chooserTarget in selectableTargetInfo are null"); return; } targetList = new ArrayList<>(); - targetList.add(selectableTargetInfo.getDisplayResolveInfo()); + targetList.add(targetInfo.getDisplayResolveInfo()); bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_ID_KEY, - selectableTargetInfo.getChooserTarget().getIntentExtras().getString( + targetInfo.getChooserTarget().getIntentExtras().getString( Intent.EXTRA_SHORTCUT_ID)); bundle.putBoolean(ChooserTargetActionsDialogFragment.IS_SHORTCUT_PINNED_KEY, - selectableTargetInfo.isPinned()); + targetInfo.isPinned()); bundle.putParcelable(ChooserTargetActionsDialogFragment.INTENT_FILTER_KEY, getTargetIntentFilter()); - if (selectableTargetInfo.getDisplayLabel() != null) { + if (targetInfo.getDisplayLabel() != null) { bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_TITLE_KEY, - selectableTargetInfo.getDisplayLabel().toString()); + targetInfo.getDisplayLabel().toString()); } } else if (targetInfo.isMultiDisplayResolveInfo()) { // For multiple targets, include info on all targets @@ -1830,15 +1828,14 @@ public class ChooserActivity extends ResolverActivity implements 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. - ChooserTargetInfo selectableTargetInfo = (ChooserTargetInfo) targetInfo; - ChooserTarget target = selectableTargetInfo.getChooserTarget(); + ChooserTarget target = targetInfo.getChooserTarget(); directTargetHashed = HashedStringCache.getInstance().hashString( this, TAG, target.getComponentName().getPackageName() + target.getTitle().toString(), mMaxHashSaltDays); - directTargetAlsoRanked = getRankedPosition(selectableTargetInfo); + directTargetAlsoRanked = getRankedPosition(targetInfo); if (mCallerChooserTargets != null) { numCallerProvided = mCallerChooserTargets.length; @@ -1847,7 +1844,7 @@ public class ChooserActivity extends ResolverActivity implements SELECTION_TYPE_SERVICE, targetInfo.getResolveInfo().activityInfo.processName, value, - selectableTargetInfo.isPinned() + targetInfo.isPinned() ); break; case ChooserListAdapter.TARGET_CALLER: @@ -1905,7 +1902,7 @@ public class ChooserActivity extends ResolverActivity implements } } - private int getRankedPosition(ChooserTargetInfo targetInfo) { + private int getRankedPosition(TargetInfo targetInfo) { String targetPackageName = targetInfo.getChooserTarget().getComponentName().getPackageName(); ChooserListAdapter currentListAdapter = @@ -2158,9 +2155,9 @@ public class ChooserActivity extends ResolverActivity implements if (targetInfo.isChooserTargetInfo()) { return; } - List surfacedTargetInfo = adapter.getSurfacedTargetInfo(); + List surfacedTargetInfo = adapter.getSurfacedTargetInfo(); List targetIds = new ArrayList<>(); - for (ChooserTargetInfo chooserTargetInfo : surfacedTargetInfo) { + for (TargetInfo chooserTargetInfo : surfacedTargetInfo) { ChooserTarget chooserTarget = chooserTargetInfo.getChooserTarget(); ComponentName componentName = chooserTarget.getComponentName(); if (mDirectShareShortcutInfoCache.containsKey(chooserTarget)) { @@ -2182,7 +2179,7 @@ public class ChooserActivity extends ResolverActivity implements if (!targetInfo.isChooserTargetInfo()) { return; } - ChooserTarget chooserTarget = ((ChooserTargetInfo) targetInfo).getChooserTarget(); + ChooserTarget chooserTarget = targetInfo.getChooserTarget(); AppTarget appTarget = null; if (mDirectShareAppTargetCache != null) { appTarget = mDirectShareAppTargetCache.get(chooserTarget); diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index 98e3dcdb..9c5c979c 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -44,7 +44,6 @@ import android.view.ViewGroup; import android.widget.TextView; import com.android.intentresolver.ResolverActivity.ResolvedComponentInfo; -import com.android.intentresolver.chooser.ChooserTargetInfo; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.MultiDisplayResolveInfo; import com.android.intentresolver.chooser.NotSelectableTargetInfo; @@ -86,9 +85,9 @@ public class ChooserListAdapter extends ResolverListAdapter { private final Map mIconLoaders = new HashMap<>(); // Reserve spots for incoming direct share targets by adding placeholders - private ChooserTargetInfo mPlaceHolderTargetInfo = + private TargetInfo mPlaceHolderTargetInfo = NotSelectableTargetInfo.newPlaceHolderTargetInfo(); - private final List mServiceTargets = new ArrayList<>(); + private final List mServiceTargets = new ArrayList<>(); private final List mCallerTargets = new ArrayList<>(); private boolean mListViewDataChanged = false; @@ -272,14 +271,14 @@ public class ChooserListAdapter extends ResolverListAdapter { holder.bindIcon(info); if (info.isSelectableTargetInfo()) { // direct share targets should append the application name for a better readout - DisplayResolveInfo rInfo = ((ChooserTargetInfo) info).getDisplayResolveInfo(); + DisplayResolveInfo rInfo = info.getDisplayResolveInfo(); CharSequence appName = rInfo != null ? rInfo.getDisplayLabel() : ""; CharSequence extendedInfo = info.getExtendedInfo(); String contentDescription = String.join(" ", info.getDisplayLabel(), extendedInfo != null ? extendedInfo : "", appName); holder.updateContentDescription(contentDescription); if (!info.hasDisplayIcon()) { - loadDirectShareIcon((ChooserTargetInfo) info); + loadDirectShareIcon(info); } } else if (info.isDisplayResolveInfo()) { DisplayResolveInfo dri = (DisplayResolveInfo) info; @@ -325,7 +324,7 @@ public class ChooserListAdapter extends ResolverListAdapter { } } - private void loadDirectShareIcon(ChooserTargetInfo info) { + private void loadDirectShareIcon(TargetInfo info) { LoadDirectShareIconTask task = (LoadDirectShareIconTask) mIconLoaders.get(info); if (task == null) { task = createLoadDirectShareIconTask(info); @@ -335,7 +334,7 @@ public class ChooserListAdapter extends ResolverListAdapter { } @VisibleForTesting - protected LoadDirectShareIconTask createLoadDirectShareIconTask(ChooserTargetInfo info) { + protected LoadDirectShareIconTask createLoadDirectShareIconTask(TargetInfo info) { return new LoadDirectShareIconTask(info); } @@ -405,7 +404,7 @@ public class ChooserListAdapter extends ResolverListAdapter { */ public int getSelectableServiceTargetCount() { int count = 0; - for (ChooserTargetInfo info : mServiceTargets) { + for (TargetInfo info : mServiceTargets) { if (info.isSelectableTargetInfo()) { count++; } @@ -532,7 +531,7 @@ public class ChooserListAdapter extends ResolverListAdapter { /** * Fetch surfaced direct share target info */ - public List getSurfacedTargetInfo() { + public List getSurfacedTargetInfo() { int maxSurfacedTargets = mChooserListCommunicator.getMaxRankedTargets(); return mServiceTargets.subList(0, Math.min(maxSurfacedTargets, getSelectableServiceTargetCount())); @@ -676,9 +675,9 @@ public class ChooserListAdapter extends ResolverListAdapter { */ @VisibleForTesting public class LoadDirectShareIconTask extends AsyncTask { - private final ChooserTargetInfo mTargetInfo; + private final TargetInfo mTargetInfo; - private LoadDirectShareIconTask(ChooserTargetInfo targetInfo) { + private LoadDirectShareIconTask(TargetInfo targetInfo) { mTargetInfo = targetInfo; } diff --git a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java index 8ec227fc..07573d1b 100644 --- a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java +++ b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java @@ -22,10 +22,10 @@ import android.content.pm.ShortcutInfo; import android.service.chooser.ChooserTarget; import android.util.Log; -import com.android.intentresolver.chooser.ChooserTargetInfo; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.SelectableTargetInfo; import com.android.intentresolver.chooser.SelectableTargetInfo.SelectableTargetInfoCommunicator; +import com.android.intentresolver.chooser.TargetInfo; import java.util.Collections; import java.util.Comparator; @@ -65,7 +65,7 @@ class ShortcutSelectionLogic { Context userContext, SelectableTargetInfoCommunicator mSelectableTargetInfoCommunicator, int maxRankedTargets, - List serviceTargets) { + List serviceTargets) { if (DEBUG) { Log.d(TAG, "addServiceResults " + (origTarget == null ? null : origTarget.getResolvedComponentName()) + ", " @@ -126,12 +126,12 @@ class ShortcutSelectionLogic { } private boolean insertServiceTarget( - SelectableTargetInfo chooserTargetInfo, + TargetInfo chooserTargetInfo, int maxRankedTargets, - List serviceTargets) { + List serviceTargets) { // Check for duplicates and abort if found - for (ChooserTargetInfo otherTargetInfo : serviceTargets) { + for (TargetInfo otherTargetInfo : serviceTargets) { if (chooserTargetInfo.isSimilar(otherTargetInfo)) { return false; } @@ -141,7 +141,7 @@ class ShortcutSelectionLogic { final float newScore = chooserTargetInfo.getModifiedScore(); for (int i = 0; i < Math.min(currentSize, maxRankedTargets); i++) { - final ChooserTargetInfo serviceTarget = serviceTargets.get(i); + final TargetInfo serviceTarget = serviceTargets.get(i); if (serviceTarget == null) { serviceTargets.set(i, chooserTargetInfo); return true; diff --git a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java index a1ba87f8..38a2759e 100644 --- a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java @@ -16,7 +16,6 @@ package com.android.intentresolver.chooser; -import android.annotation.Nullable; import android.service.chooser.ChooserTarget; import android.text.TextUtils; @@ -25,59 +24,14 @@ import android.text.TextUtils; * Direct Share deep link into an application. */ public abstract class ChooserTargetInfo implements TargetInfo { - /** - * @return the target score, including any Chooser-specific modifications that may have been - * applied (either overriding by special-case for "non-selectable" targets, or by twiddling the - * scores of "selectable" targets in {@link ChooserListAdapter}). Higher scores are "better." - */ - public abstract float getModifiedScore(); - - /** - * @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 - public abstract ChooserTarget getChooserTarget(); @Override public final boolean isChooserTargetInfo() { return true; } - /** - * Attempt to load the display icon, if we have the info for one but it hasn't been loaded yet. - * @return true if an icon may have been loaded as the result of this operation, potentially - * prompting a UI refresh. If this returns false, clients can safely assume there was no change. - */ - public boolean loadIcon() { - return false; - } - - /** - * Get more info about this target in the form of a {@link DisplayResolveInfo}, if available. - * TODO: determine the meaning of a TargetInfo (ChooserTargetInfo) embedding another kind of - * TargetInfo (DisplayResolveInfo) in this way, and - at least - improve this documentation; - * OTOH this probably indicates an opportunity to simplify or better separate these APIs. - */ - @Nullable - public DisplayResolveInfo getDisplayResolveInfo() { - return null; - } - - /** - * Do not label as 'equals', since this doesn't quite work - * as intended with java 8. - */ - public boolean isSimilar(ChooserTargetInfo other) { + @Override + public boolean isSimilar(TargetInfo other) { if (other == null) return false; ChooserTarget ct1 = getChooserTarget(); diff --git a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java index 3a488e32..66e2022c 100644 --- a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java @@ -38,7 +38,7 @@ import java.util.List; */ public abstract class NotSelectableTargetInfo extends ChooserTargetInfo { /** Create a non-selectable {@link TargetInfo} with no content. */ - public static ChooserTargetInfo newEmptyTargetInfo() { + public static TargetInfo newEmptyTargetInfo() { return new NotSelectableTargetInfo() { @Override public boolean isEmptyTargetInfo() { @@ -61,7 +61,7 @@ public abstract class NotSelectableTargetInfo extends ChooserTargetInfo { * Create a non-selectable {@link TargetInfo} with placeholder content to be displayed * unless/until it can be replaced by the result of a pending asynchronous load. */ - public static ChooserTargetInfo newPlaceHolderTargetInfo() { + public static TargetInfo newPlaceHolderTargetInfo() { return new NotSelectableTargetInfo() { @Override public boolean isPlaceHolderTargetInfo() { diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java index 220b6467..4842cd80 100644 --- a/java/src/com/android/intentresolver/chooser/TargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java @@ -17,6 +17,7 @@ package com.android.intentresolver.chooser; +import android.annotation.Nullable; import android.app.Activity; import android.content.ComponentName; import android.content.Context; @@ -25,10 +26,12 @@ import android.content.pm.ResolveInfo; import android.graphics.drawable.Drawable; import android.os.Bundle; import android.os.UserHandle; +import android.service.chooser.ChooserTarget; import com.android.intentresolver.ResolverActivity; import java.util.List; +import java.util.Objects; /** * A single target as represented in the chooser. @@ -135,6 +138,77 @@ public interface TargetInfo { */ boolean isPinned(); + /** + * Determine whether two targets represent "similar" content that could be de-duped. + * Note an earlier version of this code cautioned maintainers, + * "do not label as 'equals', since this doesn't quite work as intended with java 8." + * This seems to refer to the rule that interfaces can't provide defaults that conflict with the + * definitions of "real" methods in {@code java.lang.Object}, and (if desired) it could be + * presumably resolved by converting {@code TargetInfo} from an interface to an abstract class. + */ + default boolean isSimilar(TargetInfo other) { + return Objects.equals(this, other); + } + + /** + * @return the target score, including any Chooser-specific modifications that may have been + * applied (either overriding by special-case for "non-selectable" targets, or by twiddling the + * scores of "selectable" targets in {@link ChooserListAdapter}). Higher scores are "better." + * Targets that aren't intended for ranking/scoring should return a negative value. + */ + default float getModifiedScore() { + return -0.1f; + } + + /** + * @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; + } + + /** + * Attempt to load the display icon, if we have the info for one but it hasn't been loaded yet. + * @return true if an icon may have been loaded as the result of this operation, potentially + * prompting a UI refresh. If this returns false, clients can safely assume there was no change. + */ + default boolean loadIcon() { + return false; + } + + /** + * Get more info about this target in the form of a {@link DisplayResolveInfo}, if available. + * TODO: this seems to return non-null only for ChooserTargetInfo subclasses. Determine the + * meaning of a TargetInfo (ChooserTargetInfo) embedding another kind of TargetInfo + * (DisplayResolveInfo) in this way, and - at least - improve this documentation; OTOH this + * probably indicates an opportunity to simplify or better separate these APIs. (For example, + * targets that don't descend from ChooserTargetInfo instead descend directly from + * DisplayResolveInfo; should they return `this`? Do we always use DisplayResolveInfo to + * represent visual properties, and then either assume some implicit metadata properties *or* + * embed that visual representation within a ChooserTargetInfo to carry additional metadata? If + * that's the case, maybe we could decouple by saying that all TargetInfos compose-in their + * visual representation [as a DisplayResolveInfo, now the root of its own class hierarchy] and + * then add a new TargetInfo type that explicitly represents the "implicit metadata" that we + * previously assumed for "naked DisplayResolveInfo targets" that weren't wrapped as + * ChooserTargetInfos. Or does all this complexity disappear once we stop relying on the + * deprecated ChooserTarget type?) + */ + @Nullable + default DisplayResolveInfo getDisplayResolveInfo() { + return null; + } + /** * @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 -- cgit v1.2.3-59-g8ed1b