Prevent one app from blocking all download threads + fix getIconResource
Since we can't cancel tasks without requesting developers to handle it,
this CL groups loading tasks for the same application into the same thread
pool, and keeps a cache of pools with up to
image_fetcher_thread_pools_max_count entries.
Fix UriUtils.getIconResource that was incorrectly equating the authority
of a content provider uri with the package name.
Fixes: 140265697
Test: manual
Change-Id: I8378ff0efdb06024c04c11187cecc61b47007049
diff --git a/car-apps-common/res/values/integers.xml b/car-apps-common/res/values/integers.xml
index 4f01c36..bd93714 100644
--- a/car-apps-common/res/values/integers.xml
+++ b/car-apps-common/res/values/integers.xml
@@ -26,8 +26,12 @@
<!-- The amount of time it takes for a new image in a CrossfadeImageView to fade in. -->
<integer name="crossfade_image_view_fade_in_duration">250</integer>
- <!-- The number of threads used to fetch (local) images. -->
- <integer name="image_fetcher_thread_pool_size">5</integer>
+ <!-- The maximum number of thread pools used to fetch images. Applications don't share
+ pools to prevent one bad app from starving others. -->
+ <integer name="image_fetcher_thread_pools_max_count">5</integer>
+
+ <!-- The number of threads in each pool used to fetch images. -->
+ <integer name="image_fetcher_thread_pool_size">3</integer>
<!-- The amount of memory (in megabytes) LocalImageFetcher allocates to caching bitmaps
(and drawables) in memory. -->
diff --git a/car-apps-common/src/com/android/car/apps/common/UriUtils.java b/car-apps-common/src/com/android/car/apps/common/UriUtils.java
index 9af74b2..199635b 100644
--- a/car-apps-common/src/com/android/car/apps/common/UriUtils.java
+++ b/car-apps-common/src/com/android/car/apps/common/UriUtils.java
@@ -15,10 +15,14 @@
*/
package com.android.car.apps.common;
+import static android.content.pm.PackageManager.MATCH_ALL;
+
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent.ShortcutIconResource;
+import android.content.pm.PackageManager;
import android.content.pm.PackageManager.NameNotFoundException;
+import android.content.pm.ProviderInfo;
import android.content.res.Resources;
import android.graphics.drawable.Drawable;
import android.net.Uri;
@@ -177,6 +181,18 @@
}
/**
+ * Finds the packageName of the application to which the content authority of the given uri
+ * belongs to.
+ */
+ @Nullable
+ public static String getPackageName(Context context, Uri uri) {
+ PackageManager pm = context.getPackageManager();
+ ProviderInfo info = pm.resolveContentProvider(uri.getAuthority(), MATCH_ALL);
+ // Info can be null when the app doesn't define a provider.
+ return (info != null) ? info.packageName : uri.getAuthority();
+ }
+
+ /**
* Returns {@code true} if the URI refers to a content URI which can be opened via
* {@link ContentResolver#openInputStream(Uri)}.
*/
@@ -195,21 +211,24 @@
/**
* Creates a shortcut icon resource object from an Android resource URI.
*/
- public static ShortcutIconResource getIconResource(Uri uri) {
+ public static ShortcutIconResource getIconResource(Context context, Uri uri) {
if(isAndroidResourceUri(uri)) {
ShortcutIconResource iconResource = new ShortcutIconResource();
- iconResource.packageName = uri.getAuthority();
- // Trim off the scheme + 3 extra for "://", then replace the first "/" with a ":"
- iconResource.resourceName = uri.toString().substring(
- ContentResolver.SCHEME_ANDROID_RESOURCE.length() + SCHEME_DELIMITER.length())
- .replaceFirst(URI_PATH_DELIMITER, URI_PACKAGE_DELIMITER);
+ iconResource.packageName = getPackageName(context, uri);
+ // Trim off the scheme + 3 extra for "://" + authority, then replace the first "/"
+ // with a ":" and add to packageName.
+ int resStart = ContentResolver.SCHEME_ANDROID_RESOURCE.length()
+ + SCHEME_DELIMITER.length() + uri.getAuthority().length();
+ iconResource.resourceName = iconResource.packageName
+ + uri.toString().substring(resStart)
+ .replaceFirst(URI_PATH_DELIMITER, URI_PACKAGE_DELIMITER);
return iconResource;
} else if(isShortcutIconResourceUri(uri)) {
ShortcutIconResource iconResource = new ShortcutIconResource();
- iconResource.packageName = uri.getAuthority();
+ iconResource.packageName = getPackageName(context, uri);
iconResource.resourceName = uri.toString().substring(
SCHEME_SHORTCUT_ICON_RESOURCE.length() + SCHEME_DELIMITER.length()
- + iconResource.packageName.length() + URI_PATH_DELIMITER.length())
+ + uri.getAuthority().length() + URI_PATH_DELIMITER.length())
.replaceFirst(URI_PATH_DELIMITER, URI_PACKAGE_DELIMITER);
return iconResource;
} else {
diff --git a/car-apps-common/src/com/android/car/apps/common/imaging/LocalImageFetcher.java b/car-apps-common/src/com/android/car/apps/common/imaging/LocalImageFetcher.java
index 9882216..185ec67 100644
--- a/car-apps-common/src/com/android/car/apps/common/imaging/LocalImageFetcher.java
+++ b/car-apps-common/src/com/android/car/apps/common/imaging/LocalImageFetcher.java
@@ -77,7 +77,9 @@
return sInstance;
}
- private final Executor mThreadPool;
+ private final int mPoolSize;
+
+ private final LruCache<String, Executor> mThreadPools;
private final Map<ImageKey, HashSet<BiConsumer<ImageKey, Drawable>>> mConsumers =
new HashMap<>(20);
@@ -90,8 +92,9 @@
@UiThread
private LocalImageFetcher(Context context) {
Resources res = context.getResources();
- int poolSize = res.getInteger(R.integer.image_fetcher_thread_pool_size);
- mThreadPool = Executors.newFixedThreadPool(poolSize);
+ int maxPools = res.getInteger(R.integer.image_fetcher_thread_pools_max_count);
+ mPoolSize = res.getInteger(R.integer.image_fetcher_thread_pool_size);
+ mThreadPools = new LruCache<>(maxPools);
int cacheSizeMB = res.getInteger(R.integer.bitmap_memory_cache_max_size_mb);
int drawableDefaultWeightKB = res.getInteger(R.integer.drawable_default_weight_kb);
@@ -111,6 +114,15 @@
mFlagRemoteImages = CommonFlags.getInstance(context).shouldFlagImproperImageRefs();
}
+ private Executor getThreadPool(String packageName) {
+ Executor result = mThreadPools.get(packageName);
+ if (result == null) {
+ result = Executors.newFixedThreadPool(mPoolSize);
+ mThreadPools.put(packageName, result);
+ }
+ return result;
+ }
+
/** Fetches an image. The resulting drawable may be null. */
@UiThread
public void getImage(Context context, ImageKey key, BiConsumer<ImageKey, Drawable> consumer) {
@@ -133,9 +145,14 @@
consumers.add(consumer);
if (task == null) {
- task = new ImageLoadingTask(context, key, mFlagRemoteImages);
- mTasks.put(key, task);
- task.executeOnExecutor(mThreadPool);
+ String packageName = UriUtils.getPackageName(context, key.mImageUri);
+ if (packageName != null) {
+ task = new ImageLoadingTask(context, key, mFlagRemoteImages);
+ mTasks.put(key, task);
+ task.executeOnExecutor(getThreadPool(packageName));
+ } else {
+ Log.e(TAG, "No package for " + key.mImageUri);
+ }
}
}
@@ -237,7 +254,8 @@
if (UriUtils.isAndroidResourceUri(imageUri)) {
// ImageDecoder doesn't support all resources via the content provider...
- return UriUtils.getDrawable(context, UriUtils.getIconResource(imageUri));
+ return UriUtils.getDrawable(context,
+ UriUtils.getIconResource(context, imageUri));
} else if (UriUtils.isContentUri(imageUri)) {
ContentResolver resolver = context.getContentResolver();