Bug 961499 - Return correct size favicons for Top Sites' add to home screen functionality. r=bnicholson

mobile/android/base/GeckoAppShell.java

@@ -6,6 +6,7 @@
 package org.mozilla.gecko;
 
 import org.mozilla.gecko.favicons.OnFaviconLoadedListener;
+import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.gfx.GeckoLayerClient;
 import org.mozilla.gecko.gfx.LayerView;
@@ -771,51 +772,67 @@ public class GeckoAppShell
         createShortcut(aTitle, aURI, aURI, aIconData, aType);
     }
 
-    // for non-webapps
+    // For non-webapps.
     public static void createShortcut(String aTitle, String aURI, Bitmap aBitmap, String aType) {
         createShortcut(aTitle, aURI, aURI, aBitmap, aType);
     }
 
-    // internal, for webapps
+    // Internal, for webapps.
-    static void createShortcut(String aTitle, String aURI, String aUniqueURI, String aIconData, String aType) {
+    static void createShortcut(final String aTitle, final String aURI, final String aUniqueURI, final String aIconData, final String aType) {
-        createShortcut(aTitle, aURI, aUniqueURI, BitmapUtils.getBitmapFromDataURI(aIconData), aType);
-    }
-
-    public static void createShortcut(final String aTitle, final String aURI, final String aUniqueURI,
-                                      final Bitmap aIcon, final String aType)
-    {
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
-                // the intent to be launched by the shortcut
+                // TODO: use the cache. Bug 961600.
-                Intent shortcutIntent;
+                Bitmap icon = FaviconDecoder.getMostSuitableBitmapFromDataURI(aIconData, getPreferredIconSize());
-                if (aType.equalsIgnoreCase(SHORTCUT_TYPE_WEBAPP)) {
+                GeckoAppShell.doCreateShortcut(aTitle, aURI, aURI, icon, aType);
-                    shortcutIntent = getWebAppIntent(aURI, aUniqueURI, aTitle, aIcon);
-                } else {
-                    shortcutIntent = new Intent();
-                    shortcutIntent.setAction(GeckoApp.ACTION_BOOKMARK);
-                    shortcutIntent.setData(Uri.parse(aURI));
-                    shortcutIntent.setClassName(AppConstants.ANDROID_PACKAGE_NAME,
-                                                AppConstants.BROWSER_INTENT_CLASS);
-                }
-        
-                Intent intent = new Intent();
-                intent.putExtra(Intent.EXTRA_SHORTCUT_INTENT, shortcutIntent);
-                if (aTitle != null)
-                    intent.putExtra(Intent.EXTRA_SHORTCUT_NAME, aTitle);
-                else
-                    intent.putExtra(Intent.EXTRA_SHORTCUT_NAME, aURI);
-                intent.putExtra(Intent.EXTRA_SHORTCUT_ICON, getLauncherIcon(aIcon, aType));
-
-                // Do not allow duplicate items
-                intent.putExtra("duplicate", false);
-
-                intent.setAction("com.android.launcher.action.INSTALL_SHORTCUT");
-                getContext().sendBroadcast(intent);
             }
         });
     }
 
+    public static void createShortcut(final String aTitle, final String aURI, final String aUniqueURI,
+                                      final Bitmap aIcon, final String aType) {
+        ThreadUtils.postToBackgroundThread(new Runnable() {
+            @Override
+            public void run() {
+                GeckoAppShell.doCreateShortcut(aTitle, aURI, aUniqueURI, aIcon, aType);
+            }
+        });
+    }
+
+    /**
+     * Call this method only on the background thread.
+     */
+    private static void doCreateShortcut(final String aTitle, final String aURI, final String aUniqueURI,
+                                         final Bitmap aIcon, final String aType) {
+        // The intent to be launched by the shortcut.
+        Intent shortcutIntent;
+        if (aType.equalsIgnoreCase(SHORTCUT_TYPE_WEBAPP)) {
+            shortcutIntent = getWebAppIntent(aURI, aUniqueURI, aTitle, aIcon);
+        } else {
+            shortcutIntent = new Intent();
+            shortcutIntent.setAction(GeckoApp.ACTION_BOOKMARK);
+            shortcutIntent.setData(Uri.parse(aURI));
+            shortcutIntent.setClassName(AppConstants.ANDROID_PACKAGE_NAME,
+                                        AppConstants.BROWSER_INTENT_CLASS);
+        }
+
+        Intent intent = new Intent();
+        intent.putExtra(Intent.EXTRA_SHORTCUT_INTENT, shortcutIntent);
+        intent.putExtra(Intent.EXTRA_SHORTCUT_ICON, getLauncherIcon(aIcon, aType));
+
+        if (aTitle != null) {
+            intent.putExtra(Intent.EXTRA_SHORTCUT_NAME, aTitle);
+        } else {
+            intent.putExtra(Intent.EXTRA_SHORTCUT_NAME, aURI);
+        }
+
+        // Do not allow duplicate items.
+        intent.putExtra("duplicate", false);
+
+        intent.setAction("com.android.launcher.action.INSTALL_SHORTCUT");
+        getContext().sendBroadcast(intent);
+    }
+
     public static void removeShortcut(final String aTitle, final String aURI, final String aType) {
         removeShortcut(aTitle, aURI, null, aType);
     }

