From 8d51b6f18d47cc6d019529a6b7f1938936d14b4a Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 26 Oct 2022 16:03:10 -0400 Subject: Clean up ChooserActivity's per-profile bookkeeping This is mostly taken from ag/20236439, reimagined for not having extracted the new `ShortcutLoader` component yet. The most notable change is the decoupling of ChooserListAdapter from any AppPredictor instance (or its callback). The previous model gave us a mechanism for associating each of our user IDs to their respective AppPredictor/Callback instances, but otherwise wasn't necessary to support any usage in ChooserListAdapter. OTOH it fell short of actually assigning any responsibility to ChooserListAdapter as the "owner" of these instances, since they're created and retained externally (in ChooserActivity), by the same component that uses them and is ultimately responsible for requesting their destruction (according to its own mechanism for getting back to a particular handle/adapter/AppPredictor/callback "tuple"). Not only is this unnecessarily complex, it also creates a denormalization hazard since ChooserListAdapter resetting its own AppPredictor reference to null after destruction doesn't reset ChooserActivity's reference, which could still be used (and would throw on the use-after-free). The CL also includes some minor cleanups that are helpful to set up for ag/20236439. Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: Ia22c425fcd34627fb67a3ad2543b6d7ac51b5dea --- .../android/intentresolver/ChooserActivity.java | 116 ++++++++++++++------- .../android/intentresolver/ChooserListAdapter.java | 34 ++---- 2 files changed, 88 insertions(+), 62 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index c88b2eb9..6455e4d1 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -74,6 +74,7 @@ import android.os.Message; import android.os.Parcelable; import android.os.PatternMatcher; import android.os.ResultReceiver; +import android.os.SystemClock; import android.os.UserHandle; import android.os.UserManager; import android.os.storage.StorageManager; @@ -87,6 +88,7 @@ import android.text.TextUtils; import android.util.AttributeSet; import android.util.HashedStringCache; import android.util.Log; +import android.util.Pair; import android.util.PluralsMessageFormatter; import android.util.Size; import android.util.Slog; @@ -270,8 +272,6 @@ public class ChooserActivity extends ResolverActivity implements private long mChooserShownTime; protected boolean mIsSuccessfullySelected; - private long mQueriedSharingShortcutsTimeMs; - private int mCurrAvailableWidth = 0; private Insets mLastAppliedInsets = null; private int mLastNumberOfChildren = -1; @@ -313,7 +313,7 @@ public class ChooserActivity extends ResolverActivity implements private final ShortcutToChooserTargetConverter mShortcutToChooserTargetConverter = new ShortcutToChooserTargetConverter(); - private final SparseArray mProfileAppPredictors = new SparseArray<>(); + private final SparseArray mProfileRecords = new SparseArray<>(); private class ContentPreviewCoordinator { private static final int IMAGE_FADE_IN_MILLIS = 150; @@ -483,14 +483,18 @@ public class ChooserActivity extends ResolverActivity implements Log.d(TAG, "LIST_VIEW_UPDATE_MESSAGE; "); } - UserHandle userHandle = (UserHandle) msg.obj; - mChooserMultiProfilePagerAdapter.getListAdapterForUserHandle(userHandle) + mChooserMultiProfilePagerAdapter + .getListAdapterForUserHandle((UserHandle) msg.obj) .refreshListView(); break; case SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS: if (DEBUG) Log.d(TAG, "SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS"); - final ServiceResultInfo[] resultInfos = (ServiceResultInfo[]) msg.obj; + final Pair args = + (Pair) msg.obj; + final UserHandle userHandle = args.first; + final ServiceResultInfo[] resultInfos = args.second; + for (ServiceResultInfo resultInfo : resultInfos) { if (resultInfo.resultTargets != null) { ChooserListAdapter adapterForUserHandle = @@ -508,7 +512,9 @@ public class ChooserActivity extends ResolverActivity implements } logDirectShareTargetReceived( - MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER); + MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER, + userHandle); + sendVoiceChoicesIfNeeded(); getChooserActivityLogger().logSharesheetDirectLoadComplete(); @@ -660,7 +666,7 @@ public class ChooserActivity extends ResolverActivity implements mShouldDisplayLandscape = shouldDisplayLandscape(getResources().getConfiguration().orientation); setRetainInOnStop(intent.getBooleanExtra(EXTRA_PRIVATE_RETAIN_IN_ON_STOP, false)); - createAppPredictors( + createProfileRecords( new AppPredictorFactory( this, target.getStringExtra(Intent.EXTRA_TEXT), @@ -738,31 +744,35 @@ public class ChooserActivity extends ResolverActivity implements return R.style.Theme_DeviceDefault_Chooser; } - private void createAppPredictors(AppPredictorFactory factory) { + private void createProfileRecords(AppPredictorFactory factory) { UserHandle mainUserHandle = getPersonalProfileUserHandle(); - createAppPredictorForProfile(mainUserHandle, factory); + createProfileRecord(mainUserHandle, factory); + UserHandle workUserHandle = getWorkProfileUserHandle(); if (workUserHandle != null) { - createAppPredictorForProfile(workUserHandle, factory); + createProfileRecord(workUserHandle, factory); } } - private void createAppPredictorForProfile(UserHandle userHandle, AppPredictorFactory factory) { + private void createProfileRecord(UserHandle userHandle, AppPredictorFactory factory) { AppPredictor appPredictor = factory.create(userHandle); - if (appPredictor != null) { - mProfileAppPredictors.put(userHandle.getIdentifier(), appPredictor); - } + mProfileRecords.put( + userHandle.getIdentifier(), new ProfileRecord(appPredictor)); + } + + @Nullable + private ProfileRecord getProfileRecord(UserHandle userHandle) { + return mProfileRecords.get(userHandle.getIdentifier(), null); } - private AppPredictor setupAppPredictorForUser(UserHandle userHandle, - AppPredictor.Callback appPredictorCallback) { + private void setupAppPredictorForUser( + UserHandle userHandle, AppPredictor.Callback appPredictorCallback) { AppPredictor appPredictor = getAppPredictor(userHandle); if (appPredictor == null) { - return null; + return; } mDirectShareAppTargetCache = new HashMap<>(); appPredictor.registerPredictionUpdates(this.getMainExecutor(), appPredictorCallback); - return appPredictor; } private AppPredictor.Callback createAppPredictorCallback( @@ -1638,11 +1648,14 @@ public class ChooserActivity extends ResolverActivity implements if (mPreviewCoord != null) mPreviewCoord.cancelLoads(); - mChooserMultiProfilePagerAdapter.getActiveListAdapter().destroyAppPredictor(); - if (mChooserMultiProfilePagerAdapter.getInactiveListAdapter() != null) { - mChooserMultiProfilePagerAdapter.getInactiveListAdapter().destroyAppPredictor(); + destroyProfileRecords(); + } + + private void destroyProfileRecords() { + for (int i = 0; i < mProfileRecords.size(); ++i) { + mProfileRecords.valueAt(i).destroy(); } - mProfileAppPredictors.clear(); + mProfileRecords.clear(); } @Override // ResolverListCommunicator @@ -1987,12 +2000,17 @@ public class ChooserActivity extends ResolverActivity implements @VisibleForTesting protected void queryDirectShareTargets( ChooserListAdapter adapter, boolean skipAppPredictionService) { - mQueriedSharingShortcutsTimeMs = System.currentTimeMillis(); + ProfileRecord record = getProfileRecord(adapter.getUserHandle()); + if (record == null) { + return; + } + + record.loadingStartTime = SystemClock.elapsedRealtime(); + UserHandle userHandle = adapter.getUserHandle(); if (!skipAppPredictionService) { - AppPredictor appPredictor = getAppPredictor(userHandle); - if (appPredictor != null) { - appPredictor.requestPredictionUpdate(); + if (record.appPredictor != null) { + record.appPredictor.requestPredictionUpdate(); return; } } @@ -2062,8 +2080,7 @@ public class ChooserActivity extends ResolverActivity implements // for direct share targets. After ShareSheet is refactored we should use the // ShareShortcutInfos directly. List resultRecords = new ArrayList<>(); - for (int i = 0; i < chooserListAdapter.getDisplayResolveInfoCount(); i++) { - DisplayResolveInfo displayResolveInfo = chooserListAdapter.getDisplayResolveInfo(i); + for (DisplayResolveInfo displayResolveInfo : chooserListAdapter.getDisplayResolveInfos()) { List matchingShortcuts = filterShortcutsByTargetComponentName( resultList, displayResolveInfo.getResolvedComponentName()); @@ -2085,7 +2102,7 @@ public class ChooserActivity extends ResolverActivity implements } sendShortcutManagerShareTargetResults( - shortcutType, resultRecords.toArray(new ServiceResultInfo[0])); + userHandle, shortcutType, resultRecords.toArray(new ServiceResultInfo[0])); } private List filterShortcutsByTargetComponentName( @@ -2100,10 +2117,10 @@ public class ChooserActivity extends ResolverActivity implements } private void sendShortcutManagerShareTargetResults( - int shortcutType, ServiceResultInfo[] results) { + UserHandle userHandle, int shortcutType, ServiceResultInfo[] results) { final Message msg = Message.obtain(); msg.what = ChooserHandler.SHORTCUT_MANAGER_ALL_SHARE_TARGET_RESULTS; - msg.obj = results; + msg.obj = Pair.create(userHandle, results); msg.arg1 = shortcutType; mChooserHandler.sendMessage(msg); } @@ -2126,8 +2143,14 @@ public class ChooserActivity extends ResolverActivity implements return false; } - private void logDirectShareTargetReceived(int logCategory) { - final int apiLatency = (int) (System.currentTimeMillis() - mQueriedSharingShortcutsTimeMs); + private void logDirectShareTargetReceived(int logCategory, UserHandle forUser) { + ProfileRecord profileRecord = getProfileRecord(forUser); + if (profileRecord == null) { + return; + } + + final int apiLatency = + (int) (SystemClock.elapsedRealtime() - profileRecord.loadingStartTime); getMetricsLogger().write(new LogMaker(logCategory).setSubtype(apiLatency)); } @@ -2213,7 +2236,8 @@ public class ChooserActivity extends ResolverActivity implements @Nullable private AppPredictor getAppPredictor(UserHandle userHandle) { - return mProfileAppPredictors.get(userHandle.getIdentifier(), null); + ProfileRecord record = getProfileRecord(userHandle); + return (record == null) ? null : record.appPredictor; } void onRefinementResult(TargetInfo selectedTarget, Intent matchingIntent) { @@ -2333,9 +2357,7 @@ public class ChooserActivity extends ResolverActivity implements if (!ActivityManager.isLowRamDeviceStatic()) { AppPredictor.Callback appPredictorCallback = createAppPredictorCallback(chooserListAdapter); - AppPredictor appPredictor = setupAppPredictorForUser(userHandle, appPredictorCallback); - chooserListAdapter.setAppPredictor(appPredictor); - chooserListAdapter.setAppPredictorCallback(appPredictorCallback); + setupAppPredictorForUser(userHandle, appPredictorCallback); } return new ChooserGridAdapter(chooserListAdapter); } @@ -4028,4 +4050,22 @@ public class ChooserActivity extends ResolverActivity implements private static Map emptyIfNull(@Nullable Map map) { return map == null ? Collections.emptyMap() : map; } + + private static class ProfileRecord { + /** The {@link AppPredictor} for this profile, if any. */ + @Nullable + public final AppPredictor appPredictor; + + public long loadingStartTime; + + ProfileRecord(@Nullable AppPredictor appPredictor) { + this.appPredictor = appPredictor; + } + + public void destroy() { + if (appPredictor != null) { + appPredictor.destroy(); + } + } + } } diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index 25b50625..2443659f 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -21,7 +21,6 @@ import static com.android.intentresolver.ChooserActivity.TARGET_TYPE_SHORTCUTS_F import android.annotation.Nullable; import android.app.ActivityManager; -import android.app.prediction.AppPredictor; import android.app.prediction.AppTarget; import android.content.ComponentName; import android.content.Context; @@ -95,8 +94,6 @@ public class ChooserListAdapter extends ResolverListAdapter { // Sorted list of DisplayResolveInfos for the alphabetical app section. private List mSortedList = new ArrayList<>(); - private AppPredictor mAppPredictor; - private AppPredictor.Callback mAppPredictorCallback; private final ShortcutSelectionLogic mShortcutSelectionLogic; @@ -218,10 +215,6 @@ public class ChooserListAdapter extends ResolverListAdapter { } } - AppPredictor getAppPredictor() { - return mAppPredictor; - } - @Override public void handlePackagesChanged() { if (DEBUG) { @@ -435,6 +428,16 @@ public class ChooserListAdapter extends ResolverListAdapter { return Math.min(spacesAvailable, super.getCount()); } + /** Get all the {@link DisplayResolveInfo} data for our targets. */ + public DisplayResolveInfo[] getDisplayResolveInfos() { + int size = getDisplayResolveInfoCount(); + DisplayResolveInfo[] resolvedTargets = new DisplayResolveInfo[size]; + for (int i = 0; i < size; i++) { + resolvedTargets[i] = getDisplayResolveInfo(i); + } + return resolvedTargets; + } + public int getPositionTargetType(int position) { int offset = 0; @@ -469,7 +472,6 @@ public class ChooserListAdapter extends ResolverListAdapter { return targetInfoForPosition(position, true); } - /** * Find target info for a given position. * Since ChooserActivity displays several sections of content, determine which @@ -642,22 +644,6 @@ public class ChooserListAdapter extends ResolverListAdapter { }; } - public void setAppPredictor(AppPredictor appPredictor) { - mAppPredictor = appPredictor; - } - - public void setAppPredictorCallback(AppPredictor.Callback appPredictorCallback) { - mAppPredictorCallback = appPredictorCallback; - } - - public void destroyAppPredictor() { - if (getAppPredictor() != null) { - getAppPredictor().unregisterPredictionUpdates(mAppPredictorCallback); - getAppPredictor().destroy(); - setAppPredictor(null); - } - } - /** * Necessary methods to communicate between {@link ChooserListAdapter} * and {@link ChooserActivity}. -- cgit v1.2.3-59-g8ed1b