Bug 1238656 - Part 2: introduce FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS to avoid icon downscaling r=mcomella draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 02 Feb 2016 17:23:35 -0800
changeset 331388 38e58631796d042efa28d0349b0c94065f9ca7b8
parent 331387 8b9c96ffaa605deb5901d931ca16587bfa8929dc
child 331389 12920b739fa840651ff85450ba2e36079521f3e8
push id10975
push userahunt@mozilla.com
push dateWed, 17 Feb 2016 01:06:19 +0000
reviewersmcomella
bugs1238656
milestone47.0a1
Bug 1238656 - Part 2: introduce FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS to avoid icon downscaling r=mcomella MozReview-Commit-ID: BXLqJ9wjzf7
mobile/android/base/java/org/mozilla/gecko/favicons/LoadFaviconTask.java
--- a/mobile/android/base/java/org/mozilla/gecko/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/java/org/mozilla/gecko/favicons/LoadFaviconTask.java
@@ -20,22 +20,24 @@ import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.favicons.decoders.LoadFaviconResult;
 import org.mozilla.gecko.util.GeckoJarReader;
 import org.mozilla.gecko.util.IOUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.mozilla.gecko.util.IOUtils.ConsumedInputStream;
 
 /**
  * Class representing the asynchronous task to load a Favicon which is not currently in the in-memory
  * cache.
@@ -45,16 +47,24 @@ import static org.mozilla.gecko.util.IOU
 public class LoadFaviconTask {
     private static final String LOGTAG = "LoadFaviconTask";
 
     // Access to this map needs to be synchronized prevent multiple jobs loading the same favicon
     // from executing concurrently.
     private static final HashMap<String, LoadFaviconTask> loadsInFlight = new HashMap<>();
 
     public static final int FLAG_PERSIST = 1;
+    /**
+     * Bypass all caches - this is used to directly retrieve the requested icon. Without this flag,
+     * favicons will first be pushed into the memory cache (and possibly permanent cache if using FLAG_PERSIST),
+     * where they will be downscaled to the maximum cache size, before being retrieved from the cache (resulting
+     * in a possibly smaller icon size).
+     */
+    public static final int FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS = 2;
+
     private static final int MAX_REDIRECTS_TO_FOLLOW = 5;
     // The default size of the buffer to use for downloading Favicons in the event no size is given
     // by the server.
     public static final int DEFAULT_FAVICON_BUFFER_SIZE = 25000;
 
     private static final AtomicInteger nextFaviconLoadId = new AtomicInteger(0);
     private final Context context;
     private final int id;
@@ -412,20 +422,23 @@ public class LoadFaviconTask {
             // chain onto the same parent task.
             loadsInFlight.put(faviconURL, this);
         }
 
         if (isCancelled()) {
             return null;
         }
 
+        LoadFaviconResult loadedBitmaps = null;
         // If there are no valid bitmaps decoded, the returned LoadFaviconResult is null.
-        LoadFaviconResult loadedBitmaps = loadFaviconFromDb(db);
-        if (loadedBitmaps != null) {
-            return pushToCacheAndGetResult(loadedBitmaps);
+        if ((flags & FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS) == 0) {
+            loadedBitmaps = loadFaviconFromDb(db);
+            if (loadedBitmaps != null) {
+                return pushToCacheAndGetResult(loadedBitmaps);
+            }
         }
 
         if (onlyFromLocal || isCancelled()) {
             return null;
         }
 
         // Let's see if it's in a JAR.
         image = fetchJARFavicon(faviconURL);
@@ -440,21 +453,35 @@ public class LoadFaviconTask {
         } catch (URISyntaxException e) {
             Log.e(LOGTAG, "The provided favicon URL is not valid");
             return null;
         } catch (Exception e) {
             Log.e(LOGTAG, "Couldn't download favicon.", e);
         }
 
         if (loadedBitmaps != null) {
-            // Fetching bytes to store can fail. saveFaviconToDb will
-            // do the right thing, but we still choose to cache the
-            // downloaded icon in memory.
-            saveFaviconToDb(db, loadedBitmaps.getBytesForDatabaseStorage());
-            return pushToCacheAndGetResult(loadedBitmaps);
+            if ((flags & FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS) == 0) {
+                // Fetching bytes to store can fail. saveFaviconToDb will
+                // do the right thing, but we still choose to cache the
+                // downloaded icon in memory.
+                saveFaviconToDb(db, loadedBitmaps.getBytesForDatabaseStorage());
+                return pushToCacheAndGetResult(loadedBitmaps);
+            } else {
+                Map<Integer, Bitmap> iconMap = new HashMap<>();
+                List<Integer> sizes = new ArrayList<>();
+
+                while (loadedBitmaps.getBitmaps().hasNext()) {
+                    Bitmap b = loadedBitmaps.getBitmaps().next();
+                    iconMap.put(b.getWidth(), b);
+                    sizes.add(b.getWidth());
+                }
+
+                int bestSize = Favicons.selectBestSizeFromList(sizes, targetWidth);
+                return iconMap.get(bestSize);
+            }
         }
 
         if (isUsingDefaultURL) {
             Favicons.putFaviconInFailedCache(faviconURL);
             return null;
         }
 
         if (isCancelled()) {
@@ -540,21 +567,25 @@ public class LoadFaviconTask {
                 // image now, or the call into the cache in processResult will fetch the right one.
                 t.processResult(image);
             }
         }
     }
 
     private void processResult(Bitmap image) {
         Favicons.removeLoadTask(id);
-        Bitmap scaled = image;
+        final Bitmap scaled;
 
         // Notify listeners, scaling if required.
-        if (targetWidth != -1 && image != null &&  image.getWidth() != targetWidth) {
+        if ((flags & FLAG_BYPASS_CACHE_WHEN_DOWNLOADING_ICONS) != 0) {
+            scaled = Bitmap.createScaledBitmap(image, targetWidth, targetWidth, true);
+        } else if (targetWidth != -1 && image != null &&  image.getWidth() != targetWidth) {
             scaled = Favicons.getSizedFaviconFromCache(faviconURL, targetWidth);
+        } else {
+            scaled = image;
         }
 
         Favicons.dispatchResult(pageUrl, faviconURL, scaled, listener);
     }
 
     void onCancelled() {
         Favicons.removeLoadTask(id);