mobile/android/base/favicons/Favicons.java

@@ -7,6 +7,7 @@ package org.mozilla.gecko.favicons;
 
 import org.mozilla.gecko.AboutPages;
 import org.mozilla.gecko.AppConstants;
+import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Tab;
 import org.mozilla.gecko.Tabs;
@@ -465,15 +466,19 @@ public class Favicons {
     }
 
     /**
-     * Sidestep the cache and get, from either the database or the internet, the largest available
+     * Sidestep the cache and get, from either the database or the internet, a favicon
-     * Favicon for the given page URL. Useful for creating homescreen shortcuts without being limited
+     * suitable for use as an app icon for the provided URL.
-     * by possibly low-resolution values in the cache.
-     * Deduces the favicon URL from the history database and, ultimately, guesses.
      *
-     * @param url Page URL to get a large favicon image fro.
+     * Useful for creating homescreen shortcuts without being limited
-     * @param onFaviconLoadedListener Listener to call back with the result.
+     * by possibly low-resolution values in the cache.
+     *
+     * Deduces the favicon URL from the browser database, guessing if necessary.
+     *
+     * @param url page URL to get a large favicon image for.
+     * @param onFaviconLoadedListener listener to call back with the result.
      */
-    public static void getLargestFaviconForPage(String url, OnFaviconLoadedListener onFaviconLoadedListener) {
+    public static void getPreferredSizeFaviconForPage(String url, OnFaviconLoadedListener onFaviconLoadedListener) {
-        loadUncachedFavicon(url, null, 0, -1, onFaviconLoadedListener);
+        int preferredSize = GeckoAppShell.getPreferredIconSize();
+        loadUncachedFavicon(url, null, 0, preferredSize, onFaviconLoadedListener);
     }
 }

mobile/android/base/favicons/LoadFaviconTask.java

