Merge "Cache shortcut icons" into main

8 files changed, 170 insertions, 20 deletions

diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java
index 8b848e55..ff0c40d7 100644
--- a/java/src/com/android/intentresolver/ChooserListAdapter.java
+++ b/java/src/com/android/intentresolver/ChooserListAdapter.java
@@ -479,17 +479,23 @@ public class ChooserListAdapter extends ResolverListAdapter {
 
     private void loadDirectShareIcon(SelectableTargetInfo info) {
         if (mRequestedIcons.add(info)) {
-            mTargetDataLoader.loadDirectShareIcon(
+            Drawable icon = mTargetDataLoader.getOrLoadDirectShareIcon(
                     info,
                     getUserHandle(),
-                    (drawable) -> onDirectShareIconLoaded(info, drawable));
+                    (drawable) -> onDirectShareIconLoaded(info, drawable, true));
+            if (icon != null) {
+                onDirectShareIconLoaded(info, icon, false);
+            }
         }
     }
 
-    private void onDirectShareIconLoaded(SelectableTargetInfo mTargetInfo, Drawable icon) {
+    private void onDirectShareIconLoaded(
+            SelectableTargetInfo mTargetInfo, @Nullable Drawable icon, boolean notify) {
         if (icon != null && !mTargetInfo.hasDisplayIcon()) {
             mTargetInfo.getDisplayIconHolder().setDisplayIcon(icon);
-            notifyDataSetChanged();
+            if (notify) {
+                notifyDataSetChanged();
+            }
         }
     }
 
diff --git a/java/src/com/android/intentresolver/icons/CachingTargetDataLoader.kt b/java/src/com/android/intentresolver/icons/CachingTargetDataLoader.kt
index b3054231..8474b4c3 100644
--- a/java/src/com/android/intentresolver/icons/CachingTargetDataLoader.kt
+++ b/java/src/com/android/intentresolver/icons/CachingTargetDataLoader.kt
@@ -28,7 +28,7 @@ import javax.inject.Qualifier
 
 @Qualifier @MustBeDocumented @Retention(AnnotationRetention.BINARY) annotation class Caching
 
-private typealias IconCache = LruCache<ComponentName, Drawable>
+private typealias IconCache = LruCache<String, Drawable>
 
 class CachingTargetDataLoader(
     private val targetDataLoader: TargetDataLoader,
@@ -49,18 +49,27 @@ class CachingTargetDataLoader(
             }
     }
 
-    override fun loadDirectShareIcon(
+    override fun getOrLoadDirectShareIcon(
         info: SelectableTargetInfo,
         userHandle: UserHandle,
         callback: Consumer<Drawable>
-    ) = targetDataLoader.loadDirectShareIcon(info, userHandle, callback)
+    ): Drawable? {
+        val cacheKey = info.toCacheKey()
+        return cacheKey?.let { getCachedAppIcon(it, userHandle) }
+            ?: targetDataLoader.getOrLoadDirectShareIcon(info, userHandle) { drawable ->
+                if (cacheKey != null) {
+                    getProfileIconCache(userHandle).put(cacheKey, drawable)
+                }
+                callback.accept(drawable)
+            }
+    }
 
     override fun loadLabel(info: DisplayResolveInfo, callback: Consumer<LabelInfo>) =
         targetDataLoader.loadLabel(info, callback)
 
     override fun getOrLoadLabel(info: DisplayResolveInfo) = targetDataLoader.getOrLoadLabel(info)
 
-    private fun getCachedAppIcon(component: ComponentName, userHandle: UserHandle): Drawable? =
+    private fun getCachedAppIcon(component: String, userHandle: UserHandle): Drawable? =
         getProfileIconCache(userHandle)[component]
 
     private fun getProfileIconCache(userHandle: UserHandle): IconCache =
@@ -70,7 +79,20 @@ class CachingTargetDataLoader(
 
     private fun DisplayResolveInfo.toCacheKey() =
         ComponentName(
-            resolveInfo.activityInfo.packageName,
-            resolveInfo.activityInfo.name,
-        )
+                resolveInfo.activityInfo.packageName,
+                resolveInfo.activityInfo.name,
+            )
+            .flattenToString()
+
+    private fun SelectableTargetInfo.toCacheKey(): String? =
+        if (chooserTargetIcon != null) {
+            // do not cache icons for caller-provided targets
+            null
+        } else {
+            buildString {
+                append(chooserTargetComponentName?.flattenToString() ?: "")
+                append("|")
+                append(directShareShortcutInfo?.id ?: "")
+            }
+        }
 }
diff --git a/java/src/com/android/intentresolver/icons/DefaultTargetDataLoader.kt b/java/src/com/android/intentresolver/icons/DefaultTargetDataLoader.kt
index 1a724d73..e7392f58 100644
--- a/java/src/com/android/intentresolver/icons/DefaultTargetDataLoader.kt
+++ b/java/src/com/android/intentresolver/icons/DefaultTargetDataLoader.kt
@@ -77,11 +77,11 @@ class DefaultTargetDataLoader(
         return null
     }
 
-    override fun loadDirectShareIcon(
+    override fun getOrLoadDirectShareIcon(
         info: SelectableTargetInfo,
         userHandle: UserHandle,
         callback: Consumer<Drawable>,
-    ) {
+    ): Drawable? {
         val taskId = nextTaskId.getAndIncrement()
         LoadDirectShareIconTask(
                 context.createContextAsUser(userHandle, 0),
@@ -93,6 +93,7 @@ class DefaultTargetDataLoader(
             }
             .also { addTask(taskId, it) }
             .executeOnExecutor(executor)
+        return null
     }
 
     override fun loadLabel(info: DisplayResolveInfo, callback: Consumer<LabelInfo>) {
diff --git a/java/src/com/android/intentresolver/icons/LoadDirectShareIconTask.java b/java/src/com/android/intentresolver/icons/LoadDirectShareIconTask.java
index 0f135d63..e2c0362d 100644
--- a/java/src/com/android/intentresolver/icons/LoadDirectShareIconTask.java
+++ b/java/src/com/android/intentresolver/icons/LoadDirectShareIconTask.java
@@ -57,7 +57,7 @@ class LoadDirectShareIconTask extends BaseLoadIconTask {
 
     @Override
     protected Drawable doInBackground(Void... voids) {
-        Drawable drawable;
+        Drawable drawable = null;
         Trace.beginSection("shortcut-icon");
         try {
             final Icon icon = mTargetInfo.getChooserTargetIcon();
@@ -70,6 +70,8 @@ class LoadDirectShareIconTask extends BaseLoadIconTask {
             } else {
                 Log.e(TAG, "Failed to load shortcut icon for "
                         + mTargetInfo.getChooserTargetComponentName() + "; no access");
+            }
+            if (drawable == null) {
                 drawable = loadIconPlaceholder();
             }
         } catch (Exception e) {
@@ -86,6 +88,7 @@ class LoadDirectShareIconTask extends BaseLoadIconTask {
     }
 
     @WorkerThread
+    @Nullable
     private Drawable getChooserTargetIconDrawable(
             Context context,
             @Nullable Icon icon,
diff --git a/java/src/com/android/intentresolver/icons/TargetDataLoader.kt b/java/src/com/android/intentresolver/icons/TargetDataLoader.kt
index 7789df44..935b527a 100644
--- a/java/src/com/android/intentresolver/icons/TargetDataLoader.kt
+++ b/java/src/com/android/intentresolver/icons/TargetDataLoader.kt
@@ -32,11 +32,11 @@ abstract class TargetDataLoader {
     ): Drawable?
 
     /** Load a shortcut icon */
-    abstract fun loadDirectShareIcon(
+    abstract fun getOrLoadDirectShareIcon(
         info: SelectableTargetInfo,
         userHandle: UserHandle,
         callback: Consumer<Drawable>,
-    )
+    ): Drawable?
 
     /** Load target label */
     abstract fun loadLabel(info: DisplayResolveInfo, callback: Consumer<LabelInfo>)
diff --git a/tests/activity/src/com/android/intentresolver/ResolverWrapperActivity.java b/tests/activity/src/com/android/intentresolver/ResolverWrapperActivity.java
index b46d8bc3..22633085 100644
--- a/tests/activity/src/com/android/intentresolver/ResolverWrapperActivity.java
+++ b/tests/activity/src/com/android/intentresolver/ResolverWrapperActivity.java
@@ -180,11 +180,12 @@ public class ResolverWrapperActivity extends ResolverActivity {
         }
 
         @Override
-        public void loadDirectShareIcon(
+        @Nullable
+        public Drawable getOrLoadDirectShareIcon(
                 @NonNull SelectableTargetInfo info,
                 @NonNull UserHandle userHandle,
                 @NonNull Consumer<Drawable> callback) {
-            mTargetDataLoader.loadDirectShareIcon(info, userHandle, callback);
+            return mTargetDataLoader.getOrLoadDirectShareIcon(info, userHandle, callback);
         }
 
         @Override
diff --git a/tests/unit/src/com/android/intentresolver/ChooserListAdapterTest.kt b/tests/unit/src/com/android/intentresolver/ChooserListAdapterTest.kt
index 5ac4f2b0..bad3b18c 100644
--- a/tests/unit/src/com/android/intentresolver/ChooserListAdapterTest.kt
+++ b/tests/unit/src/com/android/intentresolver/ChooserListAdapterTest.kt
@@ -101,7 +101,7 @@ class ChooserListAdapterTest {
         val targetInfo = createSelectableTargetInfo()
         testSubject.onBindView(view, targetInfo, 0)
 
-        verify(mTargetDataLoader, times(1)).loadDirectShareIcon(any(), any(), any())
+        verify(mTargetDataLoader, times(1)).getOrLoadDirectShareIcon(any(), any(), any())
     }
 
     @Test
@@ -117,7 +117,7 @@ class ChooserListAdapterTest {
 
         testSubject.onBindView(view, targetInfo, 0)
 
-        verify(mTargetDataLoader, times(1)).loadDirectShareIcon(any(), any(), any())
+        verify(mTargetDataLoader, times(1)).getOrLoadDirectShareIcon(any(), any(), any())
     }
 
     @Test
diff --git a/tests/unit/src/com/android/intentresolver/icons/CachingTargetDataLoaderTest.kt b/tests/unit/src/com/android/intentresolver/icons/CachingTargetDataLoaderTest.kt
new file mode 100644
index 00000000..a36b512b
--- /dev/null
+++ b/tests/unit/src/com/android/intentresolver/icons/CachingTargetDataLoaderTest.kt
@@ -0,0 +1,117 @@
+/*
+ * Copyright 2024 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.icons
+
+import android.content.ComponentName
+import android.content.Context
+import android.content.Intent
+import android.content.pm.ShortcutInfo
+import android.graphics.Bitmap
+import android.graphics.drawable.BitmapDrawable
+import android.graphics.drawable.Drawable
+import android.graphics.drawable.Icon
+import android.os.UserHandle
+import com.android.intentresolver.chooser.SelectableTargetInfo
+import java.util.function.Consumer
+import org.junit.Test
+import org.mockito.kotlin.any
+import org.mockito.kotlin.doAnswer
+import org.mockito.kotlin.doReturn
+import org.mockito.kotlin.eq
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.verify
+import org.mockito.kotlin.whenever
+
+class CachingTargetDataLoaderTest {
+    private val userHandle = UserHandle.of(1)
+
+    @Test
+    fun doNotCacheCallerProvidedShortcuts() {
+        val callerTarget =
+            SelectableTargetInfo.newSelectableTargetInfo(
+                /* sourceInfo = */ null,
+                /* backupResolveInfo = */ null,
+                /* resolvedIntent = */ Intent(),
+                /* chooserTargetComponentName =*/ ComponentName("package", "Activity"),
+                "chooserTargetUninitializedTitle",
+                /* chooserTargetIcon =*/ Icon.createWithContentUri("content://package/icon.png"),
+                /* chooserTargetIntentExtras =*/ null,
+                /* modifiedScore =*/ 1f,
+                /* shortcutInfo = */ null,
+                /* appTarget = */ null,
+                /* referrerFillInIntent = */ Intent(),
+            ) as SelectableTargetInfo
+
+        val targetDataLoader =
+            mock<TargetDataLoader> {
+                on { getOrLoadDirectShareIcon(eq(callerTarget), eq(userHandle), any()) } doReturn
+                    null
+            }
+        val testSubject = CachingTargetDataLoader(targetDataLoader)
+        val callback = Consumer<Drawable> {}
+
+        testSubject.getOrLoadDirectShareIcon(callerTarget, userHandle, callback)
+        testSubject.getOrLoadDirectShareIcon(callerTarget, userHandle, callback)
+
+        verify(targetDataLoader) {
+            2 * { getOrLoadDirectShareIcon(eq(callerTarget), eq(userHandle), any()) }
+        }
+    }
+
+    @Test
+    fun serviceShortcutsAreCached() {
+        val context =
+            mock<Context> {
+                on { userId } doReturn 1
+                on { packageName } doReturn "package"
+            }
+        val targetInfo =
+            SelectableTargetInfo.newSelectableTargetInfo(
+                /* sourceInfo = */ null,
+                /* backupResolveInfo = */ null,
+                /* resolvedIntent = */ Intent(),
+                /* chooserTargetComponentName =*/ ComponentName("package", "Activity"),
+                "chooserTargetUninitializedTitle",
+                /* chooserTargetIcon =*/ null,
+                /* chooserTargetIntentExtras =*/ null,
+                /* modifiedScore =*/ 1f,
+                /* shortcutInfo = */ ShortcutInfo.Builder(context, "1").build(),
+                /* appTarget = */ null,
+                /* referrerFillInIntent = */ Intent(),
+            ) as SelectableTargetInfo
+
+        val targetDataLoader = mock<TargetDataLoader>()
+        doAnswer {
+                val callback = it.arguments[2] as Consumer<Drawable>
+                callback.accept(BitmapDrawable(createBitmap()))
+                null
+            }
+            .whenever(targetDataLoader)
+            .getOrLoadDirectShareIcon(eq(targetInfo), eq(userHandle), any())
+        val testSubject = CachingTargetDataLoader(targetDataLoader)
+        val callback = Consumer<Drawable> {}
+
+        testSubject.getOrLoadDirectShareIcon(targetInfo, userHandle, callback)
+        testSubject.getOrLoadDirectShareIcon(targetInfo, userHandle, callback)
+
+        verify(targetDataLoader) {
+            1 * { getOrLoadDirectShareIcon(eq(targetInfo), eq(userHandle), any()) }
+        }
+    }
+}
+
+private fun createBitmap() = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)