Fixing some IconCache methods not thread safe

Bug: 17981568
Change-Id: I0d49604c2e38bc9017cba527d87e24e8b086f1da

2 files changed, 70 insertions, 108 deletions

diff --git a/src/com/android/launcher3/IconCache.java b/src/com/android/launcher3/IconCache.java
index 06f9f2941..5a0875b30 100644
--- a/src/com/android/launcher3/IconCache.java
+++ b/src/com/android/launcher3/IconCache.java
@@ -24,7 +24,6 @@ import android.content.pm.ActivityInfo;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
-import android.content.pm.ResolveInfo;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
@@ -117,8 +116,7 @@ public class IconCache {
     }
 
     public Drawable getFullResDefaultActivityIcon() {
-        return getFullResIcon(Resources.getSystem(),
-                android.R.mipmap.sym_def_app_icon);
+        return getFullResIcon(Resources.getSystem(), android.R.mipmap.sym_def_app_icon);
     }
 
     private Drawable getFullResIcon(Resources resources, int iconId) {
@@ -151,12 +149,7 @@ public class IconCache {
         return mIconDpi;
     }
 
-    public Drawable getFullResIcon(ResolveInfo info) {
-        return getFullResIcon(info.activityInfo);
-    }
-
     public Drawable getFullResIcon(ActivityInfo info) {
-
         Resources resources;
         try {
             resources = mPackageManager.getResourcesForApplication(
@@ -190,16 +183,14 @@ public class IconCache {
     /**
      * Remove any records for the supplied ComponentName.
      */
-    public void remove(ComponentName componentName, UserHandleCompat user) {
-        synchronized (mCache) {
-            mCache.remove(new CacheKey(componentName, user));
-        }
+    public synchronized void remove(ComponentName componentName, UserHandleCompat user) {
+        mCache.remove(new CacheKey(componentName, user));
     }
 
     /**
      * Remove any records for the supplied package name.
      */
-    public void remove(String packageName, UserHandleCompat user) {
+    public synchronized void remove(String packageName, UserHandleCompat user) {
         HashSet<CacheKey> forDeletion = new HashSet<CacheKey>();
         for (CacheKey key: mCache.keySet()) {
             if (key.componentName.getPackageName().equals(packageName)
@@ -215,24 +206,20 @@ public class IconCache {
     /**
      * Empty out the cache.
      */
-    public void flush() {
-        synchronized (mCache) {
-            mCache.clear();
-        }
+    public synchronized void flush() {
+        mCache.clear();
     }
 
     /**
      * Empty out the cache that aren't of the correct grid size
      */
-    public void flushInvalidIcons(DeviceProfile grid) {
-        synchronized (mCache) {
-            Iterator<Entry<CacheKey, CacheEntry>> it = mCache.entrySet().iterator();
-            while (it.hasNext()) {
-                final CacheEntry e = it.next().getValue();
-                if ((e.icon != null) && (e.icon.getWidth() < grid.iconSizePx
-                        || e.icon.getHeight() < grid.iconSizePx)) {
-                    it.remove();
-                }
+    public synchronized void flushInvalidIcons(DeviceProfile grid) {
+        Iterator<Entry<CacheKey, CacheEntry>> it = mCache.entrySet().iterator();
+        while (it.hasNext()) {
+            final CacheEntry e = it.next().getValue();
+            if ((e.icon != null) && (e.icon.getWidth() < grid.iconSizePx
+                    || e.icon.getHeight() < grid.iconSizePx)) {
+                it.remove();
             }
         }
     }
@@ -240,90 +227,78 @@ public class IconCache {
     /**
      * Fill in "application" with the icon and label for "info."
      */
-    public void getTitleAndIcon(AppInfo application, LauncherActivityInfoCompat info,
+    public synchronized void getTitleAndIcon(AppInfo application, LauncherActivityInfoCompat info,
             HashMap<Object, CharSequence> labelCache) {
-        synchronized (mCache) {
-            CacheEntry entry = cacheLocked(application.componentName, info, labelCache,
-                    info.getUser(), false);
-
-            application.title = entry.title;
-            application.iconBitmap = entry.icon;
-            application.contentDescription = entry.contentDescription;
-        }
-    }
+        CacheEntry entry = cacheLocked(application.componentName, info, labelCache,
+                info.getUser(), false);
 
-    public Bitmap getIcon(Intent intent, UserHandleCompat user) {
-        return getIcon(intent, null, user, true);
+        application.title = entry.title;
+        application.iconBitmap = entry.icon;
+        application.contentDescription = entry.contentDescription;
     }
 
-    private Bitmap getIcon(Intent intent, String title, UserHandleCompat user, boolean usePkgIcon) {
-        synchronized (mCache) {
-            ComponentName component = intent.getComponent();
-            // null info means not installed, but if we have a component from the intent then
-            // we should still look in the cache for restored app icons.
-            if (component == null) {
-                return getDefaultIcon(user);
-            }
-
-            LauncherActivityInfoCompat launcherActInfo = mLauncherApps.resolveActivity(intent, user);
-            CacheEntry entry = cacheLocked(component, launcherActInfo, null, user, usePkgIcon);
-            if (title != null) {
-                entry.title = title;
-                entry.contentDescription = mUserManager.getBadgedLabelForUser(title, user);
-            }
-            return entry.icon;
+    public synchronized Bitmap getIcon(Intent intent, UserHandleCompat user) {
+        ComponentName component = intent.getComponent();
+        // null info means not installed, but if we have a component from the intent then
+        // we should still look in the cache for restored app icons.
+        if (component == null) {
+            return getDefaultIcon(user);
         }
+
+        LauncherActivityInfoCompat launcherActInfo = mLauncherApps.resolveActivity(intent, user);
+        CacheEntry entry = cacheLocked(component, launcherActInfo, null, user, true);
+        return entry.icon;
     }
 
     /**
      * Fill in "shortcutInfo" with the icon and label for "info."
      */
-    public void getTitleAndIcon(ShortcutInfo shortcutInfo, Intent intent, UserHandleCompat user,
-            boolean usePkgIcon) {
-        synchronized (mCache) {
-            ComponentName component = intent.getComponent();
-            // null info means not installed, but if we have a component from the intent then
-            // we should still look in the cache for restored app icons.
-            if (component == null) {
-                shortcutInfo.setIcon(getDefaultIcon(user));
-                shortcutInfo.title = "";
-                shortcutInfo.usingFallbackIcon = true;
-            } else {
-                LauncherActivityInfoCompat launcherActInfo =
-                        mLauncherApps.resolveActivity(intent, user);
-                CacheEntry entry = cacheLocked(component, launcherActInfo, null, user, usePkgIcon);
+    public synchronized void getTitleAndIcon(ShortcutInfo shortcutInfo, Intent intent,
+            UserHandleCompat user, boolean usePkgIcon) {
+        ComponentName component = intent.getComponent();
+        // null info means not installed, but if we have a component from the intent then
+        // we should still look in the cache for restored app icons.
+        if (component == null) {
+            shortcutInfo.setIcon(getDefaultIcon(user));
+            shortcutInfo.title = "";
+            shortcutInfo.usingFallbackIcon = true;
+        } else {
+            LauncherActivityInfoCompat launcherActInfo =
+                    mLauncherApps.resolveActivity(intent, user);
+            CacheEntry entry = cacheLocked(component, launcherActInfo, null, user, usePkgIcon);
 
-                shortcutInfo.setIcon(entry.icon);
-                shortcutInfo.title = entry.title;
-                shortcutInfo.usingFallbackIcon = isDefaultIcon(entry.icon, user);
-            }
+            shortcutInfo.setIcon(entry.icon);
+            shortcutInfo.title = entry.title;
+            shortcutInfo.usingFallbackIcon = isDefaultIcon(entry.icon, user);
         }
     }
 
 
-    public Bitmap getDefaultIcon(UserHandleCompat user) {
+    public synchronized Bitmap getDefaultIcon(UserHandleCompat user) {
         if (!mDefaultIcons.containsKey(user)) {
             mDefaultIcons.put(user, makeDefaultIcon(user));
         }
         return mDefaultIcons.get(user);
     }
 
-    public Bitmap getIcon(ComponentName component, LauncherActivityInfoCompat info,
+    public synchronized Bitmap getIcon(ComponentName component, LauncherActivityInfoCompat info,
             HashMap<Object, CharSequence> labelCache) {
-        synchronized (mCache) {
-            if (info == null || component == null) {
-                return null;
-            }
-
-            CacheEntry entry = cacheLocked(component, info, labelCache, info.getUser(), false);
-            return entry.icon;
+        if (info == null || component == null) {
+            return null;
         }
+
+        CacheEntry entry = cacheLocked(component, info, labelCache, info.getUser(), false);
+        return entry.icon;
     }
 
     public boolean isDefaultIcon(Bitmap icon, UserHandleCompat user) {
         return mDefaultIcons.get(user) == icon;
     }
 
+    /**
+     * Retrieves the entry from the cache. If the entry is not present, it creates a new entry.
+     * This method is not thread safe, it must be called from a synchronized method.
+     */
     private CacheEntry cacheLocked(ComponentName componentName, LauncherActivityInfoCompat info,
             HashMap<Object, CharSequence> labelCache, UserHandleCompat user, boolean usePackageIcon) {
         CacheKey cacheKey = new CacheKey(componentName, user);
@@ -380,7 +355,7 @@ public class IconCache {
      * Adds a default package entry in the cache. This entry is not persisted and will be removed
      * when the cache is flushed.
      */
-    public void cachePackageInstallInfo(String packageName, UserHandleCompat user,
+    public synchronized void cachePackageInstallInfo(String packageName, UserHandleCompat user,
             Bitmap icon, CharSequence title) {
         remove(packageName, user);
 
@@ -395,9 +370,10 @@ public class IconCache {
 
     /**
      * Gets an entry for the package, which can be used as a fallback entry for various components.
+     * This method is not thread safe, it must be called from a synchronized method.
      */
     private CacheEntry getEntryForPackage(String packageName, UserHandleCompat user) {
-        ComponentName cn = getPackageComponent(packageName);
+        ComponentName cn = new ComponentName(packageName, EMPTY_CLASS_NAME);;
         CacheKey cacheKey = new CacheKey(cn, user);
         CacheEntry entry = mCache.get(cacheKey);
         if (entry == null) {
@@ -420,15 +396,13 @@ public class IconCache {
         return entry;
     }
 
-    public HashMap<ComponentName,Bitmap> getAllIcons() {
-        synchronized (mCache) {
-            HashMap<ComponentName,Bitmap> set = new HashMap<ComponentName,Bitmap>();
-            for (CacheKey ck : mCache.keySet()) {
-                final CacheEntry e = mCache.get(ck);
-                set.put(ck.componentName, e.icon);
-            }
-            return set;
+    public synchronized HashMap<ComponentName,Bitmap> getAllIcons() {
+        HashMap<ComponentName,Bitmap> set = new HashMap<ComponentName,Bitmap>();
+        for (CacheKey ck : mCache.keySet()) {
+            final CacheEntry e = mCache.get(ck);
+            set.put(ck.componentName, e.icon);
         }
+        return set;
     }
 
     /**
@@ -534,23 +508,15 @@ public class IconCache {
      * Remove a pre-loaded icon from the persistent icon cache.
      *
      * @param componentName the component that should own the icon
-     * @returns true on success
      */
-    public boolean deletePreloadedIcon(ComponentName componentName, UserHandleCompat user) {
+    public void deletePreloadedIcon(ComponentName componentName, UserHandleCompat user) {
         // We don't keep icons for other profiles in persistent cache.
-        if (!user.equals(UserHandleCompat.myUserHandle())) {
-            return false;
-        }
-        if (componentName == null) {
-            return false;
-        }
-        if (mCache.remove(componentName) != null) {
-            if (DEBUG) Log.d(TAG, "removed pre-loaded icon from the in-memory cache");
+        if (!user.equals(UserHandleCompat.myUserHandle()) || componentName == null) {
+            return;
         }
+        remove(componentName, user);
         boolean success = mContext.deleteFile(getResourceFilename(componentName));
         if (DEBUG && success) Log.d(TAG, "removed pre-loaded icon from persistent cache");
-
-        return success;
     }
 
     private static String getResourceFilename(ComponentName component) {
@@ -558,8 +524,4 @@ public class IconCache {
         String filename = resourceName.replace(File.separatorChar, '_');
         return RESOURCE_FILE_PREFIX + filename;
     }
-
-    static ComponentName getPackageComponent(String packageName) {
-        return new ComponentName(packageName, EMPTY_CLASS_NAME);
-    }
 }
diff --git a/src/com/android/launcher3/WidgetPreviewLoader.java b/src/com/android/launcher3/WidgetPreviewLoader.java
index 92d7c7d7f..9cedae0f9 100644
--- a/src/com/android/launcher3/WidgetPreviewLoader.java
+++ b/src/com/android/launcher3/WidgetPreviewLoader.java
@@ -638,7 +638,7 @@ public class WidgetPreviewLoader {
             c.setBitmap(null);
         }
         // Render the icon
-        Drawable icon = mutateOnMainThread(mIconCache.getFullResIcon(info));
+        Drawable icon = mutateOnMainThread(mIconCache.getFullResIcon(info.activityInfo));
 
         int paddingTop = mContext.
                 getResources().getDimensionPixelOffset(R.dimen.shortcut_preview_padding_top);