@@ -474,7 +474,6 @@ public class LoadFaviconTask extends UiAsyncTask<Void, Void, Bitmap> {
 
     private void processResult(Bitmap image) {
         Favicons.removeLoadTask(mId);
-
         Bitmap scaled = image;
 
         // Notify listeners, scaling if required.

mobile/android/base/favicons/cache/FaviconCache.java

@@ -332,7 +332,8 @@ public class FaviconCache {
 
             FaviconCacheElement cacheElement;
 
-            int cacheElementIndex = container.getNextHighestIndex(targetSize);
+            // If targetSize is -1, it means we want the largest possible icon.
+            int cacheElementIndex = (targetSize == -1) ? -1 : container.getNextHighestIndex(targetSize);
 
             // cacheElementIndex now holds either the index of the next least largest bitmap from
             // targetSize, or -1 if targetSize > all bitmaps.
@@ -362,12 +363,16 @@ public class FaviconCache {
             // If there is no such primary, we'll upscale the next least smaller one instead.
             cacheElement = container.getNextPrimary(cacheElementIndex);
 
-
             if (cacheElement == null) {
                 // The primary has been invalidated! Fail! Need to get it back from the database.
                 return null;
             }
 
+            if (targetSize == -1) {
+                // We got the biggest primary, so that's what we'll return.
+                return cacheElement.mFaviconPayload;
+            }
+
             // Having got this far, we'll be needing to write the new secondary to the cache, which
             // involves us falling through to the next try block. This flag lets us do this (Other
             // paths prior to this end in returns.)

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

@@ -145,6 +145,50 @@ 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/home/HomeFragment.java

@@ -137,8 +137,8 @@ abstract class HomeFragment extends Fragment {
                 return false;
             }
 
-            // Fetch the largest cacheable icon size.
+            // Fetch an icon big enough for use as a home screen icon.
-            Favicons.getLargestFaviconForPage(info.url, new GeckoAppShell.CreateShortcutFaviconLoadedListener(info.url, info.getDisplayTitle()));
+            Favicons.getPreferredSizeFaviconForPage(info.url, new GeckoAppShell.CreateShortcutFaviconLoadedListener(info.url, info.getDisplayTitle()));
             return true;
         }
 

mobile/android/base/home/TopSitesPanel.java

@@ -363,8 +363,8 @@ public class TopSitesPanel extends HomeFragment {
                 return false;
             }
 
-            // Fetch the largest cacheable icon size.
+            // Fetch an icon big enough for use as a home screen icon.
-            Favicons.getLargestFaviconForPage(info.url, new GeckoAppShell.CreateShortcutFaviconLoadedListener(info.url, info.getDisplayTitle()));
+            Favicons.getPreferredSizeFaviconForPage(info.url, new GeckoAppShell.CreateShortcutFaviconLoadedListener(info.url, info.getDisplayTitle()));
             return true;
         }
 

mobile/android/base/preferences/SearchEnginePreference.java

@@ -135,31 +135,6 @@ public class SearchEnginePreference extends CustomListPreference {
         final String iconURI = geckoEngineJSON.getString("iconURI");
         // Keep a reference to the bitmap - we'll need it later in onBindView.
         try {
-
-            // 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.
-
-            LoadFaviconResult result = FaviconDecoder.decodeDataURI(iconURI);
-            if (result == null) {
-                // Nothing we can do.
-                Log.w(LOGTAG, "Unable to decode icon URI.");
-                return;
-            }
-
-            Iterator<Bitmap> bitmaps = result.getBitmaps();
-            if (!bitmaps.hasNext()) {
-                Log.w(LOGTAG, "No bitmaps in decoded icon.");
-                return;
-            }
-
-            mIconBitmap = bitmaps.next();
-
-            if (!bitmaps.hasNext()) {
-                // We're done! There was only one, so this is as big as it gets.
-                return;
-            }
-
-            // Find a bitmap of a more suitable size.
             final int desiredWidth;
             if (mFaviconView != null) {
                 desiredWidth = mFaviconView.getWidth();
@@ -174,15 +149,8 @@ public class SearchEnginePreference extends CustomListPreference {
                 }
             }
 
-            int currentWidth = mIconBitmap.getWidth();
+            // TODO: use the cache. Bug 961600.
-            while ((currentWidth < desiredWidth) &&
+            mIconBitmap = FaviconDecoder.getMostSuitableBitmapFromDataURI(iconURI, desiredWidth);
-                   bitmaps.hasNext()) {
-                Bitmap b = bitmaps.next();
-                if (b.getWidth() > currentWidth) {
-                    currentWidth = b.getWidth();
-                    mIconBitmap = b;
-                }
-            }
 
         } catch (IllegalArgumentException e) {
             Log.e(LOGTAG, "IllegalArgumentException creating Bitmap. Most likely a zero-length bitmap.", e);

mobile/android/base/webapp/EventListener.java

@@ -8,6 +8,7 @@ package org.mozilla.gecko.webapp;
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.GeckoProfile;
+import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.util.ActivityResultHandler;
 import org.mozilla.gecko.util.EventDispatcher;
@@ -114,7 +115,9 @@ public class EventListener implements GeckoEventListener {
         int index = allocator.getIndexForApp(aOriginalOrigin);
 
         assert aIconURL != null;
-        Bitmap icon = BitmapUtils.getBitmapFromDataURI(aIconURL);
+
+        final int preferredSize = GeckoAppShell.getPreferredIconSize();
+        Bitmap icon = FaviconDecoder.getMostSuitableBitmapFromDataURI(aIconURL, preferredSize);
 
         assert aOrigin != null && index != -1;
         allocator.updateAppAllocation(aOrigin, index, icon);

mobile/android/components/build/nsIShellService.idl

@@ -19,7 +19,7 @@ interface nsIShellService : nsISupports
    *
    * @param aTitle     the user-friendly name of the shortcut.
    * @param aURI       the URI to open.
-   * @param aIconData  a base64 encoded representation of the shortcut's icon.
+   * @param aIconData  a base64-encoded data: URI representation of the shortcut's icon, as accepted by the favicon decoder.
    * @param aIntent    how the URI should be opened. Examples: "default", "bookmark" and "webapp"
    */
   void createShortcut(in AString aTitle, in AString aURI, in AString aIconData, in AString aIntent);