Bug 1003911 - Part 2: Don't write null favicons or thumbnails into the DB. r=margaret, a=sledru
authorRichard Newman <rnewman@mozilla.com>
Thu, 01 May 2014 14:19:34 -0700
changeset 192189 e03ba68c6044
parent 192188 97392de21322
child 192190 2ad68daa2b58
push id3516
push userryanvm@gmail.com
push date2014-05-02 12:22 +0000
treeherdermozilla-beta@e03ba68c6044 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmargaret, sledru
bugs1003911
milestone30.0
Bug 1003911 - Part 2: Don't write null favicons or thumbnails into the DB. r=margaret, a=sledru
mobile/android/base/Tab.java
mobile/android/base/favicons/LoadFaviconTask.java
mobile/android/base/favicons/decoders/LoadFaviconResult.java
--- a/mobile/android/base/Tab.java
+++ b/mobile/android/base/Tab.java
@@ -696,22 +696,28 @@ public class Tab {
         }, 500);
     }
 
     void handleContentLoaded() {
         setLoadProgressIfLoading(LOAD_PROGRESS_LOADED);
     }
 
     protected void saveThumbnailToDB() {
+        final BitmapDrawable thumbnail = mThumbnail;
+        if (thumbnail == null) {
+            return;
+        }
+
         try {
             String url = getURL();
-            if (url == null)
+            if (url == null) {
                 return;
+            }
 
-            BrowserDB.updateThumbnailForUrl(getContentResolver(), url, mThumbnail);
+            BrowserDB.updateThumbnailForUrl(getContentResolver(), url, thumbnail);
         } catch (Exception e) {
             // ignore
         }
     }
 
     public void addPluginView(View view) {
         mPluginViews.add(view);
     }
--- a/mobile/android/base/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/favicons/LoadFaviconTask.java
@@ -92,16 +92,20 @@ public class LoadFaviconTask extends UiA
     // Runs in background thread
     private LoadFaviconResult loadFaviconFromDb() {
         ContentResolver resolver = context.getContentResolver();
         return BrowserDB.getFaviconForFaviconUrl(resolver, faviconURL);
     }
 
     // Runs in background thread
     private void saveFaviconToDb(final byte[] encodedFavicon) {
+        if (encodedFavicon == null) {
+            return;
+        }
+
         if ((flags & FLAG_PERSIST) == 0) {
             return;
         }
 
         ContentResolver resolver = context.getContentResolver();
         BrowserDB.updateFaviconForUrl(resolver, pageUrl, encodedFavicon, faviconURL);
     }
 
@@ -371,16 +375,19 @@ public class LoadFaviconTask extends UiA
         } 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(loadedBitmaps.getBytesForDatabaseStorage());
             return pushToCacheAndGetResult(loadedBitmaps);
         }
 
         if (isUsingDefaultURL) {
             Favicons.putFaviconInFailedCache(faviconURL);
             return null;
         }
--- a/mobile/android/base/favicons/decoders/LoadFaviconResult.java
+++ b/mobile/android/base/favicons/decoders/LoadFaviconResult.java
@@ -29,46 +29,48 @@ public class LoadFaviconResult {
     Iterator<Bitmap> bitmapsDecoded;
 
     public Iterator<Bitmap> getBitmaps() {
         return bitmapsDecoded;
     }
 
     /**
      * Return a representation of this result suitable for storing in the database.
-     * For
      *
-     * @return A byte array containing the bytes from which this result was decoded.
+     * @return A byte array containing the bytes from which this result was decoded,
+     *         or null if re-encoding failed.
      */
     public byte[] getBytesForDatabaseStorage() {
         // Begin by normalising the buffer.
         if (offset != 0 || length != faviconBytes.length) {
             final byte[] normalised = new byte[length];
             System.arraycopy(faviconBytes, offset, normalised, 0, length);
             offset = 0;
             faviconBytes = normalised;
         }
 
-        // For results containing a single image, we re-encode the result as a PNG in an effort to
-        // save space.
-        if (!isICO) {
-            Bitmap favicon = ((FaviconDecoder.SingleBitmapIterator) bitmapsDecoded).peek();
-            byte[] data = null;
-            ByteArrayOutputStream stream = new ByteArrayOutputStream();
-
-            if (favicon.compress(Bitmap.CompressFormat.PNG, 100, stream)) {
-                data = stream.toByteArray();
-            } else {
-                Log.w(LOGTAG, "Favicon compression failed.");
-            }
-
-            return data;
-        }
-
         // For results containing multiple images, we store the result verbatim. (But cutting the
         // buffer to size first).
         // We may instead want to consider re-encoding the entire ICO as a collection of efficiently
         // encoded PNGs. This may not be worth the CPU time (Indeed, the encoding of single-image
         // favicons may also not be worth the time/space tradeoff.).
-        return faviconBytes;
+        if (isICO) {
+            return faviconBytes;
+        }
+
+        // For results containing a single image, we re-encode the
+        // result as a PNG in an effort to save space.
+        final Bitmap favicon = ((FaviconDecoder.SingleBitmapIterator) bitmapsDecoded).peek();
+        final ByteArrayOutputStream stream = new ByteArrayOutputStream();
+
+        try {
+            if (favicon.compress(Bitmap.CompressFormat.PNG, 100, stream)) {
+                return stream.toByteArray();
+            }
+        } catch (OutOfMemoryError e) {
+            Log.w(LOGTAG, "Out of memory re-compressing favicon.");
+        }
+
+        Log.w(LOGTAG, "Favicon re-compression failed.");
+        return null;
     }
 
 }