Inline & remove extraneous helper method.

This is trivially a "pure refactoring" (after a fair amount of
reasoning, to follow in this CL description). We have ongoing work
both to unify the TargetInfo APIs (go/chooser-targetinfo-cleanup) and
to reduce our internal reliance on the deprecated `ChooserTarget` API.
This CL simplifies one usage site in advance of further cleanup work,
while removing one of our `ChooserTarget`-based internal APIs. The old
design was also unnecessarily flexible, requiring us to reason through
runtime behavior to prove(*) the equivalence of this CL; that
reasoning is much more apparent the way it's written now.

 * [Proof of equivalence]

By inspection in `ChooserListAdapter`, the `ChooserTarget` was
originally accessed by

```
public ChooserTarget getChooserTargetForValue(int value) {
        return mServiceTargets.get(value).getChooserTarget();
}
```

We assert that the call to `mServiceTargets.get(value)` will retrieve
the same `ChooserTargetInfo` instance as already returned by
`ChooserListAdapter.targetInfoForPosition(which, filtered)` (noting
that `value` and `which` are aliases in all the relevant legacy code).

The logic in `targetInfoForPosition()` returns the same
`mServiceTargets.get(position)` (with `position` another alias of
`value`/`which`) for any non-negative `position`, up to some
`serviceTargetCount` which depends on the value of `filtered` (a
parameter that seems a little confusing & not well-documented, but
we'll leave that detail out-of-scope for now). This is somewhat
concerning because our `filtered` value comes from the argument to
`ChooserActivity.startSelected()`, and it's passed on to
`ChooserListAdapter.targetInfoForPosition()` but *not* to the helper
`getChooserTargetForValue()` -- suggesting a possible bug if
`targetInfoForPosition()` ever in fact returns a different value
depending on the choice of `filtered` parameter, since then the two
getters would return info about different targets, and our log event
would get misattributed. (The ability to return a `ChooserTarget` that
isn't composed-into the `TargetInfo` at that position is the
aforementioned "extra flexibility" that can do us no good.)

The proposed change only applies in the case when
`ChooserListAdapter.getPositionTargetType(which)` returns
`TARGET_SERVICE`, implying `which < getServiceTargetCount()`; this is
the same limit selected for `targetInfoForPosition()` when `filtered`
is true, so in that event the behavior will be the same.

If `filtered` is false, `targetInfoForPosition()` instead compares to
a limit given by `getSelectableServiceTargetCount()`, the number of
elements in `mServiceTargets` that are instances of
`SelectableTargetInfo`. (We can already see this *should* be safe,
since the pre-existing code in `ChooserActivity.startSelected()`
already included an unchecked downcast to `SelectableTargetInfo`, and
we would've observed a "noisier" crash if there was a problem with
that assumption; note no other branch of `targetInfoForPosition()`
returns objects of any subtype of `SelectableTargetInfo`, so these
instances must have been retrieved directly from `mServiceTargets`.
It may also be reasonable to assume that a target we examine in the
`startSelected()` method is in fact "Selectable", which in fact it
already checks -- *as long as the target is a subclass of
`ChooserTargetInfo`*, and thus partitioned into being either a
`Selectable-` or `NotSelectableTargetInfo`).

For the hypothetical bug to occur, we would need to be requesting the
`TargetInfo` for a `position` greater than the value returned from
`getSelectableServiceTargetCount()` (so that `targetInfoForPosition()`
goes on to take the target from somewhere other than the corresponding
position in the `mServiceTargets` list), but less than the value
returned from `getServiceTargetCount()` (so that the target is still
classified as a `TARGET_SERVICE`). Thus we're concerned with cases
when the "selectable" target count might be lower than the total
target count (even after accounting for any other conditions in
`getServiceTargetCount()`). Inspecting all the insertions to
`ChooserListAdapter.mServiceTargets` we note that each site takes care
to honor the limit set by
`mChooserListCommunicator.getMaxRankedTargets()`; that is,
`mServiceTargets.size()` should never exceed this limit, and the
`Math.min` expression in `getServiceTargetCount()` should always be a
no-op. Thus the result of `getSelectableServiceTargetCount()` can only
be lower *because* it filtered out exactly that many service targets
that didn't inherit from `SelectableTargetInfo`. OTOH we can see from
`ChooserListAdapter.getSurfacedTargetInfo()` that the "selectable"
targets are represented as a *prefix* of the `mServiceTargets` list
(i.e., any filtered-out "non-selectable" items can only occur after
all others), and thus any problematic `position` we might query beyond
the limit of `getSelectableServiceTargetCount()` would have to be a
non-selectable item (by inspection, one of `ChooserActivity`'s two
inner-class implementations of `NotSelectableTargetInfo`). But any
item of these non-selectable types would be discarded by the guard
clause at the top of `ChooserActivity.startSelected()`, so any item
that we went on to process for logging must be from one of the
earlier indices directly into `mServiceTargets`, QED.

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

2 files changed, 2 insertions, 6 deletions

diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index 1e745d06..9d4515fa 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -1830,14 +1830,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.
-                    ChooserTarget target = currentListAdapter.getChooserTargetForValue(value);
+                    ChooserTargetInfo selectableTargetInfo = (ChooserTargetInfo) targetInfo;
+                    ChooserTarget target = selectableTargetInfo.getChooserTarget();
                     directTargetHashed = HashedStringCache.getInstance().hashString(
                             this,
                             TAG,
                             target.getComponentName().getPackageName()
                                     + target.getTitle().toString(),
                             mMaxHashSaltDays);
-                    ChooserTargetInfo selectableTargetInfo = (ChooserTargetInfo) targetInfo;
                     directTargetAlsoRanked = getRankedPosition(selectableTargetInfo);
 
                     if (mCallerChooserTargets != null) {
diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java
index 72382663..98e3dcdb 100644
--- a/java/src/com/android/intentresolver/ChooserListAdapter.java
+++ b/java/src/com/android/intentresolver/ChooserListAdapter.java
@@ -605,10 +605,6 @@ public class ChooserListAdapter extends ResolverListAdapter {
         notifyDataSetChanged();
     }
 
-    public ChooserTarget getChooserTargetForValue(int value) {
-        return mServiceTargets.get(value).getChooserTarget();
-    }
-
     protected boolean alwaysShowSubLabel() {
         // Always show a subLabel for visual consistency across list items. Show an empty
         // subLabel if the subLabel is the same as the label