diff options
4 files changed, 28 insertions, 304 deletions
diff --git a/core/java/com/android/internal/app/ChooserListAdapter.java b/core/java/com/android/internal/app/ChooserListAdapter.java index e57b90ad1612..1ec5325623ec 100644 --- a/core/java/com/android/internal/app/ChooserListAdapter.java +++ b/core/java/com/android/internal/app/ChooserListAdapter.java @@ -43,7 +43,6 @@ import android.view.ViewGroup; import android.widget.TextView; import com.android.internal.R; -import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.ResolverActivity.ResolvedComponentInfo; import com.android.internal.app.chooser.ChooserTargetInfo; import com.android.internal.app.chooser.DisplayResolveInfo; @@ -87,7 +86,7 @@ public class ChooserListAdapter extends ResolverListAdapter { private final ChooserActivityLogger mChooserActivityLogger; private int mNumShortcutResults = 0; - private final Map<TargetInfo, AsyncTask> mIconLoaders = new HashMap<>(); + private Map<DisplayResolveInfo, LoadIconTask> mIconLoaders = new HashMap<>(); private boolean mApplySharingAppLimits; // Reserve spots for incoming direct share targets by adding placeholders @@ -105,8 +104,6 @@ public class ChooserListAdapter extends ResolverListAdapter { private AppPredictor mAppPredictor; private AppPredictor.Callback mAppPredictorCallback; - private LoadDirectShareIconTaskProvider mTestLoadDirectShareTaskProvider; - // 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 @@ -243,6 +240,7 @@ public class ChooserListAdapter extends ResolverListAdapter { mListViewDataChanged = false; } + private void createPlaceHolders() { mNumShortcutResults = 0; mServiceTargets.clear(); @@ -267,25 +265,31 @@ public class ChooserListAdapter extends ResolverListAdapter { return; } - if (info instanceof DisplayResolveInfo) { - DisplayResolveInfo dri = (DisplayResolveInfo) info; - holder.bindLabel(dri.getDisplayLabel(), dri.getExtendedInfo(), alwaysShowSubLabel()); - startDisplayResolveInfoIconLoading(holder, dri); - } else { + if (!(info instanceof DisplayResolveInfo)) { holder.bindLabel(info.getDisplayLabel(), info.getExtendedInfo(), alwaysShowSubLabel()); + holder.bindIcon(info); if (info instanceof SelectableTargetInfo) { - SelectableTargetInfo selectableInfo = (SelectableTargetInfo) info; // direct share targets should append the application name for a better readout - DisplayResolveInfo rInfo = selectableInfo.getDisplayResolveInfo(); + DisplayResolveInfo rInfo = ((SelectableTargetInfo) info).getDisplayResolveInfo(); CharSequence appName = rInfo != null ? rInfo.getDisplayLabel() : ""; - CharSequence extendedInfo = selectableInfo.getExtendedInfo(); - String contentDescription = String.join(" ", selectableInfo.getDisplayLabel(), + CharSequence extendedInfo = info.getExtendedInfo(); + String contentDescription = String.join(" ", info.getDisplayLabel(), extendedInfo != null ? extendedInfo : "", appName); holder.updateContentDescription(contentDescription); - startSelectableTargetInfoIconLoading(holder, selectableInfo); + } + } else { + DisplayResolveInfo dri = (DisplayResolveInfo) info; + holder.bindLabel(dri.getDisplayLabel(), dri.getExtendedInfo(), alwaysShowSubLabel()); + LoadIconTask task = mIconLoaders.get(dri); + if (task == null) { + task = new LoadIconTask(dri, holder); + mIconLoaders.put(dri, task); + task.execute(); } else { - holder.bindIcon(info); + // The holder was potentially changed as the underlying items were + // reshuffled, so reset the target holder + task.setViewHolder(holder); } } @@ -326,32 +330,6 @@ public class ChooserListAdapter extends ResolverListAdapter { } } - private void startDisplayResolveInfoIconLoading(ViewHolder holder, DisplayResolveInfo info) { - LoadIconTask task = (LoadIconTask) mIconLoaders.get(info); - if (task == null) { - task = new LoadIconTask(info, holder); - mIconLoaders.put(info, task); - task.execute(); - } else { - // The holder was potentially changed as the underlying items were - // reshuffled, so reset the target holder - task.setViewHolder(holder); - } - } - - private void startSelectableTargetInfoIconLoading( - ViewHolder holder, SelectableTargetInfo info) { - LoadDirectShareIconTask task = (LoadDirectShareIconTask) mIconLoaders.get(info); - if (task == null) { - task = mTestLoadDirectShareTaskProvider == null - ? new LoadDirectShareIconTask(info) - : mTestLoadDirectShareTaskProvider.get(); - mIconLoaders.put(info, task); - task.loadIcon(); - } - task.setViewHolder(holder); - } - void updateAlphabeticalList() { new AsyncTask<Void, Void, List<DisplayResolveInfo>>() { @Override @@ -366,7 +344,7 @@ public class ChooserListAdapter extends ResolverListAdapter { Map<String, DisplayResolveInfo> consolidated = new HashMap<>(); for (DisplayResolveInfo info : allTargets) { String resolvedTarget = info.getResolvedComponentName().getPackageName() - + '#' + info.getDisplayLabel(); + + '#' + info.getDisplayLabel(); DisplayResolveInfo multiDri = consolidated.get(resolvedTarget); if (multiDri == null) { consolidated.put(resolvedTarget, info); @@ -375,7 +353,7 @@ public class ChooserListAdapter extends ResolverListAdapter { } else { // create consolidated target from the single DisplayResolveInfo MultiDisplayResolveInfo multiDisplayResolveInfo = - new MultiDisplayResolveInfo(resolvedTarget, multiDri); + new MultiDisplayResolveInfo(resolvedTarget, multiDri); multiDisplayResolveInfo.addTarget(info); consolidated.put(resolvedTarget, multiDisplayResolveInfo); } @@ -762,24 +740,10 @@ public class ChooserListAdapter extends ResolverListAdapter { } /** - * An alias for onBindView to use with unit tests. - */ - @VisibleForTesting - public void testViewBind(View view, TargetInfo info, int position) { - onBindView(view, info, position); - } - - @VisibleForTesting - public void setTestLoadDirectShareTaskProvider(LoadDirectShareIconTaskProvider provider) { - mTestLoadDirectShareTaskProvider = provider; - } - - /** * Necessary methods to communicate between {@link ChooserListAdapter} * and {@link ChooserActivity}. */ - @VisibleForTesting - public interface ChooserListCommunicator extends ResolverListCommunicator { + interface ChooserListCommunicator extends ResolverListCommunicator { int getMaxRankedTargets(); @@ -787,59 +751,4 @@ public class ChooserListAdapter extends ResolverListAdapter { boolean isSendAction(Intent targetIntent); } - - /** - * Loads direct share targets icons. - */ - @VisibleForTesting - public class LoadDirectShareIconTask extends AsyncTask<Void, Void, Void> { - private final SelectableTargetInfo mTargetInfo; - private ViewHolder mViewHolder; - - private LoadDirectShareIconTask(SelectableTargetInfo targetInfo) { - mTargetInfo = targetInfo; - } - - @Override - protected Void doInBackground(Void... voids) { - mTargetInfo.loadIcon(); - return null; - } - - @Override - protected void onPostExecute(Void arg) { - if (mViewHolder != null) { - mViewHolder.bindIcon(mTargetInfo); - notifyDataSetChanged(); - } - } - - /** - * Specifies a view holder that will be updated when the task is completed. - */ - public void setViewHolder(ViewHolder viewHolder) { - mViewHolder = viewHolder; - mViewHolder.bindIcon(mTargetInfo); - notifyDataSetChanged(); - } - - /** - * An alias for execute to use with unit tests. - */ - public void loadIcon() { - 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/core/java/com/android/internal/app/ResolverListAdapter.java b/core/java/com/android/internal/app/ResolverListAdapter.java index 3a3baa726f17..66fff5c13ab7 100644 --- a/core/java/com/android/internal/app/ResolverListAdapter.java +++ b/core/java/com/android/internal/app/ResolverListAdapter.java @@ -834,11 +834,7 @@ public class ResolverListAdapter extends BaseAdapter { void onHandlePackagesChanged(ResolverListAdapter listAdapter); } - /** - * A view holder. - */ - @VisibleForTesting - public static class ViewHolder { + static class ViewHolder { public View itemView; public Drawable defaultItemViewBackground; @@ -846,8 +842,7 @@ public class ResolverListAdapter extends BaseAdapter { public TextView text2; public ImageView icon; - @VisibleForTesting - public ViewHolder(View view) { + ViewHolder(View view) { itemView = view; defaultItemViewBackground = view.getBackground(); text = (TextView) view.findViewById(com.android.internal.R.id.text1); diff --git a/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java b/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java index 37eab40ba2b8..264e4f76d35d 100644 --- a/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java +++ b/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java @@ -37,7 +37,6 @@ import android.service.chooser.ChooserTarget; import android.text.SpannableStringBuilder; import android.util.Log; -import com.android.internal.annotations.GuardedBy; import com.android.internal.app.ChooserActivity; import com.android.internal.app.ResolverActivity; import com.android.internal.app.ResolverListAdapter.ActivityInfoPresentationGetter; @@ -60,11 +59,8 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { private final String mDisplayLabel; private final PackageManager mPm; private final SelectableTargetInfoCommunicator mSelectableTargetInfoCommunicator; - @GuardedBy("this") - private ShortcutInfo mShortcutInfo; private Drawable mBadgeIcon = null; private CharSequence mBadgeContentDescription; - @GuardedBy("this") private Drawable mDisplayIcon; private final Intent mFillInIntent; private final int mFillInFlags; @@ -82,7 +78,6 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { mModifiedScore = modifiedScore; mPm = mContext.getPackageManager(); mSelectableTargetInfoCommunicator = selectableTargetInfoComunicator; - mShortcutInfo = shortcutInfo; mIsPinned = shortcutInfo != null && shortcutInfo.isPinned(); if (sourceInfo != null) { final ResolveInfo ri = sourceInfo.getResolveInfo(); @@ -97,6 +92,8 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { } } } + // TODO(b/121287224): do this in the background thread, and only for selected targets + mDisplayIcon = getChooserTargetIconDrawable(chooserTarget, shortcutInfo); if (sourceInfo != null) { mBackupResolveInfo = null; @@ -121,10 +118,7 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { mChooserTarget = other.mChooserTarget; mBadgeIcon = other.mBadgeIcon; mBadgeContentDescription = other.mBadgeContentDescription; - synchronized (other) { - mShortcutInfo = other.mShortcutInfo; - mDisplayIcon = other.mDisplayIcon; - } + mDisplayIcon = other.mDisplayIcon; mFillInIntent = fillInIntent; mFillInFlags = flags; mModifiedScore = other.mModifiedScore; @@ -147,25 +141,6 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { return mSourceInfo; } - /** - * Load display icon, if needed. - */ - public void loadIcon() { - ShortcutInfo shortcutInfo; - Drawable icon; - synchronized (this) { - shortcutInfo = mShortcutInfo; - icon = mDisplayIcon; - } - if (icon == null && shortcutInfo != null) { - icon = getChooserTargetIconDrawable(mChooserTarget, shortcutInfo); - synchronized (this) { - mDisplayIcon = icon; - mShortcutInfo = null; - } - } - } - private Drawable getChooserTargetIconDrawable(ChooserTarget target, @Nullable ShortcutInfo shortcutInfo) { Drawable directShareIcon = null; @@ -295,7 +270,7 @@ public final class SelectableTargetInfo implements ChooserTargetInfo { } @Override - public synchronized Drawable getDisplayIcon(Context context) { + public Drawable getDisplayIcon(Context context) { return mDisplayIcon; } diff --git a/core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt b/core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt deleted file mode 100644 index f027e0b50c18..000000000000 --- a/core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt +++ /dev/null @@ -1,155 +0,0 @@ -/* - * 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.internal.app - -import android.content.ComponentName -import android.content.pm.PackageManager -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.internal.R -import com.android.internal.app.chooser.SelectableTargetInfo -import com.android.internal.app.chooser.SelectableTargetInfo.SelectableTargetInfoCommunicator -import com.android.server.testutils.any -import com.android.server.testutils.mock -import com.android.server.testutils.whenever -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.Mockito.anyInt -import org.mockito.Mockito.doNothing -import org.mockito.Mockito.times -import org.mockito.Mockito.verify - -@RunWith(AndroidJUnit4::class) -class ChooserListAdapterTest { - private val packageManager = mock<PackageManager> { - whenever(resolveActivity(any(), anyInt())).thenReturn(mock()) - } - private val context = InstrumentationRegistry.getInstrumentation().getContext() - private val resolverListController = mock<ResolverListController>() - private val chooserListCommunicator = mock<ChooserListAdapter.ChooserListCommunicator> { - whenever(maxRankedTargets).thenReturn(0) - } - private val selectableTargetInfoCommunicator = - mock<SelectableTargetInfoCommunicator> { - whenever(targetIntent).thenReturn(mock()) - } - private val chooserActivityLogger = mock<ChooserActivityLogger>() - - private val testSubject = ChooserListAdapter( - context, - emptyList(), - emptyArray(), - emptyList(), - false, - resolverListController, - chooserListCommunicator, - selectableTargetInfoCommunicator, - packageManager, - chooserActivityLogger - ) - - @Test - fun testDirectShareTargetLoadingIconIsStarted() { - val view = createView() - val viewHolder = ResolverListAdapter.ViewHolder(view) - view.tag = viewHolder - val targetInfo = createSelectableTargetInfo() - val iconTask = mock<ChooserListAdapter.LoadDirectShareIconTask> { - doNothing().whenever(this).loadIcon() - } - testSubject.setTestLoadDirectShareTaskProvider( - mock { - whenever(get()).thenReturn(iconTask) - } - ) - testSubject.testViewBind(view, targetInfo, 0) - - verify(iconTask, times(1)).loadIcon() - verify(iconTask, times(1)).setViewHolder(viewHolder) - } - - @Test - fun testOnlyOneTaskPerTarget() { - val view = createView() - val viewHolderOne = ResolverListAdapter.ViewHolder(view) - view.tag = viewHolderOne - val targetInfo = createSelectableTargetInfo() - val iconTaskOne = mock<ChooserListAdapter.LoadDirectShareIconTask> { - doNothing().whenever(this).loadIcon() - } - val testTaskProvider = mock<ChooserListAdapter.LoadDirectShareIconTaskProvider> { - whenever(get()).thenReturn(iconTaskOne) - } - testSubject.setTestLoadDirectShareTaskProvider( - testTaskProvider - ) - testSubject.testViewBind(view, targetInfo, 0) - - val viewHolderTwo = ResolverListAdapter.ViewHolder(view) - view.tag = viewHolderTwo - whenever(testTaskProvider.get()).thenReturn(mock()) - - testSubject.testViewBind(view, targetInfo, 0) - - verify(iconTaskOne, times(1)).loadIcon() - verify(iconTaskOne, times(1)).setViewHolder(viewHolderOne) - verify(iconTaskOne, times(1)).setViewHolder(viewHolderTwo) - verify(testTaskProvider, times(1)).get() - } - - 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 - } -} |