Expose ShortcutInfo & AppTarget from TargetInfo

This allows `TargetInfo` clients to look up the same auxiliary source
data that was already retained in `ChooserActivity`'s "cache"
structures, but it removes the extra complexity of holding that data
outside of the target itself only to need to look it back up again
when we try to use the target later. (For now this CL leaves the
auxiliary "cache" structures in place for one specific usage that's
out-of-scope to refactor here, but it removes any other indeterminate
subsequent uses). Incidentally, those "subsequent uses" that are
removed were some of the only remaining places where we made calls to
`TargetInfo.getChooserTarget()`, and we're attempting to burn down
all the remaining calls to *that* API since `ChooserTarget` is
deprecated.

`SelectableTargetInfo` objects aleady retain the (nullable)
`ShortcutInfo`, but for historical reasons it's been used as mutable
state related to icon loading and could sometimes be reset to null. In
order to expose the `ShortcutInfo` this CL makes it a `final` member
and introduces an explicit state bit to replace the null-reset used in
the past. This CL also makes some other minor cleanups in
`SelectableTargetInfo` to clarify the immutability (precomputability)
of some additional fields (to support unrelated upcoming CLs, while
I happen to be making similar changes in here anyways). This brought
attention to a likely bug where the target `mIsSuspended` bit was
neglected in the implementation of `SelectableTargetInfo`'s "copy
constructor" (fixed in this CL).

Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: I1a18a1aea3acdded7c360311bf216f05fc75cb67

5 files changed, 145 insertions, 58 deletions

diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index a73424bf..193956f9 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -190,6 +190,11 @@ public class ChooserActivity extends ResolverActivity implements
 
     private static final String IMAGE_EDITOR_SHARED_ELEMENT = "screenshot_preview_image";
 
+    // TODO: these data structures are for one-time use in shuttling data from where they're
+    // populated in `ShortcutToChooserTargetConverter` to where they're consumed in
+    // `ShortcutSelectionLogic` which packs the appropriate elements into the final `TargetInfo`.
+    // That flow should be refactored so that `ChooserActivity` isn't responsible for holding their
+    // intermediate data, and then these members can be removed.
     private Map<ChooserTarget, AppTarget> mDirectShareAppTargetCache;
     private Map<ChooserTarget, ShortcutInfo> mDirectShareShortcutInfoCache;
 
@@ -494,7 +499,8 @@ public class ChooserActivity extends ResolverActivity implements
                                 adapterForUserHandle.addServiceResults(
                                         resultInfo.originalTarget,
                                         resultInfo.resultTargets, msg.arg1,
-                                        mDirectShareShortcutInfoCache);
+                                        mDirectShareShortcutInfoCache,
+                                        mDirectShareAppTargetCache);
                             }
                         }
                     }
@@ -1683,7 +1689,8 @@ public class ChooserActivity extends ResolverActivity implements
                     /* origTarget */ null,
                     Lists.newArrayList(mCallerChooserTargets),
                     TARGET_TYPE_DEFAULT,
-                    /* directShareShortcutInfoCache */ null);
+                    /* directShareShortcutInfoCache */ null,
+                    /* directShareAppTargetCache */ null);
         }
     }
 
