diff options
| author | 2022-10-20 09:18:06 -0400 | |
|---|---|---|
| committer | 2022-10-24 13:28:25 +0000 | |
| commit | deb9ddfb590c7533e1be565c40f9a522071c19a9 (patch) | |
| tree | e73c8b845c66d66212731e7d64c09abcd85f0e30 /java | |
| parent | 5551f9d425395ae427c98785425b81a32b1103dd (diff) | |
Rename & pull `getTargets` up to base `TargetInfo`
This was originally only offered on `MultiDisplayResolveInfo` but
that just resulted in some type-checked conditional logic where the
caller (`ChooserActivity`) took responsibility for implementing the
other cases. Thus this CL is mainly based on the "Replace Conditional
with Polymorphism" item in Fowler's _Refactoring_. (It also separates
one piece of unrelated conditional behavior from the call site in
`ChooserActivity` but leaves any further cleanup of that separate
behavior out of scope.) The method rename is an attempt to call
attention to the use of the legacy `DisplayResolveInfo` type (as
distinguished from other types of "targets" i.e. `ChooserTargetInfo`),
in advance of any cleanup of that design.
This both simplifies the call site (in `ChooserActivity`) and
consolidates `TargetInfo` requirements that were previously diffuse.
It also progresses several of our tactical goals in the ongoing
`TargetInfo` cleanup (go/chooser-targetinfo-cleanup):
* In order to unify the types, all methods offered by any
`TargetInfo` implementation have to be pulled up to the base
interface; this CL prioritizes one method of particular benefit to
callers in its new location, but it was going to have to move
eventually regardless.
* This encapsulates one of the two call sites of
`TargetInfo.getDisplayResolveInfo()`. That API assumes callers have
an understanding of the semantics of the legacy
`DisplayResolveInfo` implementation, along with the semantics of
the (legacy) concrete type of any other `TargetInfo` they might
hold (in order to know whether they can use it as-is vs. descending
through its `getDisplayResolveInfo()`). The new API focuses
(slightly) more on the application requirements that client is
trying to fulfill.
* More broadly, this reduces call sites where clients depend on the
(new-but-deprecated) `TargetInfo` methods that query the legacy
type information. These usages _all_ need to be replaced with new
APIs that are more expressive in the application domain.
Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: Ibc6cbfb01f323db4b820053babfeb5f7c8024f26
Diffstat (limited to 'java')
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 26c7fbc9..91f1ce74 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() |