Bug 1003911 - Part 2: don't write null favicons or thumbnails into the DB. r=margaret

mobile/android/base/Tab.java

@@ -700,12 +700,18 @@ public class Tab {
     }
 
     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
         }

mobile/android/base/favicons/LoadFaviconTask.java

@@ -97,6 +97,10 @@ public class LoadFaviconTask extends UiAsyncTask<Void, Void, Bitmap> {
 
     // Runs in background thread
     private void saveFaviconToDb(final byte[] encodedFavicon) {
+        if (encodedFavicon == null) {
+            return;
+        }
+
         if ((flags & FLAG_PERSIST) == 0) {
             return;
         }
@@ -376,6 +380,9 @@ public class LoadFaviconTask extends UiAsyncTask<Void, Void, Bitmap> {
         }
 
         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);
         }

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

@@ -34,9 +34,9 @@ public class LoadFaviconResult {
 
     /**
      * 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.
@@ -47,28 +47,30 @@ public class LoadFaviconResult {
             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;
     }
 
 }