diff options
| author | 2023-02-22 18:23:44 +0000 | |
|---|---|---|
| committer | 2023-02-22 19:36:27 +0000 | |
| commit | 248b655453b87149990b9e9138139bcf54291a90 (patch) | |
| tree | d2ac471b4a43bdd2f0947171a37a6d539b0d2279 /java/src | |
| parent | 25c119f04c375740903779693520a45b506d35fa (diff) | |
Remove UserHandle ivar from ResolverListController.
ResolverListController instances are 1:1 with the ResolverListAdapter
instances that hold them, and the adapters already carry their
associated UserHandles, so this was just one more "source of truth"
to potentially get out of sync as we're trying to clean up management
of these handles throughout the project. (And by exposing the handle
in a now-removed getter, it also added a redundant mechanism for
clients to access this value, making it harder to track down all the
usages in our application.)
I also took this opportunity to factor out a lot of boilerplate in
our tests to use a helper. That's not generally considered a good
practice for testing, but in this case the helper already existed
and just wasn't used in some cases -- where we instead duplicated
verbose boilerplate that represents less about the conditions we'd
actually expect to see in practice. Long-term we should clean up a
*lot* of details in our test design, so I think for now it's OK to
just use a more terse version of the design we already have.
Eventually I'd like to break up ResolverListController into a few
components with narrower focus, so removing any unnecessary
responsibilities is a great start.
Test: `atest IntentResolverUnitTests`
Bug: 202167050
Change-Id: Iade5b8d1f4e31d5439c234fbb1d82169ec01a386
Diffstat (limited to 'java/src')
4 files changed, 31 insertions, 38 deletions
diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 72336e84..da3694c4 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -1165,14 +1165,19 @@ public class ChooserActivity extends ResolverActivity implements      }      public class ChooserListController extends ResolverListController { -        public ChooserListController(Context context, +        public ChooserListController( +                Context context,                  PackageManager pm,                  Intent targetIntent,                  String referrerPackageName,                  int launchedFromUid, -                UserHandle userId,                  AbstractResolverComparator resolverComparator) { -            super(context, pm, targetIntent, referrerPackageName, launchedFromUid, userId, +            super( +                    context, +                    pm, +                    targetIntent, +                    referrerPackageName, +                    launchedFromUid,                      resolverComparator);          } @@ -1308,8 +1313,9 @@ public class ChooserActivity extends ResolverActivity implements                  maxTargetsPerRow);      } +    @Override      @VisibleForTesting -    protected ResolverListController createListController(UserHandle userHandle) { +    protected ChooserListController createListController(UserHandle userHandle) {          AppPredictor appPredictor = getAppPredictor(userHandle);          AbstractResolverComparator resolverComparator;          if (appPredictor != null) { @@ -1327,7 +1333,6 @@ public class ChooserActivity extends ResolverActivity implements                  getTargetIntent(),                  getReferrerPackageName(),                  getAnnotatedUserHandles().userIdOfCallingApp, -                userHandle,                  resolverComparator);      } diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index faccbc36..08f42404 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -849,15 +849,25 @@ public class ResolverActivity extends FragmentActivity implements          return !target.isSuspended();      } +    // TODO: this method takes an unused `UserHandle` because the override in `ChooserActivity` uses +    // that data to set up other components as dependencies of the controller. In reality, these +    // methods don't require polymorphism, because they're only invoked from within their respective +    // concrete class; `ResolverActivity` will never call this method expecting to get a +    // `ChooserListController` (subclass) result, because `ResolverActivity` only invokes this +    // method as part of handling `createMultiProfilePagerAdapter()`, which is itself overridden in +    // `ChooserActivity`. A future refactoring could better express the coupling between the adapter +    // and controller types; in the meantime, structuring as an override (with matching signatures) +    // shows that these methods are *structurally* related, and helps to prevent any regressions in +    // the future if resolver *were* to make any (non-overridden) calls to a version that used a +    // different signature (and thus didn't return the subclass type).      @VisibleForTesting -    protected ResolverListController createListController(UserHandle userHandle) { +    protected ResolverListController createListController(UserHandle unused) {          return new ResolverListController(                  this,                  mPm,                  getTargetIntent(),                  getReferrerPackageName(), -                getAnnotatedUserHandles().userIdOfCallingApp, -                userHandle); +                getAnnotatedUserHandles().userIdOfCallingApp);      }      /** @@ -1719,8 +1729,7 @@ public class ResolverActivity extends FragmentActivity implements          findViewById(com.android.internal.R.id.button_open).setOnClickListener(v -> {              Intent intent = otherProfileResolveInfo.getResolvedIntent(); -            safelyStartActivityAsUser(otherProfileResolveInfo, -                    inactiveAdapter.mResolverListController.getUserHandle()); +            safelyStartActivityAsUser(otherProfileResolveInfo, inactiveAdapter.getUserHandle());              finish();          });      } diff --git a/java/src/com/android/intentresolver/ResolverListAdapter.java b/java/src/com/android/intentresolver/ResolverListAdapter.java index 7a258a4c..c8a9d5dc 100644 --- a/java/src/com/android/intentresolver/ResolverListAdapter.java +++ b/java/src/com/android/intentresolver/ResolverListAdapter.java @@ -289,11 +289,7 @@ public class ResolverListAdapter extends BaseAdapter {                      mBaseResolveList);              return currentResolveList;          } else { -            return mResolverListController.getResolversForIntent( -                            /* shouldGetResolvedFilter= */ true, -                            mResolverListCommunicator.shouldGetActivityMetadata(), -                            mResolverListCommunicator.shouldGetOnlyDefaultActivities(), -                            mIntents); +            return getResolversForUser(mUserHandle);          }      } @@ -804,10 +800,12 @@ public class ResolverListAdapter extends BaseAdapter {      }      protected List<ResolvedComponentInfo> getResolversForUser(UserHandle userHandle) { -        return mResolverListController.getResolversForIntentAsUser(true, +        return mResolverListController.getResolversForIntentAsUser( +                /* shouldGetResolvedFilter= */ true,                  mResolverListCommunicator.shouldGetActivityMetadata(),                  mResolverListCommunicator.shouldGetOnlyDefaultActivities(), -                mIntents, userHandle); +                mIntents, +                userHandle);      }      protected List<Intent> getIntents() { diff --git a/java/src/com/android/intentresolver/ResolverListController.java b/java/src/com/android/intentresolver/ResolverListController.java index bfffe0d8..6eb027ea 100644 --- a/java/src/com/android/intentresolver/ResolverListController.java +++ b/java/src/com/android/intentresolver/ResolverListController.java @@ -58,7 +58,6 @@ public class ResolverListController {      private static final String TAG = "ResolverListController";      private static final boolean DEBUG = false; -    private final UserHandle mUserHandle;      private AbstractResolverComparator mResolverComparator;      private boolean isComputed = false; @@ -68,9 +67,8 @@ public class ResolverListController {              PackageManager pm,              Intent targetIntent,              String referrerPackage, -            int launchedFromUid, -            UserHandle userHandle) { -        this(context, pm, targetIntent, referrerPackage, launchedFromUid, userHandle, +            int launchedFromUid) { +        this(context, pm, targetIntent, referrerPackage, launchedFromUid,                      new ResolverRankerServiceResolverComparator(                          context, targetIntent, referrerPackage, null, null));      } @@ -81,14 +79,12 @@ public class ResolverListController {              Intent targetIntent,              String referrerPackage,              int launchedFromUid, -            UserHandle userHandle,              AbstractResolverComparator resolverComparator) {          mContext = context;          mpm = pm;          mLaunchedFromUid = launchedFromUid;          mTargetIntent = targetIntent;          mReferrerPackage = referrerPackage; -        mUserHandle = userHandle;          mResolverComparator = resolverComparator;      } @@ -108,16 +104,6 @@ public class ResolverListController {                  filter, match, intent.getComponent());      } -    @VisibleForTesting -    public List<ResolverActivity.ResolvedComponentInfo> getResolversForIntent( -            boolean shouldGetResolvedFilter, -            boolean shouldGetActivityMetadata, -            boolean shouldGetOnlyDefaultActivities, -            List<Intent> intents) { -        return getResolversForIntentAsUser(shouldGetResolvedFilter, shouldGetActivityMetadata, -                shouldGetOnlyDefaultActivities, intents, mUserHandle); -    } -      public List<ResolverActivity.ResolvedComponentInfo> getResolversForIntentAsUser(              boolean shouldGetResolvedFilter,              boolean shouldGetActivityMetadata, @@ -160,11 +146,6 @@ public class ResolverListController {      }      @VisibleForTesting -    public UserHandle getUserHandle() { -        return mUserHandle; -    } - -    @VisibleForTesting      public void addResolveListDedupe(List<ResolverActivity.ResolvedComponentInfo> into,              Intent intent,              List<ResolveInfo> from) {  |