@@ -2164,12 +2171,15 @@ public class ChooserActivity extends ResolverActivity implements
         List<TargetInfo> surfacedTargetInfo = adapter.getSurfacedTargetInfo();
         List<AppTargetId> targetIds = new ArrayList<>();
         for (TargetInfo chooserTargetInfo : surfacedTargetInfo) {
-            ChooserTarget chooserTarget = chooserTargetInfo.getChooserTarget();
-            ComponentName componentName = chooserTarget.getComponentName();
-            if (mDirectShareShortcutInfoCache.containsKey(chooserTarget)) {
-                String shortcutId = mDirectShareShortcutInfoCache.get(chooserTarget).getId();
+            ShortcutInfo shortcutInfo = chooserTargetInfo.getDirectShareShortcutInfo();
+            if (shortcutInfo != null) {
+                ComponentName componentName =
+                        chooserTargetInfo.getChooserTarget().getComponentName();
                 targetIds.add(new AppTargetId(
-                        String.format("%s/%s/%s", shortcutId, componentName.flattenToString(),
+                        String.format(
+                                "%s/%s/%s",
+                                shortcutInfo.getId(),
+                                componentName.flattenToString(),
                                 SHORTCUT_TARGET)));
             }
         }
@@ -2186,13 +2196,12 @@ public class ChooserActivity extends ResolverActivity implements
         if (directShareAppPredictor == null) {
             return;
         }
-        ChooserTarget chooserTarget = targetInfo.getChooserTarget();
-        AppTarget appTarget = null;
-        if (mDirectShareAppTargetCache != null) {
-            appTarget = mDirectShareAppTargetCache.get(chooserTarget);
+        if (!targetInfo.isChooserTargetInfo()) {
+            return;
         }
-        // This is a direct share click that was provided by the APS
+        AppTarget appTarget = targetInfo.getDirectShareAppTarget();
         if (appTarget != null) {
+            // This is a direct share click that was provided by the APS
             directShareAppPredictor.notifyAppTargetEvent(
                     new AppTargetEvent.Builder(appTarget, AppTargetEvent.ACTION_LAUNCH)
                         .setLaunchLocation(LAUNCH_LOCATION_DIRECT_SHARE)
diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java
index 85b50ab6..25b50625 100644
--- a/java/src/com/android/intentresolver/ChooserListAdapter.java
+++ b/java/src/com/android/intentresolver/ChooserListAdapter.java
@@ -22,6 +22,7 @@ import static com.android.intentresolver.ChooserActivity.TARGET_TYPE_SHORTCUTS_F
 import android.annotation.Nullable;
 import android.app.ActivityManager;
 import android.app.prediction.AppPredictor;
+import android.app.prediction.AppTarget;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -544,7 +545,8 @@ public class ChooserListAdapter extends ResolverListAdapter {
             @Nullable DisplayResolveInfo origTarget,
             List<ChooserTarget> targets,
             @ChooserActivity.ShareTargetType int targetType,
-            Map<ChooserTarget, ShortcutInfo> directShareToShortcutInfos) {
+            Map<ChooserTarget, ShortcutInfo> directShareToShortcutInfos,
+            Map<ChooserTarget, AppTarget> directShareToAppTargets) {
         // Avoid inserting any potentially late results.
         if ((mServiceTargets.size() == 1) && mServiceTargets.get(0).isEmptyTargetInfo()) {
             return;
@@ -557,6 +559,7 @@ public class ChooserListAdapter extends ResolverListAdapter {
                 targets,
                 isShortcutResult,
                 directShareToShortcutInfos,
+                directShareToAppTargets,
                 mContext.createContextAsUser(getUserHandle(), 0),
                 mSelectableTargetInfoCommunicator,
                 mChooserListCommunicator.getMaxRankedTargets(),
diff --git a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java
index a278baae..39187bdb 100644
--- a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java
+++ b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java
@@ -17,6 +17,7 @@
 package com.android.intentresolver;
 
 import android.annotation.Nullable;
+import android.app.prediction.AppTarget;
 import android.content.Context;
 import android.content.pm.ShortcutInfo;
 import android.service.chooser.ChooserTarget;
@@ -62,6 +63,7 @@ class ShortcutSelectionLogic {
             List<ChooserTarget> targets,
             boolean isShortcutResult,
             Map<ChooserTarget, ShortcutInfo> directShareToShortcutInfos,
+            Map<ChooserTarget, AppTarget> directShareToAppTargets,
             Context userContext,
             SelectableTargetInfoCommunicator mSelectableTargetInfoCommunicator,
             int maxRankedTargets,
@@ -105,7 +107,8 @@ class ShortcutSelectionLogic {
                             target,
                             targetScore,
                             mSelectableTargetInfoCommunicator,
-                            shortcutInfo),
+                            shortcutInfo,
+                            directShareToAppTargets.get(target)),
                     maxRankedTargets,
                     serviceTargets);
 
diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
index 506faa9d..3a3c3e64 100644
--- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java
@@ -18,6 +18,7 @@ package com.android.intentresolver.chooser;
 
 import android.annotation.Nullable;
 import android.app.Activity;
+import android.app.prediction.AppTarget;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -61,17 +62,23 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
     private final String mDisplayLabel;
     private final PackageManager mPm;
     private final SelectableTargetInfoCommunicator mSelectableTargetInfoCommunicator;
-    @GuardedBy("this")
-    private ShortcutInfo mShortcutInfo;
-    private Drawable mBadgeIcon = null;
-    private CharSequence mBadgeContentDescription;
-    @GuardedBy("this")
-    private Drawable mDisplayIcon;
+    @Nullable
+    private final AppTarget mAppTarget;
+    @Nullable
+    private final ShortcutInfo mShortcutInfo;
     private final Intent mFillInIntent;
     private final int mFillInFlags;
     private final boolean mIsPinned;
     private final float mModifiedScore;
-    private boolean mIsSuspended = false;
+    private final boolean mIsSuspended;
+    private final Drawable mBadgeIcon;
+    private final CharSequence mBadgeContentDescription;
+
+    @GuardedBy("this")
+    private Drawable mDisplayIcon;
+
+    @GuardedBy("this")
+    private boolean mHasAttemptedIconLoad;
 
     /** Create a new {@link TargetInfo} instance representing a selectable target. */
     public static TargetInfo newSelectableTargetInfo(
@@ -80,14 +87,16 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
             ChooserTarget chooserTarget,
             float modifiedScore,
             SelectableTargetInfoCommunicator selectableTargetInfoCommunicator,
-            @Nullable ShortcutInfo shortcutInfo) {
+            @Nullable ShortcutInfo shortcutInfo,
+            @Nullable AppTarget appTarget) {
         return new SelectableTargetInfo(
                 context,
                 sourceInfo,
                 chooserTarget,
                 modifiedScore,
                 selectableTargetInfoCommunicator,
-                shortcutInfo);
+                shortcutInfo,
+                appTarget);
     }
 
     private SelectableTargetInfo(
@@ -96,7 +105,8 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
             ChooserTarget chooserTarget,
             float modifiedScore,
             SelectableTargetInfoCommunicator selectableTargetInfoComunicator,
-            @Nullable ShortcutInfo shortcutInfo) {
+            @Nullable ShortcutInfo shortcutInfo,
+            @Nullable AppTarget appTarget) {
         mContext = context;
         mSourceInfo = sourceInfo;
         mChooserTarget = chooserTarget;
@@ -104,20 +114,17 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
         mPm = mContext.getPackageManager();
         mSelectableTargetInfoCommunicator = selectableTargetInfoComunicator;
         mShortcutInfo = shortcutInfo;
+        mAppTarget = appTarget;
         mIsPinned = shortcutInfo != null && shortcutInfo.isPinned();
-        if (sourceInfo != null) {
-            final ResolveInfo ri = sourceInfo.getResolveInfo();
-            if (ri != null) {
-                final ActivityInfo ai = ri.activityInfo;
-                if (ai != null && ai.applicationInfo != null) {
-                    final PackageManager pm = mContext.getPackageManager();
-                    mBadgeIcon = pm.getApplicationIcon(ai.applicationInfo);
-                    mBadgeContentDescription = pm.getApplicationLabel(ai.applicationInfo);
-                    mIsSuspended =
-                            (ai.applicationInfo.flags & ApplicationInfo.FLAG_SUSPENDED) != 0;
-                }
-            }
-        }
+
+        final PackageManager pm = mContext.getPackageManager();
+        final ApplicationInfo applicationInfo = getApplicationInfoFromSource(sourceInfo);
+
+        mBadgeIcon = (applicationInfo == null) ? null : pm.getApplicationIcon(applicationInfo);
+        mBadgeContentDescription =
+                (applicationInfo == null) ? null : pm.getApplicationLabel(applicationInfo);
+        mIsSuspended = (applicationInfo != null)
+                && ((applicationInfo.flags & ApplicationInfo.FLAG_SUSPENDED) != 0);
 
         if (sourceInfo != null) {
             mBackupResolveInfo = null;
@@ -141,9 +148,12 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
         mChooserTarget = other.mChooserTarget;
         mBadgeIcon = other.mBadgeIcon;
         mBadgeContentDescription = other.mBadgeContentDescription;
+        mShortcutInfo = other.mShortcutInfo;
+        mAppTarget = other.mAppTarget;
+        mIsSuspended = other.mIsSuspended;
         synchronized (other) {
-            mShortcutInfo = other.mShortcutInfo;
             mDisplayIcon = other.mDisplayIcon;
+            mHasAttemptedIconLoad = other.mHasAttemptedIconLoad;
         }
         mFillInIntent = fillInIntent;
         mFillInFlags = flags;
@@ -158,12 +168,6 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
         return true;
     }
 
-    private String sanitizeDisplayLabel(CharSequence label) {
-        SpannableStringBuilder sb = new SpannableStringBuilder(label);
-        sb.clearSpans();
-        return sb.toString();
-    }
-
     @Override
     public boolean isSuspended() {
         return mIsSuspended;
@@ -180,24 +184,35 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
      */
     @Override
     public boolean loadIcon() {
-        ShortcutInfo shortcutInfo;
-        Drawable icon;
         synchronized (this) {
-            shortcutInfo = mShortcutInfo;
-            icon = mDisplayIcon;
-        }
-        boolean shouldLoadIcon = (icon == null) && (shortcutInfo != null);
-        if (shouldLoadIcon) {
-            icon = getChooserTargetIconDrawable(mChooserTarget, shortcutInfo);
-            if (icon == null) {
+            // TODO: evaluating these conditions while `synchronized` ensures that we get consistent
+            // reads between `mDisplayIcon` and `mHasAttemptedIconLoad`, but doesn't otherwise
+            // prevent races where two threads might check the conditions (in synchrony) and then
+            // both go on to load the icon (in parallel, even though one of the loads would be
+            // redundant, and even though we have no logic to decide which result to keep if they
+            // differ). This is probably a "safe optimization" in some cases, but our correctness
+            // can't rely on this eliding the duplicate load, and with a more careful design we
+            // could probably optimize it out in more cases (or else maybe we should get rid of
+            // this complexity altogether).
+            if ((mDisplayIcon != null) || (mShortcutInfo == null) || mHasAttemptedIconLoad) {
                 return false;
             }
-            synchronized (this) {
-                mDisplayIcon = icon;
-                mShortcutInfo = null;
-            }
         }
-        return shouldLoadIcon;
+
+        Drawable icon = getChooserTargetIconDrawable(mChooserTarget, mShortcutInfo);
+        if (icon == null) {
+            return false;
+        }
+
+        synchronized (this) {
+            mDisplayIcon = icon;
+            // TODO: we only end up setting `mHasAttemptedIconLoad` if we were successful in loading
+            // a (non-null) display icon; in that case, our guard clause above will already
+            // early-return `false` regardless of `mHasAttemptedIconLoad`. This should be refined,
+            // or removed if we don't need the extra complexity (including the synchronizaiton?).
+            mHasAttemptedIconLoad = true;
+        }
+        return true;
     }
 
     private Drawable getChooserTargetIconDrawable(ChooserTarget target,
@@ -349,6 +364,18 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
     }
 
     @Override
+    @Nullable
+    public ShortcutInfo getDirectShareShortcutInfo() {
+        return mShortcutInfo;
+    }
+
+    @Override
+    @Nullable
+    public AppTarget getDirectShareAppTarget() {
+        return mAppTarget;
+    }
+
+    @Override
     public TargetInfo cloneFilledIn(Intent fillInIntent, int flags) {
         return new SelectableTargetInfo(this, fillInIntent, flags);
     }
@@ -368,6 +395,32 @@ public final class SelectableTargetInfo extends ChooserTargetInfo {
         return mIsPinned;
     }
 
+    @Nullable
+    private static ApplicationInfo getApplicationInfoFromSource(
+            @Nullable DisplayResolveInfo sourceInfo) {
+        if (sourceInfo == null) {
+            return null;
+        }
+
+        final ResolveInfo resolveInfo = sourceInfo.getResolveInfo();
+        if (resolveInfo == null) {
+            return null;
+        }
+
+        final ActivityInfo activityInfo = resolveInfo.activityInfo;
+        if (activityInfo == null) {
+            return null;
+        }
+
+        return activityInfo.applicationInfo;
+    }
+
+    private static String sanitizeDisplayLabel(CharSequence label) {
+        SpannableStringBuilder sb = new SpannableStringBuilder(label);
+        sb.clearSpans();
+        return sb.toString();
+    }
+
     /**
      * Necessary methods to communicate between {@link SelectableTargetInfo}
      * and {@link ResolverActivity} or {@link ChooserActivity}.
diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java
index bde2fcf0..29c4cfc3 100644
--- a/java/src/com/android/intentresolver/chooser/TargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java
@@ -19,10 +19,12 @@ package com.android.intentresolver.chooser;
 
 import android.annotation.Nullable;
 import android.app.Activity;
+import android.app.prediction.AppTarget;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.ResolveInfo;
+import android.content.pm.ShortcutInfo;
 import android.graphics.drawable.Drawable;
 import android.os.Bundle;
 import android.os.UserHandle;
@@ -202,6 +204,23 @@ public interface TargetInfo {
     }
 
     /**
+     * @return the {@link ShortcutManager} data for any shortcut associated with this target.
+     */
+    @Nullable
+    default ShortcutInfo getDirectShareShortcutInfo() {
+        return null;
+    }
+
+    /**
+     * @return the {@link AppTarget} metadata if this target was sourced from App Prediction
+     * service, or null otherwise.
+     */
+    @Nullable
+    default AppTarget getDirectShareAppTarget() {
+        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.