From 61a786f29bc6112c125d79a0bda8845b2049ee80 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 11 Oct 2022 12:07:49 -0700 Subject: Load direct-share icons asynchronously Adopt ag/19920873 for the unbounded chooser. Test: manual testing Test: atest IntentResolverUnitTests:ChooserListAdapterTest Bug: 215699869 Change-Id: I7c06cda250f709b62abeeda83a21c1b3f3976a9b --- .../android/intentresolver/ChooserActivity.java | 8 ++ .../android/intentresolver/ChooserListAdapter.java | 67 +++++---- .../intentresolver/ResolverListAdapter.java | 3 +- .../chooser/SelectableTargetInfo.java | 16 ++- .../android/intentresolver/chooser/TargetInfo.java | 4 + java/tests/Android.bp | 1 + .../intentresolver/ChooserListAdapterTest.kt | 157 +++++++++++++++++++++ .../UnbundledChooserActivityTest.java | 10 +- 8 files changed, 223 insertions(+), 43 deletions(-) create mode 100644 java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt (limited to 'java') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 5f81b016..75c141e7 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -2457,6 +2457,10 @@ public class ChooserActivity extends ResolverActivity implements avd.start(); // Start animation after generation return avd; } + + public boolean hasDisplayIcon() { + return true; + } } protected static final class EmptyTargetInfo extends NotSelectableTargetInfo { @@ -2470,6 +2474,10 @@ public class ChooserActivity extends ResolverActivity implements public Drawable getDisplayIcon(Context context) { return null; } + + public boolean hasDisplayIcon() { + return false; + } } private void handleScroll(View view, int x, int y, int oldx, int oldy) { diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index 0a8b3890..ad902049 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -98,8 +98,6 @@ public class ChooserListAdapter extends ResolverListAdapter { private AppPredictor mAppPredictor; private AppPredictor.Callback mAppPredictorCallback; - private LoadDirectShareIconTaskProvider mTestLoadDirectShareTaskProvider; - private final ShortcutSelectionLogic mShortcutSelectionLogic; // For pinned direct share labels, if the text spans multiple lines, the TextView will consume @@ -177,7 +175,9 @@ public class ChooserListAdapter extends ResolverListAdapter { final ComponentName cn = ii.getComponent(); if (cn != null) { try { - ai = packageManager.getActivityInfo(ii.getComponent(), 0); + ai = packageManager.getActivityInfo( + ii.getComponent(), + PackageManager.ComponentInfoFlags.of(PackageManager.GET_META_DATA)); ri = new ResolveInfo(); ri.activityInfo = ai; } catch (PackageManager.NameNotFoundException ignored) { @@ -187,7 +187,9 @@ public class ChooserListAdapter extends ResolverListAdapter { if (ai == null) { // Because of AIDL bug, resolveActivity can't accept subclasses of Intent. final Intent rii = (ii.getClass() == Intent.class) ? ii : new Intent(ii); - ri = packageManager.resolveActivity(rii, PackageManager.MATCH_DEFAULT_ONLY); + ri = packageManager.resolveActivity( + rii, + PackageManager.ResolveInfoFlags.of(PackageManager.MATCH_DEFAULT_ONLY)); ai = ri != null ? ri.activityInfo : null; } if (ai == null) { @@ -243,7 +245,6 @@ public class ChooserListAdapter extends ResolverListAdapter { mListViewDataChanged = false; } - private void createPlaceHolders() { mServiceTargets.clear(); for (int i = 0; i < mChooserListCommunicator.getMaxRankedTargets(); i++) { @@ -256,8 +257,9 @@ public class ChooserListAdapter extends ResolverListAdapter { return mInflater.inflate(R.layout.resolve_grid_item, parent, false); } + @VisibleForTesting @Override - protected void onBindView(View view, TargetInfo info, int position) { + public void onBindView(View view, TargetInfo info, int position) { final ViewHolder holder = (ViewHolder) view.getTag(); if (info == null) { @@ -270,12 +272,16 @@ public class ChooserListAdapter extends ResolverListAdapter { holder.bindIcon(info); if (info.isSelectableTargetInfo()) { // direct share targets should append the application name for a better readout - DisplayResolveInfo rInfo = ((SelectableTargetInfo) info).getDisplayResolveInfo(); + SelectableTargetInfo sti = (SelectableTargetInfo) info; + DisplayResolveInfo rInfo = sti.getDisplayResolveInfo(); CharSequence appName = rInfo != null ? rInfo.getDisplayLabel() : ""; CharSequence extendedInfo = info.getExtendedInfo(); String contentDescription = String.join(" ", info.getDisplayLabel(), extendedInfo != null ? extendedInfo : "", appName); holder.updateContentDescription(contentDescription); + if (!sti.hasDisplayIcon()) { + loadDirectShareIcon(sti); + } } else if (info.isDisplayResolveInfo()) { DisplayResolveInfo dri = (DisplayResolveInfo) info; if (!dri.hasDisplayIcon()) { @@ -320,6 +326,20 @@ public class ChooserListAdapter extends ResolverListAdapter { } } + private void loadDirectShareIcon(SelectableTargetInfo info) { + LoadDirectShareIconTask task = (LoadDirectShareIconTask) mIconLoaders.get(info); + if (task == null) { + task = createLoadDirectShareIconTask(info); + mIconLoaders.put(info, task); + task.loadIcon(); + } + } + + @VisibleForTesting + protected LoadDirectShareIconTask createLoadDirectShareIconTask(SelectableTargetInfo info) { + return new LoadDirectShareIconTask(info); + } + void updateAlphabeticalList() { new AsyncTask>() { @Override @@ -660,36 +680,25 @@ public class ChooserListAdapter extends ResolverListAdapter { * Loads direct share targets icons. */ @VisibleForTesting - public class LoadDirectShareIconTask extends AsyncTask { + public class LoadDirectShareIconTask extends AsyncTask { private final SelectableTargetInfo mTargetInfo; - private ViewHolder mViewHolder; private LoadDirectShareIconTask(SelectableTargetInfo targetInfo) { mTargetInfo = targetInfo; } @Override - protected Void doInBackground(Void... voids) { - mTargetInfo.loadIcon(); - return null; + protected Boolean doInBackground(Void... voids) { + return mTargetInfo.loadIcon(); } @Override - protected void onPostExecute(Void arg) { - if (mViewHolder != null) { - mViewHolder.bindIcon(mTargetInfo); + protected void onPostExecute(Boolean isLoaded) { + if (isLoaded) { notifyDataSetChanged(); } } - /** - * Specifies a view holder that will be updated when the task is completed. - */ - public void setViewHolder(ViewHolder viewHolder) { - mViewHolder = viewHolder; - mViewHolder.bindIcon(mTargetInfo); - } - /** * An alias for execute to use with unit tests. */ @@ -697,16 +706,4 @@ public class ChooserListAdapter extends ResolverListAdapter { execute(); } } - - /** - * An interface for the unit tests to override icon loading task creation - */ - @VisibleForTesting - public interface LoadDirectShareIconTaskProvider { - /** - * Provides an instance of the task. - * @return - */ - LoadDirectShareIconTask get(); - } } diff --git a/java/src/com/android/intentresolver/ResolverListAdapter.java b/java/src/com/android/intentresolver/ResolverListAdapter.java index 3c8eae3a..0e58aff8 100644 --- a/java/src/com/android/intentresolver/ResolverListAdapter.java +++ b/java/src/com/android/intentresolver/ResolverListAdapter.java @@ -870,7 +870,8 @@ public class ResolverListAdapter extends BaseAdapter { } /** - * A view holder. + * A view holder keeps a reference to a list view and provides functionality for managing its + * state. */ @VisibleForTesting public static class ViewHolder { diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java index 0f48cfd1..758e77ca 100644 --- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java @@ -157,20 +157,25 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { /** * Load display icon, if needed. */ - public void loadIcon() { + public boolean loadIcon() { ShortcutInfo shortcutInfo; Drawable icon; synchronized (this) { shortcutInfo = mShortcutInfo; icon = mDisplayIcon; } - if (icon == null && shortcutInfo != null) { + boolean shouldLoadIcon = (icon == null) && (shortcutInfo != null); + if (shouldLoadIcon) { icon = getChooserTargetIconDrawable(mChooserTarget, shortcutInfo); + if (icon == null) { + return false; + } synchronized (this) { mDisplayIcon = icon; mShortcutInfo = null; } } + return shouldLoadIcon; } private Drawable getChooserTargetIconDrawable(ChooserTarget target, @@ -307,6 +312,13 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { return mDisplayIcon; } + /** + * @return true if display icon is available + */ + public synchronized boolean hasDisplayIcon() { + return mDisplayIcon != null; + } + public ChooserTarget getChooserTarget() { return mChooserTarget; } diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java index 502a3342..220b6467 100644 --- a/java/src/com/android/intentresolver/chooser/TargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java @@ -111,6 +111,10 @@ public interface TargetInfo { */ Drawable getDisplayIcon(Context context); + /** + * @return true if display icon is available. + */ + boolean hasDisplayIcon(); /** * Clone this target with the given fill-in information. */ diff --git a/java/tests/Android.bp b/java/tests/Android.bp index 9598613c..2913d128 100644 --- a/java/tests/Android.bp +++ b/java/tests/Android.bp @@ -20,6 +20,7 @@ android_test { static_libs: [ "IntentResolver-core", "androidx.test.rules", + "androidx.test.ext.junit", "mockito-target-minus-junit4", "androidx.test.espresso.core", "truth-prebuilt", diff --git a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt new file mode 100644 index 00000000..87e93705 --- /dev/null +++ b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt @@ -0,0 +1,157 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver + +import android.content.ComponentName +import android.content.pm.PackageManager +import android.content.pm.PackageManager.ResolveInfoFlags +import android.os.Bundle +import android.service.chooser.ChooserTarget +import android.view.View +import android.widget.FrameLayout +import android.widget.ImageView +import android.widget.TextView +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.android.intentresolver.ChooserListAdapter.LoadDirectShareIconTask +import com.android.intentresolver.chooser.SelectableTargetInfo +import com.android.intentresolver.chooser.SelectableTargetInfo.SelectableTargetInfoCommunicator +import com.android.internal.R +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.times +import org.mockito.Mockito.verify + +@RunWith(AndroidJUnit4::class) +class ChooserListAdapterTest { + private val packageManager = mock { + whenever( + resolveActivity(any(), any()) + ).thenReturn(mock()) + } + private val context = InstrumentationRegistry.getInstrumentation().getContext() + private val resolverListController = mock() + private val chooserListCommunicator = mock { + whenever(maxRankedTargets).thenReturn(0) + } + private val selectableTargetInfoCommunicator = + mock { + whenever(targetIntent).thenReturn(mock()) + } + private val chooserActivityLogger = mock() + + private fun createChooserListAdapter( + taskProvider: (SelectableTargetInfo?) -> LoadDirectShareIconTask + ) = object : ChooserListAdapter( + context, + emptyList(), + emptyArray(), + emptyList(), + false, + resolverListController, + chooserListCommunicator, + selectableTargetInfoCommunicator, + packageManager, + chooserActivityLogger, + ) { + override fun createLoadDirectShareIconTask( + info: SelectableTargetInfo? + ): LoadDirectShareIconTask = taskProvider(info) + } + + @Before + fun setup() { + // ChooserListAdapter reads DeviceConfig and needs a permission for that. + InstrumentationRegistry + .getInstrumentation() + .getUiAutomation() + .adoptShellPermissionIdentity("android.permission.READ_DEVICE_CONFIG") + } + + @Test + fun testDirectShareTargetLoadingIconIsStarted() { + val view = createView() + val viewHolder = ResolverListAdapter.ViewHolder(view) + view.tag = viewHolder + val targetInfo = createSelectableTargetInfo() + val iconTask = mock() + val testSubject = createChooserListAdapter { iconTask } + testSubject.onBindView(view, targetInfo, 0) + + verify(iconTask, times(1)).loadIcon() + } + + @Test + fun testOnlyOneTaskPerTarget() { + val view = createView() + val viewHolderOne = ResolverListAdapter.ViewHolder(view) + view.tag = viewHolderOne + val targetInfo = createSelectableTargetInfo() + val iconTaskOne = mock() + val testTaskProvider = mock<() -> LoadDirectShareIconTask> { + whenever(invoke()).thenReturn(iconTaskOne) + } + val testSubject = createChooserListAdapter { testTaskProvider.invoke() } + testSubject.onBindView(view, targetInfo, 0) + + val viewHolderTwo = ResolverListAdapter.ViewHolder(view) + view.tag = viewHolderTwo + whenever(testTaskProvider()).thenReturn(mock()) + + testSubject.onBindView(view, targetInfo, 0) + + verify(iconTaskOne, times(1)).loadIcon() + verify(testTaskProvider, times(1)).invoke() + } + + private fun createSelectableTargetInfo(): SelectableTargetInfo = + SelectableTargetInfo( + context, + null, + createChooserTarget(), + 1f, + selectableTargetInfoCommunicator, + null + ) + + private fun createChooserTarget(): ChooserTarget = + ChooserTarget( + "Title", + null, + 1f, + ComponentName("package", "package.Class"), + Bundle() + ) + + private fun createView(): View { + val view = FrameLayout(context) + TextView(context).apply { + id = R.id.text1 + view.addView(this) + } + TextView(context).apply { + id = R.id.text2 + view.addView(this) + } + ImageView(context).apply { + id = R.id.icon + view.addView(this) + } + return view + } +} diff --git a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java index 9ac815f6..752f52fe 100644 --- a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java +++ b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java @@ -2525,7 +2525,7 @@ public class UnbundledChooserActivityTest { when( ChooserActivityOverrideData .getInstance().packageManager - .resolveActivity(any(Intent.class), anyInt())) + .resolveActivity(any(Intent.class), any())) .thenReturn(ri); waitForIdle(); @@ -2558,7 +2558,7 @@ public class UnbundledChooserActivityTest { ChooserActivityOverrideData .getInstance() .packageManager - .resolveActivity(any(Intent.class), anyInt())) + .resolveActivity(any(Intent.class), any())) .thenReturn(createFakeResolveInfo()); waitForIdle(); @@ -2591,7 +2591,7 @@ public class UnbundledChooserActivityTest { ChooserActivityOverrideData .getInstance() .packageManager - .resolveActivity(any(Intent.class), anyInt())) + .resolveActivity(any(Intent.class), any())) .thenReturn(createFakeResolveInfo()); mActivityRule.launchActivity(chooserIntent); @@ -2625,7 +2625,7 @@ public class UnbundledChooserActivityTest { ChooserActivityOverrideData .getInstance() .packageManager - .resolveActivity(any(Intent.class), anyInt())) + .resolveActivity(any(Intent.class), any())) .thenReturn(createFakeResolveInfo()); mActivityRule.launchActivity(chooserIntent); @@ -2660,7 +2660,7 @@ public class UnbundledChooserActivityTest { ChooserActivityOverrideData .getInstance() .packageManager - .resolveActivity(any(Intent.class), anyInt())) + .resolveActivity(any(Intent.class), any())) .thenReturn(ri); waitForIdle(); -- cgit v1.2.3-59-g8ed1b