From 248b655453b87149990b9e9138139bcf54291a90 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 22 Feb 2023 18:23:44 +0000 Subject: 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 --- .../android/intentresolver/ChooserActivity.java | 15 +++++++++----- .../android/intentresolver/ResolverActivity.java | 19 +++++++++++++----- .../intentresolver/ResolverListAdapter.java | 12 +++++------ .../intentresolver/ResolverListController.java | 23 ++-------------------- 4 files changed, 31 insertions(+), 38 deletions(-) (limited to 'java/src') 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 getResolversForUser(UserHandle userHandle) { - return mResolverListController.getResolversForIntentAsUser(true, + return mResolverListController.getResolversForIntentAsUser( + /* shouldGetResolvedFilter= */ true, mResolverListCommunicator.shouldGetActivityMetadata(), mResolverListCommunicator.shouldGetOnlyDefaultActivities(), - mIntents, userHandle); + mIntents, + userHandle); } protected List 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 getResolversForIntent( - boolean shouldGetResolvedFilter, - boolean shouldGetActivityMetadata, - boolean shouldGetOnlyDefaultActivities, - List intents) { - return getResolversForIntentAsUser(shouldGetResolvedFilter, shouldGetActivityMetadata, - shouldGetOnlyDefaultActivities, intents, mUserHandle); - } - public List getResolversForIntentAsUser( boolean shouldGetResolvedFilter, boolean shouldGetActivityMetadata, @@ -159,11 +145,6 @@ public class ResolverListController { return resolvedComponents; } - @VisibleForTesting - public UserHandle getUserHandle() { - return mUserHandle; - } - @VisibleForTesting public void addResolveListDedupe(List into, Intent intent, -- cgit v1.2.3-59-g8ed1b