Merge "Load direct-share icons asynchronously" into tm-qpr-dev

core/java/com/android/internal/app/ChooserListAdapter.java

@@ -43,6 +43,7 @@ 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;
@@ -86,7 +87,7 @@ public class ChooserListAdapter extends ResolverListAdapter {
     private final ChooserActivityLogger mChooserActivityLogger;
 
     private int mNumShortcutResults = 0;
-    private Map<DisplayResolveInfo, LoadIconTask> mIconLoaders = new HashMap<>();
+    private final Map<TargetInfo, AsyncTask> mIconLoaders = new HashMap<>();
     private boolean mApplySharingAppLimits;
 
     // Reserve spots for incoming direct share targets by adding placeholders
@@ -104,6 +105,8 @@ 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
@@ -240,7 +243,6 @@ public class ChooserListAdapter extends ResolverListAdapter {
         mListViewDataChanged = false;
     }
 
-
     private void createPlaceHolders() {
         mNumShortcutResults = 0;
         mServiceTargets.clear();
@@ -265,31 +267,25 @@ public class ChooserListAdapter extends ResolverListAdapter {
             return;
         }
 
-        if (!(info instanceof DisplayResolveInfo)) {
+        if (info instanceof DisplayResolveInfo) {
-            holder.bindLabel(info.getDisplayLabel(), info.getExtendedInfo(), alwaysShowSubLabel());
-            holder.bindIcon(info);
-
-            if (info instanceof SelectableTargetInfo) {
-                // direct share targets should append the application name for a better readout
-                DisplayResolveInfo rInfo = ((SelectableTargetInfo) info).getDisplayResolveInfo();
-                CharSequence appName = rInfo != null ? rInfo.getDisplayLabel() : "";
-                CharSequence extendedInfo = info.getExtendedInfo();
-                String contentDescription = String.join(" ", info.getDisplayLabel(),
-                        extendedInfo != null ? extendedInfo : "", appName);
-                holder.updateContentDescription(contentDescription);
-            }
-        } else {
             DisplayResolveInfo dri = (DisplayResolveInfo) info;
             holder.bindLabel(dri.getDisplayLabel(), dri.getExtendedInfo(), alwaysShowSubLabel());
-            LoadIconTask task = mIconLoaders.get(dri);
+            startDisplayResolveInfoIconLoading(holder, dri);
-            if (task == null) {
-                task = new LoadIconTask(dri, holder);
-                mIconLoaders.put(dri, task);
-                task.execute();
         } else {
-                // The holder was potentially changed as the underlying items were
+            holder.bindLabel(info.getDisplayLabel(), info.getExtendedInfo(), alwaysShowSubLabel());
-                // reshuffled, so reset the target holder
+
-                task.setViewHolder(holder);
+            if (info instanceof SelectableTargetInfo) {
+                SelectableTargetInfo selectableInfo = (SelectableTargetInfo) info;
+                // direct share targets should append the application name for a better readout
+                DisplayResolveInfo rInfo = selectableInfo.getDisplayResolveInfo();
+                CharSequence appName = rInfo != null ? rInfo.getDisplayLabel() : "";
+                CharSequence extendedInfo = selectableInfo.getExtendedInfo();
+                String contentDescription = String.join(" ", selectableInfo.getDisplayLabel(),
+                        extendedInfo != null ? extendedInfo : "", appName);
+                holder.updateContentDescription(contentDescription);
+                startSelectableTargetInfoIconLoading(holder, selectableInfo);
+            } else {
+                holder.bindIcon(info);
             }
         }
 
@@ -330,6 +326,32 @@ 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
@@ -739,11 +761,25 @@ 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}.
      */
-    interface ChooserListCommunicator extends ResolverListCommunicator {
+    @VisibleForTesting
+    public interface ChooserListCommunicator extends ResolverListCommunicator {
 
         int getMaxRankedTargets();
 
@@ -751,4 +787,59 @@ 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();
+    }
 }

core/java/com/android/internal/app/ResolverListAdapter.java

@@ -834,7 +834,11 @@ public class ResolverListAdapter extends BaseAdapter {
         void onHandlePackagesChanged(ResolverListAdapter listAdapter);
     }
 
-    static class ViewHolder {
+    /**
+     * A view holder.
+     */
+    @VisibleForTesting
+    public static class ViewHolder {
         public View itemView;
         public Drawable defaultItemViewBackground;
 
@@ -842,7 +846,8 @@ public class ResolverListAdapter extends BaseAdapter {
         public TextView text2;
         public ImageView icon;
 
-        ViewHolder(View view) {
+        @VisibleForTesting
+        public ViewHolder(View view) {
             itemView = view;
             defaultItemViewBackground = view.getBackground();
             text = (TextView) view.findViewById(com.android.internal.R.id.text1);

core/java/com/android/internal/app/chooser/SelectableTargetInfo.java

@@ -37,6 +37,7 @@ 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;
@@ -59,8 +60,11 @@ 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;
@@ -78,6 +82,7 @@ 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();
@@ -92,8 +97,6 @@ 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;
@@ -118,7 +121,10 @@ public final class SelectableTargetInfo implements ChooserTargetInfo {
         mChooserTarget = other.mChooserTarget;
         mBadgeIcon = other.mBadgeIcon;
         mBadgeContentDescription = other.mBadgeContentDescription;
+        synchronized (other) {
+            mShortcutInfo = other.mShortcutInfo;
             mDisplayIcon = other.mDisplayIcon;
+        }
         mFillInIntent = fillInIntent;
         mFillInFlags = flags;
         mModifiedScore = other.mModifiedScore;
@@ -141,6 +147,25 @@ 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;
@@ -270,7 +295,7 @@ public final class SelectableTargetInfo implements ChooserTargetInfo {
     }
 
     @Override
-    public Drawable getDisplayIcon(Context context) {
+    public synchronized Drawable getDisplayIcon(Context context) {
         return mDisplayIcon;
     }
 

core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt

@@ -0,0 +1,155 @@
+/*
+ * 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
+    }
+}