Load direct-share icons asynchronously

Move direct-share icons loading off the main thread in the same way we're loding A-Z icons. Test: Manual testing with rotatin and screen locking, artificially delaying image loading, debug logging for potential event loops. Test: atest ChooserListAdapterTest Fix: 215699869 Change-Id: I97e3383a74bb4ef795099fe55cae9fc5aed2b790 Change-Id: I3b110447d4cf2ab94e663c2684cd2cce4968d4ca
2022-09-09 16:27:53 -07:00
parent a04b35b697
commit a54f0d13a6
4 changed files with 286 additions and 11 deletions
--- a/core/java/com/android/internal/app/ChooserListAdapter.java
+++ b/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,6 +87,7 @@ public class ChooserListAdapter extends ResolverListAdapter {
    private final ChooserActivityLogger mChooserActivityLogger;

    private int mNumShortcutResults = 0;
+    private final Map<SelectableTargetInfo, LoadDirectShareIconTask> mIconLoaders = new HashMap<>();
    private boolean mApplySharingAppLimits;

    // Reserve spots for incoming direct share targets by adding placeholders
@@ -239,7 +241,6 @@ public class ChooserListAdapter extends ResolverListAdapter {
        mListViewDataChanged = false;
    }

-
    private void createPlaceHolders() {
        mNumShortcutResults = 0;
        mServiceTargets.clear();
@@ -268,12 +269,16 @@ public class ChooserListAdapter extends ResolverListAdapter {
        holder.bindIcon(info);
        if (info instanceof SelectableTargetInfo) {
            // 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 instanceof DisplayResolveInfo) {
            DisplayResolveInfo dri = (DisplayResolveInfo) info;
            if (!dri.hasDisplayIcon()) {
@@ -318,6 +323,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<Void, Void, List<DisplayResolveInfo>>() {
            @Override
@@ -332,7 +351,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);
@@ -341,7 +360,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);
                    }
@@ -731,7 +750,8 @@ public class ChooserListAdapter extends ResolverListAdapter {
     * Necessary methods to communicate between {@link ChooserListAdapter}
     * and {@link ChooserActivity}.
     */
-    interface ChooserListCommunicator extends ResolverListCommunicator {
+    @VisibleForTesting
+    public interface ChooserListCommunicator extends ResolverListCommunicator {

        int getMaxRankedTargets();

@@ -739,4 +759,35 @@ public class ChooserListAdapter extends ResolverListAdapter {

        boolean isSendAction(Intent targetIntent);
    }
+
+    /**
+     * Loads direct share targets icons.
+     */
+    @VisibleForTesting
+    public class LoadDirectShareIconTask extends AsyncTask<Void, Void, Boolean> {
+        private final SelectableTargetInfo mTargetInfo;
+
+        private LoadDirectShareIconTask(SelectableTargetInfo targetInfo) {
+            mTargetInfo = targetInfo;
+        }
+
+        @Override
+        protected Boolean doInBackground(Void... voids) {
+            return mTargetInfo.loadIcon();
+        }
+
+        @Override
+        protected void onPostExecute(Boolean isLoaded) {
+            if (isLoaded) {
+                notifyDataSetChanged();
+            }
+        }
+
+        /**
+         * An alias for execute to use with unit tests.
+         */
+        public void loadIcon() {
+            execute();
+        }
+    }
 }
--- a/core/java/com/android/internal/app/ResolverListAdapter.java
+++ b/core/java/com/android/internal/app/ResolverListAdapter.java
@@ -870,7 +870,12 @@ public class ResolverListAdapter extends BaseAdapter {
        void onHandlePackagesChanged(ResolverListAdapter listAdapter);
    }

-    static class ViewHolder {
+    /**
+     * A view holder keeps a reference to a list view and provides functionality for managing its
+     * state.
+     */
+    @VisibleForTesting
+    public static class ViewHolder {
        public View itemView;
        public Drawable defaultItemViewBackground;

@@ -878,7 +883,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);
--- a/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java
+++ b/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;
-        mDisplayIcon = other.mDisplayIcon;
+        synchronized (other) {
+            mShortcutInfo = other.mShortcutInfo;
+            mDisplayIcon = other.mDisplayIcon;
+        }
        mFillInIntent = fillInIntent;
        mFillInFlags = flags;
        mModifiedScore = other.mModifiedScore;
@@ -141,6 +147,27 @@ public final class SelectableTargetInfo implements ChooserTargetInfo {
        return mSourceInfo;
    }

+    /**
+     * Load display icon, if needed.
+     */
+    public boolean loadIcon() {
+        ShortcutInfo shortcutInfo;
+        Drawable icon;
+        synchronized (this) {
+            shortcutInfo = mShortcutInfo;
+            icon = mDisplayIcon;
+        }
+        boolean shouldLoadIcon = icon == null && shortcutInfo != null;
+        if (shouldLoadIcon) {
+            icon = getChooserTargetIconDrawable(mChooserTarget, shortcutInfo);
+            synchronized (this) {
+                mDisplayIcon = icon;
+                mShortcutInfo = null;
+            }
+        }
+        return shouldLoadIcon;
+    }
+
    private Drawable getChooserTargetIconDrawable(ChooserTarget target,
            @Nullable ShortcutInfo shortcutInfo) {
        Drawable directShareIcon = null;
@@ -270,10 +297,17 @@ public final class SelectableTargetInfo implements ChooserTargetInfo {
    }

    @Override
-    public Drawable getDisplayIcon(Context context) {
+    public synchronized Drawable getDisplayIcon(Context context) {
        return mDisplayIcon;
    }

+    /**
+     * @return true if display icon is available
+     */
+    public synchronized boolean hasDisplayIcon() {
+        return mDisplayIcon != null;
+    }
+
    public ChooserTarget getChooserTarget() {
        return mChooserTarget;
    }
--- a/core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt
+++ b/core/tests/coretests/src/com/android/internal/app/ChooserListAdapterTest.kt
@@ -0,0 +1,184 @@
+/*
+ * 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.Context
+import android.content.Intent
+import android.content.pm.PackageManager
+import android.content.pm.ResolveInfo
+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.ChooserListAdapter.LoadDirectShareIconTask
+import com.android.internal.app.chooser.SelectableTargetInfo
+import com.android.internal.app.chooser.SelectableTargetInfo.SelectableTargetInfoCommunicator
+import com.android.internal.app.chooser.TargetInfo
+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.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 fun createChooserListAdapter(
+        taskProvider: (SelectableTargetInfo?) -> LoadDirectShareIconTask
+    ) =
+        ChooserListAdapterOverride(
+            context,
+            emptyList(),
+            emptyArray(),
+            emptyList(),
+            false,
+            resolverListController,
+            chooserListCommunicator,
+            selectableTargetInfoCommunicator,
+            packageManager,
+            chooserActivityLogger,
+            taskProvider
+        )
+
+    @Test
+    fun testDirectShareTargetLoadingIconIsStarted() {
+        val view = createView()
+        val viewHolder = ResolverListAdapter.ViewHolder(view)
+        view.tag = viewHolder
+        val targetInfo = createSelectableTargetInfo()
+        val iconTask = mock<LoadDirectShareIconTask>()
+        val testSubject = createChooserListAdapter { iconTask }
+        testSubject.testViewBind(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<LoadDirectShareIconTask>()
+        val testTaskProvider = mock<() -> LoadDirectShareIconTask> {
+            whenever(invoke()).thenReturn(iconTaskOne)
+        }
+        val testSubject = createChooserListAdapter { testTaskProvider.invoke() }
+        testSubject.testViewBind(view, targetInfo, 0)
+
+        val viewHolderTwo = ResolverListAdapter.ViewHolder(view)
+        view.tag = viewHolderTwo
+        whenever(testTaskProvider()).thenReturn(mock())
+
+        testSubject.testViewBind(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
+    }
+}
+
+private class ChooserListAdapterOverride(
+    context: Context?,
+    payloadIntents: List<Intent>?,
+    initialIntents: Array<out Intent>?,
+    rList: List<ResolveInfo>?,
+    filterLastUsed: Boolean,
+    resolverListController: ResolverListController?,
+    chooserListCommunicator: ChooserListCommunicator?,
+    selectableTargetInfoCommunicator: SelectableTargetInfoCommunicator?,
+    packageManager: PackageManager?,
+    chooserActivityLogger: ChooserActivityLogger?,
+    private val taskProvider: (SelectableTargetInfo?) -> LoadDirectShareIconTask
+) : ChooserListAdapter(
+    context,
+    payloadIntents,
+    initialIntents,
+    rList,
+    filterLastUsed,
+    resolverListController,
+    chooserListCommunicator,
+    selectableTargetInfoCommunicator,
+    packageManager,
+    chooserActivityLogger
+) {
+    override fun createLoadDirectShareIconTask(
+        info: SelectableTargetInfo?
+    ): LoadDirectShareIconTask =
+        taskProvider.invoke(info)
+
+    fun testViewBind(view: View?, info: TargetInfo?, position: Int) {
+        onBindView(view, info, position)
+    }
+}