Bug 961600: Use the favicon cache for search engine icons. r=rnewman

mobile/android/base/GeckoAppShell.java

@@ -34,8 +34,8 @@ import java.util.concurrent.ConcurrentLinkedQueue;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.AppConstants.Versions;
+import org.mozilla.gecko.favicons.Favicons;
 import org.mozilla.gecko.favicons.OnFaviconLoadedListener;
-import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.gfx.LayerView;
 import org.mozilla.gecko.gfx.PanZoomController;
@@ -802,21 +802,24 @@ public class GeckoAppShell
     // This is the entry point from nsIShellService.
     @WrapElementForJNI
     static void createShortcut(final String aTitle, final String aURI, final String aIconData) {
-        ThreadUtils.postToBackgroundThread(new Runnable() {
+        // We have the favicon data (base64) decoded on the background thread, callback here, then
-            @Override
+        // call the other createShortcut method with the decoded favicon.
-            public void run() {
+        // This is slightly contrived, but makes the images available to the favicon cache.
-                // TODO: use the cache. Bug 961600.
+        Favicons.getSizedFavicon(getContext(), aURI, aIconData, Integer.MAX_VALUE, 0,
-                Bitmap icon = FaviconDecoder.getMostSuitableBitmapFromDataURI(aIconData, getPreferredIconSize());
+            new OnFaviconLoadedListener() {
-                GeckoAppShell.doCreateShortcut(aTitle, aURI, icon);
+                @Override
+                public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) {
+                    createShortcut(aTitle, url, favicon);
+                }
             }
-        });
+        );
     }
 
     public static void createShortcut(final String aTitle, final String aURI, final Bitmap aBitmap) {
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
-                GeckoAppShell.doCreateShortcut(aTitle, aURI, aBitmap);
+                doCreateShortcut(aTitle, aURI, aBitmap);
             }
         });
     }

mobile/android/base/favicons/decoders/FaviconDecoder.java

@@ -151,50 +151,6 @@ public class FaviconDecoder {
         return decodeFavicon(buffer, 0, buffer.length);
     }
 
-    /**
-     * Returns the smallest bitmap in the icon represented by the provided
-     * data: URI that's larger than the desired width, or the largest if
-     * there is no larger icon.
-     *
-     * Returns null if no bitmap could be extracted.
-     *
-     * Bug 961600: we shouldn't be doing all of this work. The favicon cache
-     * should be used, and will give us the right size icon.
-     */
-    public static Bitmap getMostSuitableBitmapFromDataURI(String iconURI, int desiredWidth) {
-        LoadFaviconResult result = FaviconDecoder.decodeDataURI(iconURI);
-        if (result == null) {
-            // Nothing we can do.
-            Log.w(LOG_TAG, "Unable to decode icon URI.");
-            return null;
-        }
-
-        final Iterator<Bitmap> bitmaps = result.getBitmaps();
-        if (!bitmaps.hasNext()) {
-            Log.w(LOG_TAG, "No bitmaps in decoded icon.");
-            return null;
-        }
-
-        Bitmap bitmap = bitmaps.next();
-        if (!bitmaps.hasNext()) {
-            // We're done! There was only one, so this is as big as it gets.
-            return bitmap;
-        }
-
-        // Find a bitmap of the most suitable size.
-        int currentWidth = bitmap.getWidth();
-        while ((currentWidth < desiredWidth) &&
-               bitmaps.hasNext()) {
-            final Bitmap b = bitmaps.next();
-            if (b.getWidth() > currentWidth) {
-                currentWidth = b.getWidth();
-                bitmap = b;
-            }
-        }
-
-        return bitmap;
-    }
-
     /**
      * Iterator to hold a single bitmap.
      */

mobile/android/base/preferences/SearchEnginePreference.java

@@ -8,6 +8,7 @@ import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.favicons.Favicons;
+import org.mozilla.gecko.favicons.OnFaviconLoadedListener;
 import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.widget.FaviconView;
@@ -34,6 +35,7 @@ public class SearchEnginePreference extends CustomListPreference {
 
     // The bitmap backing the drawable above - needed separately for the FaviconView.
     private Bitmap mIconBitmap;
+    private final Object bitmapLock = new Object();
 
     private FaviconView mFaviconView;
 
@@ -55,9 +57,16 @@ public class SearchEnginePreference extends CustomListPreference {
     protected void onBindView(View view) {
         super.onBindView(view);
 
-        // Set the icon in the FaviconView.
+        // We synchronise to avoid a race condition between this and the favicon loading callback in
-        mFaviconView = ((FaviconView) view.findViewById(R.id.search_engine_icon));
+        // setSearchEngineFromJSON.
-        mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString());
+        synchronized (bitmapLock) {
+            // Set the icon in the FaviconView.
+            mFaviconView = ((FaviconView) view.findViewById(R.id.search_engine_icon));
+
+            if (mIconBitmap != null) {
+                mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString());
+            }
+        }
     }
 
     @Override
@@ -161,9 +170,20 @@ public class SearchEnginePreference extends CustomListPreference {
                 }
             }
 
-            // TODO: use the cache. Bug 961600.
+            Favicons.getSizedFavicon(getContext(), mIdentifier, iconURI, desiredWidth, 0,
-            mIconBitmap = FaviconDecoder.getMostSuitableBitmapFromDataURI(iconURI, desiredWidth);
+                new OnFaviconLoadedListener() {
+                    @Override
+                    public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) {
+                        synchronized (bitmapLock) {
+                            mIconBitmap = favicon;
 
+                            if (mFaviconView != null) {
+                                mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString());
+                            }
+                        }
+                    }
+                }
+            );
         } catch (IllegalArgumentException e) {
             Log.e(LOGTAG, "IllegalArgumentException creating Bitmap. Most likely a zero-length bitmap.", e);
         } catch (NullPointerException e) {