From 3bd1033da86ba26d922e19a21bb21dec4bf7188f Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Mon, 28 Nov 2022 12:07:14 -0500 Subject: Tighten visibility in the -ListAdapter classes. As noted in ag/19933554, client code occasionally reached directly into these components to manipulate package-private members that weren't obviously intended to be part of the API. This CL narrows the visibility of those ivars (as described below), creating new methods as necessary to expose the (now-encapsulated) data for the clients that had previously been using it directly. This CL also shows that some indirect dependencies (previously accessed via these shared ivars) can be precomputed and injected, to avoid any subsequent sharing. Finally, the CL includes some other minor readability cleanups. To narrow the visibilities, almost all ivars were marked `private`. In some cases it's convenient to grant the subclass direct access to collaborating components, so those were marked `protected` in the base class; those are exclusively `final` members (and for the most part, their APIs are even "logically stateless") so they should be much easier to reason about when reading the base class source. These classes still need substantial refactoring to define their roles and responsibilities in the system, and especially to clarify their inheritance contract; this CL is just a first step to making the existing code more understandable. Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: I659197b3fe22f46813dc818e90ba78546a61bec3 --- .../AbstractMultiProfilePagerAdapter.java | 4 +- .../android/intentresolver/ChooserActivity.java | 15 +++-- .../android/intentresolver/ChooserListAdapter.java | 29 +++++---- .../android/intentresolver/ResolverActivity.java | 42 +++++++----- .../intentresolver/ResolverListAdapter.java | 76 +++++++++++++++------- .../intentresolver/ChooserListAdapterTest.kt | 2 + .../intentresolver/ChooserWrapperActivity.java | 4 ++ .../intentresolver/ResolverWrapperActivity.java | 12 +++- .../intentresolver/ResolverWrapperAdapter.java | 22 +++++-- 9 files changed, 141 insertions(+), 65 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/AbstractMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/AbstractMultiProfilePagerAdapter.java index 3e1084f4..8b0b10b0 100644 --- a/java/src/com/android/intentresolver/AbstractMultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/AbstractMultiProfilePagerAdapter.java @@ -148,7 +148,7 @@ public abstract class AbstractMultiProfilePagerAdapter extends PagerAdapter { @VisibleForTesting public UserHandle getCurrentUserHandle() { - return getActiveListAdapter().mResolverListController.getUserHandle(); + return getActiveListAdapter().getUserHandle(); } @Override @@ -263,7 +263,7 @@ public abstract class AbstractMultiProfilePagerAdapter extends PagerAdapter { } private int userHandleToPageIndex(UserHandle userHandle) { - if (userHandle.equals(getPersonalListAdapter().mResolverListController.getUserHandle())) { + if (userHandle.equals(getPersonalListAdapter().getUserHandle())) { return PROFILE_PERSONAL; } else { return PROFILE_WORK; diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index ad106bba..5535d987 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -1359,11 +1359,11 @@ public class ChooserActivity extends ResolverActivity implements targetInfo.getChooserTargetComponentName().getPackageName(); ChooserListAdapter currentListAdapter = mChooserMultiProfilePagerAdapter.getActiveListAdapter(); - int maxRankedResults = Math.min(currentListAdapter.mDisplayList.size(), - MAX_LOG_RANK_POSITION); + int maxRankedResults = Math.min( + currentListAdapter.getDisplayResolveInfoCount(), MAX_LOG_RANK_POSITION); for (int i = 0; i < maxRankedResults; i++) { - if (currentListAdapter.mDisplayList.get(i) + if (currentListAdapter.getDisplayResolveInfo(i) .getResolveInfo().activityInfo.packageName.equals(targetPackageName)) { return i; } @@ -1627,6 +1627,8 @@ public class ChooserActivity extends ResolverActivity implements rList, filterLastUsed, createListController(userHandle), + userHandle, + getTargetIntent(), mChooserRequest, mMaxTargetsPerRow); return new ChooserGridAdapter(chooserListAdapter); @@ -1640,6 +1642,8 @@ public class ChooserActivity extends ResolverActivity implements List rList, boolean filterLastUsed, ResolverListController resolverListController, + UserHandle userHandle, + Intent targetIntent, ChooserRequestParameters chooserRequest, int maxTargetsPerRow) { return new ChooserListAdapter( @@ -1649,6 +1653,8 @@ public class ChooserActivity extends ResolverActivity implements rList, filterLastUsed, resolverListController, + userHandle, + targetIntent, this, context.getPackageManager(), getChooserActivityLogger(), @@ -1898,8 +1904,7 @@ public class ChooserActivity extends ResolverActivity implements .setupListAdapter(mChooserMultiProfilePagerAdapter.getCurrentPage()); } - if (chooserListAdapter.mDisplayList == null - || chooserListAdapter.mDisplayList.isEmpty()) { + if (chooserListAdapter.getDisplayResolveInfoCount() == 0) { chooserListAdapter.notifyDataSetChanged(); } else { chooserListAdapter.updateAlphabeticalList(); diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index b18d2718..59d1a6e3 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -37,6 +37,7 @@ import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; import android.os.AsyncTask; import android.os.Trace; +import android.os.UserHandle; import android.os.UserManager; import android.provider.DeviceConfig; import android.service.chooser.ChooserTarget; @@ -67,8 +68,6 @@ public class ChooserListAdapter extends ResolverListAdapter { private static final String TAG = "ChooserListAdapter"; private static final boolean DEBUG = false; - private boolean mEnableStackedApps = true; - public static final int NO_POSITION = -1; public static final int TARGET_BAD = -1; public static final int TARGET_CALLER = 0; @@ -95,11 +94,11 @@ public class ChooserListAdapter extends ResolverListAdapter { private final List mServiceTargets = new ArrayList<>(); private final List mCallerTargets = new ArrayList<>(); + private final ShortcutSelectionLogic mShortcutSelectionLogic; + // Sorted list of DisplayResolveInfos for the alphabetical app section. private List mSortedList = new ArrayList<>(); - private final ShortcutSelectionLogic mShortcutSelectionLogic; - // For pinned direct share labels, if the text spans multiple lines, the TextView will consume // the full width, even if the characters actually take up less than that. Measure the actual // line widths and constrain the View's width based upon that so that the pin doesn't end up @@ -138,6 +137,8 @@ public class ChooserListAdapter extends ResolverListAdapter { List rList, boolean filterLastUsed, ResolverListController resolverListController, + UserHandle userHandle, + Intent targetIntent, ResolverListCommunicator resolverListCommunicator, PackageManager packageManager, ChooserActivityLogger chooserActivityLogger, @@ -145,8 +146,17 @@ public class ChooserListAdapter extends ResolverListAdapter { int maxRankedTargets) { // Don't send the initial intents through the shared ResolverActivity path, // we want to separate them into a different section. - super(context, payloadIntents, null, rList, filterLastUsed, - resolverListController, resolverListCommunicator, false); + super( + context, + payloadIntents, + null, + rList, + filterLastUsed, + resolverListController, + userHandle, + targetIntent, + resolverListCommunicator, + false); mChooserRequest = chooserRequest; mMaxRankedTargets = maxRankedTargets; @@ -334,11 +344,8 @@ public class ChooserListAdapter extends ResolverListAdapter { @Override protected List doInBackground(Void... voids) { List allTargets = new ArrayList<>(); - allTargets.addAll(mDisplayList); + allTargets.addAll(getTargetsInCurrentDisplayList()); allTargets.addAll(mCallerTargets); - if (!mEnableStackedApps) { - return allTargets; - } // Consolidate multiple targets from same app. return allTargets @@ -408,7 +415,7 @@ public class ChooserListAdapter extends ResolverListAdapter { int getAlphaTargetCount() { int groupedCount = mSortedList.size(); - int ungroupedCount = mCallerTargets.size() + mDisplayList.size(); + int ungroupedCount = mCallerTargets.size() + getDisplayResolveInfoCount(); return (ungroupedCount > mMaxRankedTargets) ? groupedCount : 0; } diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index fece8d3d..5a116b43 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -851,7 +851,6 @@ public class ResolverActivity extends FragmentActivity implements } } - @Override // SelectableTargetInfoCommunicator ResolverListCommunicator public Intent getTargetIntent() { return mIntents.isEmpty() ? null : mIntents.get(0); } @@ -1532,8 +1531,16 @@ public class ResolverActivity extends FragmentActivity implements Intent startIntent = getIntent(); boolean isAudioCaptureDevice = startIntent.getBooleanExtra(EXTRA_IS_AUDIO_CAPTURE_DEVICE, false); - return new ResolverListAdapter(context, payloadIntents, initialIntents, rList, - filterLastUsed, createListController(userHandle), this, + return new ResolverListAdapter( + context, + payloadIntents, + initialIntents, + rList, + filterLastUsed, + createListController(userHandle), + userHandle, + getTargetIntent(), + this, isAudioCaptureDevice); } @@ -1597,12 +1604,13 @@ public class ResolverActivity extends FragmentActivity implements setContentView(mLayoutId); DisplayResolveInfo sameProfileResolveInfo = - mMultiProfilePagerAdapter.getActiveListAdapter().mDisplayList.get(0); + mMultiProfilePagerAdapter.getActiveListAdapter().getFirstDisplayResolveInfo(); boolean inWorkProfile = getCurrentProfile() == PROFILE_WORK; final ResolverListAdapter inactiveAdapter = mMultiProfilePagerAdapter.getInactiveListAdapter(); - final DisplayResolveInfo otherProfileResolveInfo = inactiveAdapter.mDisplayList.get(0); + final DisplayResolveInfo otherProfileResolveInfo = + inactiveAdapter.getFirstDisplayResolveInfo(); // Load the icon asynchronously ImageView icon = findViewById(com.android.internal.R.id.icon); @@ -1653,31 +1661,29 @@ public class ResolverActivity extends FragmentActivity implements || mMultiProfilePagerAdapter.getInactiveListAdapter() == null) { return false; } - List sameProfileList = - mMultiProfilePagerAdapter.getActiveListAdapter().mDisplayList; - List otherProfileList = - mMultiProfilePagerAdapter.getInactiveListAdapter().mDisplayList; + ResolverListAdapter sameProfileAdapter = + mMultiProfilePagerAdapter.getActiveListAdapter(); + ResolverListAdapter otherProfileAdapter = + mMultiProfilePagerAdapter.getInactiveListAdapter(); - if (sameProfileList.isEmpty()) { + if (sameProfileAdapter.getDisplayResolveInfoCount() == 0) { Log.d(TAG, "No targets in the current profile"); return false; } - if (otherProfileList.size() != 1) { - Log.d(TAG, "Found " + otherProfileList.size() + " resolvers in the other profile"); + if (otherProfileAdapter.getDisplayResolveInfoCount() != 1) { + Log.d(TAG, "Other-profile count: " + otherProfileAdapter.getDisplayResolveInfoCount()); return false; } - if (otherProfileList.get(0).getResolveInfo().handleAllWebDataURI) { + if (otherProfileAdapter.allResolveInfosHandleAllWebDataUri()) { Log.d(TAG, "Other profile is a web browser"); return false; } - for (DisplayResolveInfo info : sameProfileList) { - if (!info.getResolveInfo().handleAllWebDataURI) { - Log.d(TAG, "Non-browser found in this profile"); - return false; - } + if (!sameProfileAdapter.allResolveInfosHandleAllWebDataUri()) { + Log.d(TAG, "Non-browser found in this profile"); + return false; } return true; diff --git a/java/src/com/android/intentresolver/ResolverListAdapter.java b/java/src/com/android/intentresolver/ResolverListAdapter.java index d97191c6..9f654594 100644 --- a/java/src/com/android/intentresolver/ResolverListAdapter.java +++ b/java/src/com/android/intentresolver/ResolverListAdapter.java @@ -56,6 +56,8 @@ import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.TargetInfo; import com.android.internal.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; + import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -65,37 +67,48 @@ import java.util.Map; public class ResolverListAdapter extends BaseAdapter { private static final String TAG = "ResolverListAdapter"; + @Nullable // TODO: other model for lazy computation? Or just precompute? + private static ColorMatrixColorFilter sSuspendedMatrixColorFilter; + + protected final Context mContext; + protected final LayoutInflater mInflater; + protected final ResolverListCommunicator mResolverListCommunicator; + protected final ResolverListController mResolverListController; + private final List mIntents; private final Intent[] mInitialIntents; private final List mBaseResolveList; private final PackageManager mPm; - protected final Context mContext; - private static ColorMatrixColorFilter sSuspendedMatrixColorFilter; private final int mIconDpi; - protected ResolveInfo mLastChosen; + private final boolean mIsAudioCaptureDevice; + private final UserHandle mUserHandle; + private final Intent mTargetIntent; + + private final Map mIconLoaders = new HashMap<>(); + private final Map mLabelLoaders = new HashMap<>(); + + private ResolveInfo mLastChosen; private DisplayResolveInfo mOtherProfile; - ResolverListController mResolverListController; private int mPlaceholderCount; - protected final LayoutInflater mInflater; - // This one is the list that the Adapter will actually present. - List mDisplayList; + private List mDisplayList; private List mUnfilteredResolveList; private int mLastChosenPosition = -1; private boolean mFilterLastUsed; - final ResolverListCommunicator mResolverListCommunicator; private Runnable mPostListReadyRunnable; - private final boolean mIsAudioCaptureDevice; private boolean mIsTabLoaded; - private final Map mIconLoaders = new HashMap<>(); - private final Map mLabelLoaders = new HashMap<>(); - public ResolverListAdapter(Context context, List payloadIntents, - Intent[] initialIntents, List rList, + public ResolverListAdapter( + Context context, + List payloadIntents, + Intent[] initialIntents, + List rList, boolean filterLastUsed, ResolverListController resolverListController, + UserHandle userHandle, + Intent targetIntent, ResolverListCommunicator resolverListCommunicator, boolean isAudioCaptureDevice) { mContext = context; @@ -107,12 +120,22 @@ public class ResolverListAdapter extends BaseAdapter { mDisplayList = new ArrayList<>(); mFilterLastUsed = filterLastUsed; mResolverListController = resolverListController; + mUserHandle = userHandle; + mTargetIntent = targetIntent; mResolverListCommunicator = resolverListCommunicator; mIsAudioCaptureDevice = isAudioCaptureDevice; final ActivityManager am = (ActivityManager) mContext.getSystemService(ACTIVITY_SERVICE); mIconDpi = am.getLauncherLargeIconDensity(); } + public final DisplayResolveInfo getFirstDisplayResolveInfo() { + return mDisplayList.get(0); + } + + public final ImmutableList getTargetsInCurrentDisplayList() { + return ImmutableList.copyOf(mDisplayList); + } + public void handlePackagesChanged() { mResolverListCommunicator.onHandlePackagesChanged(this); } @@ -262,7 +285,7 @@ public class ResolverListAdapter extends BaseAdapter { if (mBaseResolveList != null) { List currentResolveList = new ArrayList<>(); mResolverListController.addResolveListDedupe(currentResolveList, - mResolverListCommunicator.getTargetIntent(), + mTargetIntent, mBaseResolveList); return currentResolveList; } else { @@ -338,7 +361,12 @@ public class ResolverListAdapter extends BaseAdapter { if (otherProfileInfo != null) { mOtherProfile = makeOtherProfileDisplayResolveInfo( - mContext, otherProfileInfo, mPm, mResolverListCommunicator, mIconDpi); + mContext, + otherProfileInfo, + mPm, + mTargetIntent, + mResolverListCommunicator, + mIconDpi); } else { mOtherProfile = null; try { @@ -499,7 +527,7 @@ public class ResolverListAdapter extends BaseAdapter { final Intent replaceIntent = mResolverListCommunicator.getReplacementIntent(add.activityInfo, intent); final Intent defaultIntent = mResolverListCommunicator.getReplacementIntent( - add.activityInfo, mResolverListCommunicator.getTargetIntent()); + add.activityInfo, mTargetIntent); final DisplayResolveInfo dri = DisplayResolveInfo.newDisplayResolveInfo( intent, add, @@ -608,11 +636,15 @@ public class ResolverListAdapter extends BaseAdapter { return position; } - public int getDisplayResolveInfoCount() { + public final int getDisplayResolveInfoCount() { return mDisplayList.size(); } - public DisplayResolveInfo getDisplayResolveInfo(int index) { + public final boolean allResolveInfosHandleAllWebDataUri() { + return mDisplayList.stream().allMatch(t -> t.getResolveInfo().handleAllWebDataURI); + } + + public final DisplayResolveInfo getDisplayResolveInfo(int index) { // Used to query services. We only query services for primary targets, not alternates. return mDisplayList.get(index); } @@ -765,7 +797,7 @@ public class ResolverListAdapter extends BaseAdapter { } public UserHandle getUserHandle() { - return mResolverListController.getUserHandle(); + return mUserHandle; } protected List getResolversForUser(UserHandle userHandle) { @@ -821,6 +853,7 @@ public class ResolverListAdapter extends BaseAdapter { Context context, ResolvedComponentInfo resolvedComponentInfo, PackageManager pm, + Intent targetIntent, ResolverListCommunicator resolverListCommunicator, int iconDpi) { ResolveInfo resolveInfo = resolvedComponentInfo.getResolveInfoAt(0); @@ -829,8 +862,7 @@ public class ResolverListAdapter extends BaseAdapter { resolveInfo.activityInfo, resolvedComponentInfo.getIntentAt(0)); Intent replacementIntent = resolverListCommunicator.getReplacementIntent( - resolveInfo.activityInfo, - resolverListCommunicator.getTargetIntent()); + resolveInfo.activityInfo, targetIntent); ResolveInfoPresentationGetter presentationGetter = new ResolveInfoPresentationGetter(context, iconDpi, resolveInfo); @@ -871,8 +903,6 @@ public class ResolverListAdapter extends BaseAdapter { */ default boolean shouldGetOnlyDefaultActivities() { return true; }; - Intent getTargetIntent(); - void onHandlePackagesChanged(ResolverListAdapter listAdapter); } diff --git a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt index d054e7fa..6b34f8b9 100644 --- a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt +++ b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt @@ -56,6 +56,8 @@ class ChooserListAdapterTest { emptyList(), false, resolverListController, + null, + Intent(), mock(), packageManager, chooserActivityLogger, diff --git a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java index fe448d63..8c842786 100644 --- a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java +++ b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java @@ -73,6 +73,8 @@ public class ChooserWrapperActivity List rList, boolean filterLastUsed, ResolverListController resolverListController, + UserHandle userHandle, + Intent targetIntent, ChooserRequestParameters chooserRequest, int maxTargetsPerRow) { PackageManager packageManager = @@ -85,6 +87,8 @@ public class ChooserWrapperActivity rList, filterLastUsed, resolverListController, + userHandle, + targetIntent, this, packageManager, getChooserActivityLogger(), diff --git a/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java b/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java index 7d4b07d8..239bffe0 100644 --- a/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java +++ b/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java @@ -59,8 +59,16 @@ public class ResolverWrapperActivity extends ResolverActivity { public ResolverListAdapter createResolverListAdapter(Context context, List payloadIntents, Intent[] initialIntents, List rList, boolean filterLastUsed, UserHandle userHandle) { - return new ResolverWrapperAdapter(context, payloadIntents, initialIntents, rList, - filterLastUsed, createListController(userHandle), this); + return new ResolverWrapperAdapter( + context, + payloadIntents, + initialIntents, + rList, + filterLastUsed, + createListController(userHandle), + userHandle, + payloadIntents.get(0), // TODO: extract upstream + this); } @Override diff --git a/java/tests/src/com/android/intentresolver/ResolverWrapperAdapter.java b/java/tests/src/com/android/intentresolver/ResolverWrapperAdapter.java index 1504a8ab..a53b41d1 100644 --- a/java/tests/src/com/android/intentresolver/ResolverWrapperAdapter.java +++ b/java/tests/src/com/android/intentresolver/ResolverWrapperAdapter.java @@ -19,6 +19,7 @@ package com.android.intentresolver; import android.content.Context; import android.content.Intent; import android.content.pm.ResolveInfo; +import android.os.UserHandle; import androidx.test.espresso.idling.CountingIdlingResource; @@ -31,14 +32,27 @@ public class ResolverWrapperAdapter extends ResolverListAdapter { private CountingIdlingResource mLabelIdlingResource = new CountingIdlingResource("LoadLabelTask"); - public ResolverWrapperAdapter(Context context, + public ResolverWrapperAdapter( + Context context, List payloadIntents, Intent[] initialIntents, - List rList, boolean filterLastUsed, + List rList, + boolean filterLastUsed, ResolverListController resolverListController, + UserHandle userHandle, + Intent targetIntent, ResolverListCommunicator resolverListCommunicator) { - super(context, payloadIntents, initialIntents, rList, filterLastUsed, - resolverListController, resolverListCommunicator, false); + super( + context, + payloadIntents, + initialIntents, + rList, + filterLastUsed, + resolverListController, + userHandle, + targetIntent, + resolverListCommunicator, + false); } public CountingIdlingResource getLabelIdlingResource() { -- cgit v1.2.3-59-g8ed1b