Add a null check for a rare error case.

This just addresses the immediate NPE and doesn't explain the root
cause of the null value. I expect we *shouldn't* be seeing nulls, but
in the context of the other legacy code it seems to make sense to
take appropriate precautions against them anyways. This could've been
a regression from the legacy code, since an older version used an
`instanceof` check that would've correctly skipped over nulls. I've
chosen the narrowest fix to reproduce that logic, because it's
difficult to reason about other more-"elegant" potential improvements
(as briefly noted in b/285565219).

There's no obvious way to reproduce the null condition, so this isn't
tested to "fix"/"work in" the underlying error case overall (although
it should be clear from the code that this would prevent the specific
NPE we have at hand). Instead the test runs for this CL just validate
that the new null-check condition doesn't add regressions to our
currently-tested cases (where the result is non-null).

Bug: 285565219
Test: UnbundledChooserActivityTest, CtsSharesheetDeviceTest
Change-Id: I3b367206b4eefad579dbf613dfd38f485720cf65

1 files changed, 12 insertions, 2 deletions

diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index 63ac6435..eb38ad54 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -883,7 +883,7 @@ public class ChooserActivity extends ResolverActivity implements
 
         final long selectionCost = System.currentTimeMillis() - mChooserShownTime;
 
-        if (targetInfo.isMultiDisplayResolveInfo()) {
+        if ((targetInfo != null) && targetInfo.isMultiDisplayResolveInfo()) {
             MultiDisplayResolveInfo mti = (MultiDisplayResolveInfo) targetInfo;
             if (!mti.hasSelected()) {
                 // Add userHandle based badge to the stackedAppDialogBox.
@@ -900,7 +900,17 @@ public class ChooserActivity extends ResolverActivity implements
 
         super.startSelected(which, always, filtered);
 
-        if (currentListAdapter.getCount() > 0) {
+        // TODO: both of the conditions around this switch logic *should* be redundant, and
+        // can be removed if certain invariants can be guaranteed. In particular, it seems
+        // like targetInfo (from `ChooserListAdapter.targetInfoForPosition()`) is *probably*
+        // expected to be null only at out-of-bounds indexes where `getPositionTargetType()`
+        // returns TARGET_BAD; then the switch falls through to a default no-op, and we don't
+        // need to null-check targetInfo. We only need the null check if it's possible that
+        // the ChooserListAdapter contains null elements "in the middle" of its list data,
+        // such that they're classified as belonging to one of the real target types. That
+        // should probably never happen. But why would this method ever be invoked with a
+        // null target at all? Even an out-of-bounds index should never be "selected"...
+        if ((currentListAdapter.getCount() > 0) && (targetInfo != null)) {
             switch (currentListAdapter.getPositionTargetType(which)) {
                 case ChooserListAdapter.TARGET_SERVICE:
                     getChooserActivityLogger().logShareTargetSelected(