From 1b11cc1a17de59f284b1b3dc28375ac3bc63c767 Mon Sep 17 00:00:00 2001 From: arangelov Date: Mon, 27 Apr 2020 17:23:04 +0100 Subject: [PATCH] Only perform autolaunch if list has completely rebuilt Previously we would check for autolaunch the first time onPostListReady is called, but that is too early in the case when we start the async task for sorting the targets. In that case onPostListReady is called twice - once before task is launched, and once after task is finished. We fix this by passing the rebuildListCompleted parameter to the onPostListReady callback and use it to check for autolaunch. Test: atest ChooserActivity Test: atest ResolverActivity Test: manual Fixes: 154728817 Change-Id: Ib1b049af00d791898b908bdb65971c07c558f3ac --- .../app/AbstractMultiProfilePagerAdapter.java | 2 +- .../android/internal/app/ChooserActivity.java | 17 ++++-- .../internal/app/ChooserListAdapter.java | 16 +++--- .../internal/app/ResolverActivity.java | 5 +- .../internal/app/ResolverListAdapter.java | 12 ++-- .../internal/app/ChooserActivityTest.java | 57 ++++++++++++++++++- .../internal/app/ChooserWrapperActivity.java | 14 +++++ 7 files changed, 101 insertions(+), 22 deletions(-) diff --git a/core/java/com/android/internal/app/AbstractMultiProfilePagerAdapter.java b/core/java/com/android/internal/app/AbstractMultiProfilePagerAdapter.java index b58dbddab44f0..5850e61fd0627 100644 --- a/core/java/com/android/internal/app/AbstractMultiProfilePagerAdapter.java +++ b/core/java/com/android/internal/app/AbstractMultiProfilePagerAdapter.java @@ -301,7 +301,7 @@ public abstract class AbstractMultiProfilePagerAdapter extends PagerAdapter { private boolean rebuildTab(ResolverListAdapter activeListAdapter, boolean doPostProcessing) { if (shouldShowNoCrossProfileIntentsEmptyState(activeListAdapter)) { - activeListAdapter.postListReadyRunnable(doPostProcessing); + activeListAdapter.postListReadyRunnable(doPostProcessing, /* rebuildCompleted */ true); return false; } return activeListAdapter.rebuildList(doPostProcessing); diff --git a/core/java/com/android/internal/app/ChooserActivity.java b/core/java/com/android/internal/app/ChooserActivity.java index b671fa74bf9ae..a84d9640c7f87 100644 --- a/core/java/com/android/internal/app/ChooserActivity.java +++ b/core/java/com/android/internal/app/ChooserActivity.java @@ -2404,10 +2404,9 @@ public class ChooserActivity extends ResolverActivity implements public ChooserGridAdapter createChooserGridAdapter(Context context, List payloadIntents, Intent[] initialIntents, List rList, boolean filterLastUsed, boolean useLayoutForBrowsables, UserHandle userHandle) { - ChooserListAdapter chooserListAdapter = new ChooserListAdapter(context, payloadIntents, - initialIntents, rList, - filterLastUsed, createListController(userHandle), useLayoutForBrowsables, - this, this); + ChooserListAdapter chooserListAdapter = createChooserListAdapter(context, payloadIntents, + initialIntents, rList, filterLastUsed, + useLayoutForBrowsables, createListController(userHandle)); AppPredictor.Callback appPredictorCallback = createAppPredictorCallback(chooserListAdapter); AppPredictor appPredictor = setupAppPredictorForUser(userHandle, appPredictorCallback); chooserListAdapter.setAppPredictor(appPredictor); @@ -2415,6 +2414,16 @@ public class ChooserActivity extends ResolverActivity implements return new ChooserGridAdapter(chooserListAdapter); } + @VisibleForTesting + public ChooserListAdapter createChooserListAdapter(Context context, + List payloadIntents, Intent[] initialIntents, List rList, + boolean filterLastUsed, boolean useLayoutForBrowsables, + ResolverListController resolverListController) { + return new ChooserListAdapter(context, payloadIntents, initialIntents, rList, + filterLastUsed, resolverListController, useLayoutForBrowsables, + this, this, context.getPackageManager()); + } + @VisibleForTesting protected ResolverListController createListController(UserHandle userHandle) { AppPredictor appPredictor = getAppPredictorForShareActivitiesIfEnabled(userHandle); diff --git a/core/java/com/android/internal/app/ChooserListAdapter.java b/core/java/com/android/internal/app/ChooserListAdapter.java index bdfcda5202817..0edc5ab93763f 100644 --- a/core/java/com/android/internal/app/ChooserListAdapter.java +++ b/core/java/com/android/internal/app/ChooserListAdapter.java @@ -88,7 +88,7 @@ public class ChooserListAdapter extends ResolverListAdapter { private final int mMaxShortcutTargetsPerApp; private final ChooserListCommunicator mChooserListCommunicator; private final SelectableTargetInfo.SelectableTargetInfoCommunicator - mSelectableTargetInfoComunicator; + mSelectableTargetInfoCommunicator; private int mNumShortcutResults = 0; @@ -117,7 +117,8 @@ public class ChooserListAdapter extends ResolverListAdapter { boolean filterLastUsed, ResolverListController resolverListController, boolean useLayoutForBrowsables, ChooserListCommunicator chooserListCommunicator, - SelectableTargetInfo.SelectableTargetInfoCommunicator selectableTargetInfoComunicator) { + SelectableTargetInfo.SelectableTargetInfoCommunicator selectableTargetInfoCommunicator, + PackageManager packageManager) { // 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, @@ -128,10 +129,9 @@ public class ChooserListAdapter extends ResolverListAdapter { mMaxShortcutTargetsPerApp = context.getResources().getInteger(R.integer.config_maxShortcutTargetsPerApp); mChooserListCommunicator = chooserListCommunicator; - mSelectableTargetInfoComunicator = selectableTargetInfoComunicator; + mSelectableTargetInfoCommunicator = selectableTargetInfoCommunicator; if (initialIntents != null) { - final PackageManager pm = context.getPackageManager(); for (int i = 0; i < initialIntents.length; i++) { final Intent ii = initialIntents[i]; if (ii == null) { @@ -147,7 +147,7 @@ public class ChooserListAdapter extends ResolverListAdapter { final ComponentName cn = ii.getComponent(); if (cn != null) { try { - ai = pm.getActivityInfo(ii.getComponent(), 0); + ai = packageManager.getActivityInfo(ii.getComponent(), 0); ri = new ResolveInfo(); ri.activityInfo = ai; } catch (PackageManager.NameNotFoundException ignored) { @@ -155,7 +155,7 @@ public class ChooserListAdapter extends ResolverListAdapter { } } if (ai == null) { - ri = pm.resolveActivity(ii, PackageManager.MATCH_DEFAULT_ONLY); + ri = packageManager.resolveActivity(ii, PackageManager.MATCH_DEFAULT_ONLY); ai = ri != null ? ri.activityInfo : null; } if (ai == null) { @@ -458,7 +458,7 @@ public class ChooserListAdapter extends ResolverListAdapter { UserHandle userHandle = getUserHandle(); Context contextAsUser = mContext.createContextAsUser(userHandle, 0 /* flags */); boolean isInserted = insertServiceTarget(new SelectableTargetInfo(contextAsUser, - origTarget, target, targetScore, mSelectableTargetInfoComunicator, + origTarget, target, targetScore, mSelectableTargetInfoCommunicator, (isShortcutResult ? directShareToShortcutInfos.get(target) : null))); if (isInserted && isShortcutResult) { @@ -508,7 +508,7 @@ public class ChooserListAdapter extends ResolverListAdapter { .map(target -> new SelectableTargetInfo( contextAsUser, origTarget, target, target.getScore(), - mSelectableTargetInfoComunicator, + mSelectableTargetInfoCommunicator, (isShortcutResult ? directShareToShortcutInfos.get(target) : null)) ) diff --git a/core/java/com/android/internal/app/ResolverActivity.java b/core/java/com/android/internal/app/ResolverActivity.java index 84c833ea5ffe6..306701838ed46 100644 --- a/core/java/com/android/internal/app/ResolverActivity.java +++ b/core/java/com/android/internal/app/ResolverActivity.java @@ -1026,7 +1026,8 @@ public class ResolverActivity extends Activity implements } @Override // ResolverListCommunicator - public final void onPostListReady(ResolverListAdapter listAdapter, boolean doPostProcessing) { + public final void onPostListReady(ResolverListAdapter listAdapter, boolean doPostProcessing, + boolean rebuildCompleted) { if (isAutolaunching()) { return; } @@ -1037,7 +1038,7 @@ public class ResolverActivity extends Activity implements } // showEmptyResolverListEmptyState can mark the tab as loaded, // which is a precondition for auto launching - if (maybeAutolaunchActivity()) { + if (rebuildCompleted && maybeAutolaunchActivity()) { return; } if (doPostProcessing) { diff --git a/core/java/com/android/internal/app/ResolverListAdapter.java b/core/java/com/android/internal/app/ResolverListAdapter.java index cee8a923e198e..e8f111361e69d 100644 --- a/core/java/com/android/internal/app/ResolverListAdapter.java +++ b/core/java/com/android/internal/app/ResolverListAdapter.java @@ -275,7 +275,7 @@ public class ResolverListAdapter extends BaseAdapter { } setPlaceholderCount(placeholderCount); createSortingTask(doPostProcessing).execute(currentResolveList); - postListReadyRunnable(doPostProcessing); + postListReadyRunnable(doPostProcessing, /* rebuildCompleted */ false); return false; } else { processSortedList(currentResolveList, doPostProcessing); @@ -363,7 +363,7 @@ public class ResolverListAdapter extends BaseAdapter { } mResolverListCommunicator.sendVoiceChoicesIfNeeded(); - postListReadyRunnable(doPostProcessing); + postListReadyRunnable(doPostProcessing, /* rebuildCompleted */ true); mIsTabLoaded = true; } @@ -373,14 +373,15 @@ public class ResolverListAdapter extends BaseAdapter { * handler thread to update after the current task is finished. * @param doPostProcessing Whether to update the UI and load additional direct share targets * after the list has been rebuilt + * @param rebuildCompleted Whether the list has been completely rebuilt */ - void postListReadyRunnable(boolean doPostProcessing) { + void postListReadyRunnable(boolean doPostProcessing, boolean rebuildCompleted) { if (mPostListReadyRunnable == null) { mPostListReadyRunnable = new Runnable() { @Override public void run() { mResolverListCommunicator.onPostListReady(ResolverListAdapter.this, - doPostProcessing); + doPostProcessing, rebuildCompleted); mPostListReadyRunnable = null; } }; @@ -642,7 +643,8 @@ public class ResolverListAdapter extends BaseAdapter { Intent getReplacementIntent(ActivityInfo activityInfo, Intent defIntent); - void onPostListReady(ResolverListAdapter listAdapter, boolean updateUi); + void onPostListReady(ResolverListAdapter listAdapter, boolean updateUi, + boolean rebuildCompleted); void sendVoiceChoicesIfNeeded(); diff --git a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java index c52555b5ed326..edad5c40fccd8 100644 --- a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java +++ b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java @@ -34,6 +34,7 @@ import static com.android.internal.app.ChooserListAdapter.SHORTCUT_TARGET_SCORE_ import static com.android.internal.app.ChooserWrapperActivity.sOverrides; import static com.android.internal.app.MatcherUtils.first; +import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertTrue; import static org.hamcrest.CoreMatchers.is; @@ -41,6 +42,8 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -55,6 +58,8 @@ import android.content.ClipboardManager; import android.content.ComponentName; import android.content.Context; import android.content.Intent; +import android.content.pm.ActivityInfo; +import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.content.pm.ShortcutInfo; @@ -302,7 +307,7 @@ public class ChooserActivityTest { waitForIdle(); UsageStatsManager usm = activity.getUsageStatsManager(); verify(sOverrides.resolverListController, times(1)) - .topK(Mockito.any(List.class), Mockito.anyInt()); + .topK(any(List.class), anyInt()); assertThat(activity.getIsSelected(), is(false)); sOverrides.onSafelyStartCallback = targetInfo -> { return true; @@ -312,7 +317,7 @@ public class ChooserActivityTest { .perform(click()); waitForIdle(); verify(sOverrides.resolverListController, times(1)) - .updateChooserCounts(Mockito.anyString(), Mockito.anyInt(), Mockito.anyString()); + .updateChooserCounts(Mockito.anyString(), anyInt(), Mockito.anyString()); verify(sOverrides.resolverListController, times(1)) .updateModel(toChoose.activityInfo.getComponentName()); assertThat(activity.getIsSelected(), is(true)); @@ -1750,6 +1755,54 @@ public class ChooserActivityTest { assertThat(chosen[0], is(personalResolvedComponentInfos.get(1).getResolveInfoAt(0))); } + @Test + public void testOneInitialIntent_noAutolaunch() { + List personalResolvedComponentInfos = + createResolvedComponentsForTest(1); + when(sOverrides.resolverListController.getResolversForIntent(Mockito.anyBoolean(), + Mockito.anyBoolean(), + Mockito.isA(List.class))) + .thenReturn(new ArrayList<>(personalResolvedComponentInfos)); + Intent chooserIntent = createChooserIntent(new Intent[] {new Intent("action.fake")}); + ResolveInfo[] chosen = new ResolveInfo[1]; + sOverrides.onSafelyStartCallback = targetInfo -> { + chosen[0] = targetInfo.getResolveInfo(); + return true; + }; + sOverrides.packageManager = mock(PackageManager.class); + ResolveInfo ri = createFakeResolveInfo(); + when(sOverrides.packageManager.resolveActivity(any(Intent.class), anyInt())).thenReturn(ri); + waitForIdle(); + + mActivityRule.launchActivity(chooserIntent); + waitForIdle(); + + assertNull(chosen[0]); + } + + private Intent createChooserIntent(Intent[] initialIntents) { + Intent chooserIntent = new Intent(); + chooserIntent.setAction(Intent.ACTION_CHOOSER); + chooserIntent.putExtra(Intent.EXTRA_TEXT, "testing intent sending"); + chooserIntent.putExtra(Intent.EXTRA_TITLE, "some title"); + chooserIntent.putExtra(Intent.EXTRA_INTENT, createSendTextIntent()); + chooserIntent.setType("text/plain"); + if (initialIntents != null) { + chooserIntent.putExtra(Intent.EXTRA_INITIAL_INTENTS, initialIntents); + } + return chooserIntent; + } + + private ResolveInfo createFakeResolveInfo() { + ResolveInfo ri = new ResolveInfo(); + ri.activityInfo = new ActivityInfo(); + ri.activityInfo.name = "FakeActivityName"; + ri.activityInfo.packageName = "fake.package.name"; + ri.activityInfo.applicationInfo = new ApplicationInfo(); + ri.activityInfo.applicationInfo.packageName = "fake.package.name"; + return ri; + } + private Intent createSendTextIntent() { Intent sendIntent = new Intent(); sendIntent.setAction(Intent.ACTION_SEND); diff --git a/core/tests/coretests/src/com/android/internal/app/ChooserWrapperActivity.java b/core/tests/coretests/src/com/android/internal/app/ChooserWrapperActivity.java index 5b83f952b7454..071b2259e1f09 100644 --- a/core/tests/coretests/src/com/android/internal/app/ChooserWrapperActivity.java +++ b/core/tests/coretests/src/com/android/internal/app/ChooserWrapperActivity.java @@ -58,6 +58,18 @@ public class ChooserWrapperActivity extends ChooserActivity { return multiProfilePagerAdapter; } + @Override + public ChooserListAdapter createChooserListAdapter(Context context, List payloadIntents, + Intent[] initialIntents, List rList, boolean filterLastUsed, + boolean useLayoutForBrowsables, ResolverListController resolverListController) { + PackageManager packageManager = + sOverrides.packageManager == null ? context.getPackageManager() + : sOverrides.packageManager; + return new ChooserListAdapter(context, payloadIntents, initialIntents, rList, + filterLastUsed, resolverListController, useLayoutForBrowsables, + this, this, packageManager); + } + ChooserListAdapter getAdapter() { return mChooserMultiProfilePagerAdapter.getActiveListAdapter(); } @@ -217,6 +229,7 @@ public class ChooserWrapperActivity extends ChooserActivity { public boolean hasCrossProfileIntents; public boolean isQuietModeEnabled; public AbstractMultiProfilePagerAdapter.Injector multiPagerAdapterInjector; + public PackageManager packageManager; public void reset() { onSafelyStartCallback = null; @@ -235,6 +248,7 @@ public class ChooserWrapperActivity extends ChooserActivity { workProfileUserHandle = null; hasCrossProfileIntents = true; isQuietModeEnabled = false; + packageManager = null; multiPagerAdapterInjector = new AbstractMultiProfilePagerAdapter.Injector() { @Override public boolean hasCrossProfileIntents(List intents, int sourceUserId,