From 459d47ca20ccadda58cb2858cc9b501a67c726af Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Fri, 19 Sep 2014 18:10:01 -0700 Subject: [PATCH] Bug 961600: Use the favicon cache for search engine icons. r=rnewman --- mobile/android/base/GeckoAppShell.java | 20 +++++---- .../favicons/decoders/FaviconDecoder.java | 44 ------------------- .../preferences/SearchEnginePreference.java | 30 ++++++++++--- 3 files changed, 37 insertions(+), 57 deletions(-) diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java index 86ab8703788..3f338e40a39 100644 --- a/mobile/android/base/GeckoAppShell.java +++ b/mobile/android/base/GeckoAppShell.java @@ -31,6 +31,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; 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; @@ -806,21 +807,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() { - @Override - public void run() { - // TODO: use the cache. Bug 961600. - Bitmap icon = FaviconDecoder.getMostSuitableBitmapFromDataURI(aIconData, getPreferredIconSize()); - GeckoAppShell.doCreateShortcut(aTitle, aURI, icon); + // We have the favicon data (base64) decoded on the background thread, callback here, then + // call the other createShortcut method with the decoded favicon. + // This is slightly contrived, but makes the images available to the favicon cache. + Favicons.getSizedFavicon(getContext(), aURI, aIconData, Integer.MAX_VALUE, 0, + new OnFaviconLoadedListener() { + @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); } }); } diff --git a/mobile/android/base/favicons/decoders/FaviconDecoder.java b/mobile/android/base/favicons/decoders/FaviconDecoder.java index 4bd0da212af..a1a8e4c354a 100644 --- a/mobile/android/base/favicons/decoders/FaviconDecoder.java +++ b/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 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. */ diff --git a/mobile/android/base/preferences/SearchEnginePreference.java b/mobile/android/base/preferences/SearchEnginePreference.java index ce0fab15fd1..bf0946a79f2 100644 --- a/mobile/android/base/preferences/SearchEnginePreference.java +++ b/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. - mFaviconView = ((FaviconView) view.findViewById(R.id.search_engine_icon)); - mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString()); + // We synchronise to avoid a race condition between this and the favicon loading callback in + // setSearchEngineFromJSON. + 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. - mIconBitmap = FaviconDecoder.getMostSuitableBitmapFromDataURI(iconURI, desiredWidth); + Favicons.getSizedFavicon(getContext(), mIdentifier, iconURI, desiredWidth, 0, + 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) {