Merge "Rename & pull `getTargets` up to base `TargetInfo`" into tm-qpr-dev

7 files changed, 62 insertions, 25 deletions

diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index 31254de8..a73424bf 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -1712,18 +1712,24 @@ public class ChooserActivity extends ResolverActivity implements
     private void showTargetDetails(TargetInfo targetInfo) {
         if (targetInfo == null) return;
 
-        ArrayList<DisplayResolveInfo> targetList;
+        List<DisplayResolveInfo> targetList = targetInfo.getAllDisplayTargets();
+        if (targetList.isEmpty()) {
+            Log.e(TAG, "No displayable data to show target details");
+            return;
+        }
+
         ChooserTargetActionsDialogFragment fragment = new ChooserTargetActionsDialogFragment();
         Bundle bundle = new Bundle();
 
+        bundle.putParcelable(ChooserTargetActionsDialogFragment.USER_HANDLE_KEY,
+                mChooserMultiProfilePagerAdapter.getCurrentUserHandle());
+        bundle.putParcelableArrayList(ChooserTargetActionsDialogFragment.TARGET_INFOS_KEY,
+                new ArrayList<>(targetList));
+
         if (targetInfo.isSelectableTargetInfo()) {
-            if (targetInfo.getDisplayResolveInfo() == null
-                    || targetInfo.getChooserTarget() == null) {
-                Log.e(TAG, "displayResolveInfo or chooserTarget in selectableTargetInfo are null");
-                return;
-            }
-            targetList = new ArrayList<>();
-            targetList.add(targetInfo.getDisplayResolveInfo());
+            // TODO: migrate this condition to polymorphic calls on TargetInfo (maybe in some cases
+            // we can safely drop the `isSelectableTargetInfo()` condition and populate the bundle
+            // with any non-null values we find, regardless of the target type?)
             bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_ID_KEY,
                     targetInfo.getChooserTarget().getIntentExtras().getString(
                             Intent.EXTRA_SHORTCUT_ID));
@@ -1735,20 +1741,9 @@ public class ChooserActivity extends ResolverActivity implements
                 bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_TITLE_KEY,
                         targetInfo.getDisplayLabel().toString());
             }
-        } else if (targetInfo.isMultiDisplayResolveInfo()) {
-            // For multiple targets, include info on all targets
-            MultiDisplayResolveInfo mti = (MultiDisplayResolveInfo) targetInfo;
-            targetList = mti.getTargets();
-        } else {
-            targetList = new ArrayList<DisplayResolveInfo>();
-            targetList.add((DisplayResolveInfo) targetInfo);
         }
-        bundle.putParcelable(ChooserTargetActionsDialogFragment.USER_HANDLE_KEY,
-                mChooserMultiProfilePagerAdapter.getCurrentUserHandle());
-        bundle.putParcelableArrayList(ChooserTargetActionsDialogFragment.TARGET_INFOS_KEY,
-                targetList);
-        fragment.setArguments(bundle);
 
+        fragment.setArguments(bundle);
         fragment.show(getSupportFragmentManager(), TARGET_DETAILS_FRAGMENT_TAG);
     }
 
diff --git a/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java b/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
index ae08ace2..b4e427a1 100644
--- a/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
+++ b/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
@@ -43,7 +43,7 @@ public class ChooserStackedAppDialogFragment extends ChooserTargetActionsDialogF
 
     void setStateFromBundle(Bundle b) {
         mMultiDisplayResolveInfo = (MultiDisplayResolveInfo) b.get(MULTI_DRI_KEY);
-        mTargetInfos = mMultiDisplayResolveInfo.getTargets();
+        mTargetInfos = mMultiDisplayResolveInfo.getAllDisplayTargets();
         mUserHandle = (UserHandle) b.get(USER_HANDLE_KEY);
         mParentWhich = b.getInt(WHICH_KEY);
     }
diff --git a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
index 38a2759e..2de901cd 100644
--- a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
@@ -19,6 +19,9 @@ package com.android.intentresolver.chooser;
 import android.service.chooser.ChooserTarget;
 import android.text.TextUtils;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+
 /**
  * A TargetInfo for Direct Share. Includes a {@link ChooserTarget} representing the
  * Direct Share deep link into an application.
@@ -31,6 +34,16 @@ public abstract class ChooserTargetInfo implements TargetInfo {
     }
 
     @Override
+    public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
+        // TODO: consider making this the default behavior for all `TargetInfo` implementations
+        // (if it's reasonable for `DisplayResolveInfo.getDisplayResolveInfo()` to return `this`).
+        if (getDisplayResolveInfo() == null) {
+            return new ArrayList<>();
+        }
+        return new ArrayList<>(Arrays.asList(getDisplayResolveInfo()));
+    }
+
+    @Override
     public boolean isSimilar(TargetInfo other) {
         if (other == null) return false;
 
diff --git a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
index 5e1b8cab..cd6828c7 100644
--- a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
+++ b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
@@ -35,6 +35,7 @@ import com.android.intentresolver.ResolverActivity;
 import com.android.intentresolver.ResolverListAdapter.ResolveInfoPresentationGetter;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -173,6 +174,11 @@ public class DisplayResolveInfo implements TargetInfo, Parcelable {
         return mSourceIntents;
     }
 
+    @Override
+    public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
+        return new ArrayList<>(Arrays.asList(this));
+    }
+
     public void addAlternateSourceIntent(Intent alt) {
         mSourceIntents.add(alt);
     }
diff --git a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
index bad36077..29f00a35 100644
--- a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
+++ b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
@@ -30,8 +30,6 @@ import java.util.List;
  */
 public class MultiDisplayResolveInfo extends DisplayResolveInfo {
 
-    /* TODO: hold as a generic `List<DisplayResolveInfo>` once we're unconstrained by the TODO
-     * regarding the return type of `#getTargets()`. */
     ArrayList<DisplayResolveInfo> mTargetInfos = new ArrayList<>();
 
     // Index of selected target
@@ -71,7 +69,8 @@ public class MultiDisplayResolveInfo extends DisplayResolveInfo {
      * TODO: provide as a generic {@code List<DisplayResolveInfo>} once {@link ChooserActivity}
      * stops requiring the signature to match that of the other "lists" it builds up.
      */
-    public ArrayList<DisplayResolveInfo> getTargets() {
+    @Override
+    public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
         return mTargetInfos;
     }
 
diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java
index 4842cd80..bde2fcf0 100644
--- a/java/src/com/android/intentresolver/chooser/TargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java
@@ -30,6 +30,7 @@ import android.service.chooser.ChooserTarget;
 
 import com.android.intentresolver.ResolverActivity;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
@@ -129,6 +130,28 @@ public interface TargetInfo {
     List<Intent> getAllSourceIntents();
 
     /**
+     * @return the one or more {@link DisplayResolveInfo}s that this target represents in the UI.
+     *
+     * TODO: clarify the semantics of the {@link DisplayResolveInfo} branch of {@link TargetInfo}'s
+     * class hierarchy. Why is it that {@link MultiDisplayResolveInfo} can stand in for some
+     * "virtual" {@link DisplayResolveInfo} targets that aren't individually represented in the UI,
+     * but OTOH a {@link ChooserTargetInfo} (which doesn't inherit from {@link DisplayResolveInfo})
+     * can't provide its own UI treatment, and instead needs us to reach into its composed-in
+     * info via {@link #getDisplayResolveInfo()}? It seems like {@link DisplayResolveInfo} may be
+     * required to populate views in our UI, while {@link ChooserTargetInfo} may carry some other
+     * metadata. For non-{@link ChooserTargetInfo} targets (e.g. in {@link ResolverActivity}) the
+     * "naked" {@link DisplayResolveInfo} might also be taken to provide some of this metadata, but
+     * this presents a denormalization hazard since the "UI info" ({@link DisplayResolveInfo}) that
+     * represents a {@link ChooserTargetInfo} might provide different values than its enclosing
+     * {@link ChooserTargetInfo} (as they both implement {@link TargetInfo}). We could try to
+     * address this by splitting {@link DisplayResolveInfo} into two types; one (which implements
+     * the same {@link TargetInfo} interface as {@link ChooserTargetInfo}) provides the previously-
+     * implicit "metadata", and the other provides only the UI treatment for a target of any type
+     * (taking over the respective methods that previously belonged to {@link TargetInfo}).
+     */
+    ArrayList<DisplayResolveInfo> getAllDisplayTargets();
+
+    /**
      * @return true if this target cannot be selected by the user
      */
     boolean isSuspended();
diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
index 8494e296..4a5e1baf 100644
--- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
+++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
@@ -122,7 +122,8 @@ class TargetInfoTest {
 
         assertThat(multiTargetInfo.getExtendedInfo()).isNull()
 
-        assertThat(multiTargetInfo.getTargets()).containsExactly(firstTargetInfo, secondTargetInfo)
+        assertThat(multiTargetInfo.getAllDisplayTargets())
+                .containsExactly(firstTargetInfo, secondTargetInfo)
 
         assertThat(multiTargetInfo.hasSelected()).isFalse()
         assertThat(multiTargetInfo.getSelectedTarget()).isNull()