Bug 890515 - Purge 0-byte favicons and thumbnails from the database. r=rnewman, a=bajaj
authorBrian Nicholson <bnicholson@mozilla.com>
Wed, 24 Jul 2013 22:08:42 -0700
changeset 148124 79d58dbe9336c623cd4d04469302dbcf8da70964
parent 148123 53cf02a9bf32d81d8e41365a8d6587eec274913f
child 148125 8682482c7e2381381e7776b6a31d4df342b7f435
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, bajaj
bugs890515
milestone24.0a2
Bug 890515 - Purge 0-byte favicons and thumbnails from the database. r=rnewman, a=bajaj
mobile/android/base/AwesomeBar.java
mobile/android/base/awesomebar/AllPagesTab.java
mobile/android/base/awesomebar/BookmarksTab.java
mobile/android/base/awesomebar/HistoryTab.java
mobile/android/base/db/BrowserProvider.java
mobile/android/base/db/LocalBrowserDB.java
mobile/android/base/widget/TopSitesView.java
--- a/mobile/android/base/AwesomeBar.java
+++ b/mobile/android/base/AwesomeBar.java
@@ -649,17 +649,17 @@ public class AwesomeBar extends GeckoAct
             }
             case R.id.add_to_launcher: {
                 if (url == null) {
                     Log.e(LOGTAG, "Can't add to home screen because URL is null");
                     break;
                 }
 
                 Bitmap bitmap = null;
-                if (b != null && b.length > 0) {
+                if (b != null) {
                     bitmap = BitmapUtils.decodeByteArray(b);
                 }
 
                 String shortcutTitle = TextUtils.isEmpty(title) ? url.replaceAll("^([a-z]+://)?(www\\.)?", "") : title;
                 GeckoAppShell.createShortcut(shortcutTitle, url, bitmap, "");
                 break;
             }
             case R.id.share: {
--- a/mobile/android/base/awesomebar/AllPagesTab.java
+++ b/mobile/android/base/awesomebar/AllPagesTab.java
@@ -340,18 +340,21 @@ public class AllPagesTab extends Awesome
 
             final String url = mCursor.getString(mCursor.getColumnIndexOrThrow(URLColumns.URL));
 
             Bitmap bitmap = Favicons.getInstance().getFaviconFromMemCache(url);
             byte[] favicon = null;
 
             if (bitmap != null) {
                 ByteArrayOutputStream stream = new ByteArrayOutputStream();
-                bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream);
-                favicon = stream.toByteArray();
+                if (bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream)) {
+                    favicon = stream.toByteArray();
+                } else {
+                    Log.w(LOGTAG, "Favicon compression failed.");
+                }
             }
 
             return new ContextMenuSubject(id, url, favicon,
                                           mCursor.getString(mCursor.getColumnIndexOrThrow(URLColumns.TITLE)),
                                           keyword,
                                           mCursor.getInt(mCursor.getColumnIndexOrThrow(Combined.DISPLAY)));
         }
     }
@@ -869,17 +872,17 @@ public class AllPagesTab extends Awesome
     public void storeFaviconsInMemCache(Cursor c) {
         try {
             if (c == null || !c.moveToFirst())
                 return;
 
             do {
                 final String url = c.getString(c.getColumnIndexOrThrow(Combined.URL));
                 final byte[] b = c.getBlob(c.getColumnIndexOrThrow(Combined.FAVICON));
-                if (b == null || b.length == 0)
+                if (b == null)
                     continue;
 
                 Bitmap favicon = BitmapUtils.decodeByteArray(b);
                 if (favicon == null)
                     continue;
 
                 favicon = Favicons.getInstance().scaleImage(favicon);
                 Favicons.getInstance().putFaviconInMemCache(url, favicon);
--- a/mobile/android/base/awesomebar/BookmarksTab.java
+++ b/mobile/android/base/awesomebar/BookmarksTab.java
@@ -338,17 +338,17 @@ public class BookmarksTab extends Awesom
                 throw new IllegalStateException("Couldn't move cursor to position " + position);
 
             if (viewType == VIEW_TYPE_ITEM) {
                 updateTitle(viewHolder.titleView, cursor);
                 updateUrl(viewHolder, cursor);
 
                 byte[] b = cursor.getBlob(cursor.getColumnIndexOrThrow(URLColumns.FAVICON));
                 Bitmap favicon = null;
-                if (b != null && b.length > 0) {
+                if (b != null) {
                     Bitmap bitmap = BitmapUtils.decodeByteArray(b);
                     if (bitmap != null) {
                         favicon = Favicons.getInstance().scaleImage(bitmap);
                     }
                 }
                 String url = cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.URL));
                 updateFavicon(viewHolder.faviconView, favicon, url);
             } else {
--- a/mobile/android/base/awesomebar/HistoryTab.java
+++ b/mobile/android/base/awesomebar/HistoryTab.java
@@ -186,17 +186,17 @@ public class HistoryTab extends AwesomeB
             String url = (String) historyItem.get(URLColumns.URL);
 
             updateTitle(viewHolder.titleView, title, url);
             updateUrl(viewHolder, url);
 
             byte[] b = (byte[]) historyItem.get(URLColumns.FAVICON);
             Bitmap favicon = null;
 
-            if (b != null && b.length > 0) {
+            if (b != null) {
                 Bitmap bitmap = BitmapUtils.decodeByteArray(b);
                 if (bitmap != null) {
                     favicon = Favicons.getInstance().scaleImage(bitmap);
                 }
             }
             updateFavicon(viewHolder.faviconView, favicon, url);
 
             Integer bookmarkId = (Integer) historyItem.get(Combined.BOOKMARK_ID);
--- a/mobile/android/base/db/BrowserProvider.java
+++ b/mobile/android/base/db/BrowserProvider.java
@@ -65,17 +65,17 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public class BrowserProvider extends ContentProvider {
     private static final String LOGTAG = "GeckoBrowserProvider";
     private Context mContext;
 
     static final String DATABASE_NAME = "browser.db";
 
-    static final int DATABASE_VERSION = 16;
+    static final int DATABASE_VERSION = 17;
 
     // Maximum age of deleted records to be cleaned up (20 days in ms)
     static final long MAX_AGE_OF_DELETED_RECORDS = 86400000 * 20;
 
     // Number of records marked as deleted to be removed
     static final long DELETED_RECORDS_PURGE_LIMIT = 5;
 
     // How many records to reposition in a single query.
@@ -1204,21 +1204,28 @@ public class BrowserProvider extends Con
             bookmarkValues.put(Bookmarks.URL, url);
             bookmarkValues.put(Bookmarks.GUID, Utils.generateGuid());
             bookmarkValues.put(Bookmarks.POSITION, pos);
             db.insertOrThrow(TABLE_BOOKMARKS, Bookmarks.TITLE, bookmarkValues);
         }
 
         private void createFavicon(SQLiteDatabase db, String url, Bitmap icon) {
             ByteArrayOutputStream stream = new ByteArrayOutputStream();
-            icon.compress(Bitmap.CompressFormat.PNG, 100, stream);
 
             ContentValues iconValues = new ContentValues();
-            iconValues.put(Favicons.DATA, stream.toByteArray());
             iconValues.put(Favicons.PAGE_URL, url);
+
+            byte[] data = null;
+            if (icon.compress(Bitmap.CompressFormat.PNG, 100, stream)) {
+                data = stream.toByteArray();
+            } else {
+                Log.w(LOGTAG, "Favicon compression failed.");
+            }
+            iconValues.put(Favicons.DATA, data);
+
             insertFavicon(db, iconValues);
         }
 
         private Bitmap getDefaultFaviconFromPath(String name) {
             Class<?> stringClass = R.string.class;
             try {
                 // Look for a drawable with the id R.drawable.bookmarkdefaults_favicon_*
                 Field faviconField = stringClass.getField(name.replace("_title_", "_favicon_"));
@@ -1784,16 +1791,28 @@ public class BrowserProvider extends Con
 
         private void upgradeDatabaseFrom15to16(SQLiteDatabase db) {
             db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
             db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
 
             createCombinedViewOn16(db);
         }
 
+        private void upgradeDatabaseFrom16to17(SQLiteDatabase db) {
+            // Purge any 0-byte favicons/thumbnails
+            try {
+                db.execSQL("DELETE FROM " + TABLE_FAVICONS +
+                        " WHERE length(" + Favicons.DATA + ") = 0");
+                db.execSQL("DELETE FROM " + TABLE_THUMBNAILS +
+                        " WHERE length(" + Thumbnails.DATA + ") = 0");
+            } catch (SQLException e) {
+                Log.e(LOGTAG, "Error purging invalid favicons or thumbnails", e);
+            }
+        }
+
         @Override
         public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
             debug("Upgrading browser.db: " + db.getPath() + " from " +
                     oldVersion + " to " + newVersion);
 
             // We have to do incremental upgrades until we reach the current
             // database schema version.
             for (int v = oldVersion + 1; v <= newVersion; v++) {
@@ -1852,16 +1871,20 @@ public class BrowserProvider extends Con
 
                     case 15:
                         upgradeDatabaseFrom14to15(db);
                         break;
 
                     case 16:
                         upgradeDatabaseFrom15to16(db);
                         break;
+
+                    case 17:
+                        upgradeDatabaseFrom16to17(db);
+                        break;
                 }
             }
 
             // If an upgrade after 12->13 fails, the entire upgrade is rolled
             // back, but we can't undo the deletion of favicon_urls.db if we
             // delete this in step 13; therefore, we wait until all steps are
             // complete before removing it.
             if (oldVersion < 13 && newVersion >= 13
@@ -3060,21 +3083,22 @@ public class BrowserProvider extends Con
 
     long insertFavicon(Uri uri, ContentValues values) {
         return insertFavicon(getWritableDatabase(uri), values);
     }
 
     long insertFavicon(SQLiteDatabase db, ContentValues values) {
         String faviconUrl = values.getAsString(Favicons.URL);
         String pageUrl = null;
-        Cursor cursor = null;
         long faviconId;
 
         trace("Inserting favicon for URL: " + faviconUrl);
 
+        stripEmptyByteArray(values, Favicons.DATA);
+
         // Extract the page URL from the ContentValues
         if (values.containsKey(Favicons.PAGE_URL)) {
             pageUrl = values.getAsString(Favicons.PAGE_URL);
             values.remove(Favicons.PAGE_URL);
         }
 
         long now = System.currentTimeMillis();
         values.put(Favicons.DATE_CREATED, now);
@@ -3107,16 +3131,18 @@ public class BrowserProvider extends Con
         int updated = 0;
         final SQLiteDatabase db = getWritableDatabase(uri);
         Cursor cursor = null;
         Long faviconId = null;
         long now = System.currentTimeMillis();
 
         trace("Updating favicon for URL: " + faviconUrl);
 
+        stripEmptyByteArray(values, Favicons.DATA);
+
         // Extract the page URL from the ContentValues
         if (values.containsKey(Favicons.PAGE_URL)) {
             pageUrl = values.getAsString(Favicons.PAGE_URL);
             values.remove(Favicons.PAGE_URL);
         }
 
         values.put(Favicons.DATE_MODIFIED, now);
 
@@ -3159,16 +3185,18 @@ public class BrowserProvider extends Con
     }
 
     long insertThumbnail(Uri uri, ContentValues values) {
         String url = values.getAsString(Thumbnails.URL);
         final SQLiteDatabase db = getWritableDatabase(uri);
 
         trace("Inserting thumbnail for URL: " + url);
 
+        stripEmptyByteArray(values, Thumbnails.DATA);
+
         return db.insertOrThrow(TABLE_THUMBNAILS, null, values);
     }
 
     int updateOrInsertThumbnail(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         return updateThumbnail(uri, values, selection, selectionArgs,
                 true /* insert if needed */);
     }
@@ -3180,29 +3208,46 @@ public class BrowserProvider extends Con
     }
 
     int updateThumbnail(Uri uri, ContentValues values, String selection,
             String[] selectionArgs, boolean insertIfNeeded) {
         String url = values.getAsString(Thumbnails.URL);
         int updated = 0;
         final SQLiteDatabase db = getWritableDatabase(uri);
 
+        stripEmptyByteArray(values, Thumbnails.DATA);
+
         trace("Updating thumbnail for URL: " + url);
 
         updated = db.update(TABLE_THUMBNAILS, values, selection, selectionArgs);
 
         if (updated == 0 && insertIfNeeded) {
             trace("No update, inserting thumbnail for URL: " + url);
             db.insert(TABLE_THUMBNAILS, null, values);
             updated = 1;
         }
 
         return updated;
     }
 
+    /**
+     * Verifies that 0-byte arrays aren't added as favicon or thumbnail data.
+     * @param values        ContentValues of query
+     * @param columnName    Name of data column to verify
+     */
+    private void stripEmptyByteArray(ContentValues values, String columnName) {
+        if (values.containsKey(columnName)) {
+            byte[] data = values.getAsByteArray(columnName);
+            if (data == null || data.length == 0) {
+                Log.w(LOGTAG, "Tried to insert an empty or non-byte-array image. Ignoring.");
+                values.putNull(columnName);
+            }
+        }
+    }
+
     int deleteHistory(Uri uri, String selection, String[] selectionArgs) {
         debug("Deleting history entry for URI: " + uri);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
 
         if (isCallerSync(uri)) {
             return db.delete(TABLE_HISTORY, selection, selectionArgs);
         }
--- a/mobile/android/base/db/LocalBrowserDB.java
+++ b/mobile/android/base/db/LocalBrowserDB.java
@@ -726,18 +726,19 @@ public class LocalBrowserDB implements B
             c.close();
             return null;
         }
 
         int faviconIndex = c.getColumnIndexOrThrow(Combined.FAVICON);
         byte[] b = c.getBlob(faviconIndex);
         c.close();
 
-        if (b == null || b.length == 0)
+        if (b == null) {
             return null;
+        }
 
         return BitmapUtils.decodeByteArray(b);
     }
 
     @Override
     public String getFaviconUrlForHistoryUrl(ContentResolver cr, String uri) {
         Cursor c = cr.query(mHistoryUriWithProfile,
                             new String[] { History.FAVICON_URL },
@@ -776,24 +777,29 @@ public class LocalBrowserDB implements B
                         selection.toString(),
                         selectionArgs,
                         null);
     }
 
     @Override
     public void updateFaviconForUrl(ContentResolver cr, String pageUri,
             Bitmap favicon, String faviconUri) {
-        ByteArrayOutputStream stream = new ByteArrayOutputStream();
-        favicon.compress(Bitmap.CompressFormat.PNG, 100, stream);
-
         ContentValues values = new ContentValues();
         values.put(Favicons.URL, faviconUri);
-        values.put(Favicons.DATA, stream.toByteArray());
         values.put(Favicons.PAGE_URL, pageUri);
 
+        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.");
+        }
+        values.put(Favicons.DATA, data);
+
         // Update or insert
         Uri faviconsUri = getAllFaviconsUri().buildUpon().
                 appendQueryParameter(BrowserContract.PARAM_INSERT_IF_NEEDED, "true").build();
 
         int updated = cr.update(faviconsUri,
                                 values,
                                 Favicons.URL + " = ?",
                                 new String[] { faviconUri });
@@ -802,21 +808,26 @@ public class LocalBrowserDB implements B
             cr.insert(mFaviconsUriWithProfile, values);
     }
 
     @Override
     public void updateThumbnailForUrl(ContentResolver cr, String uri,
             BitmapDrawable thumbnail) {
         Bitmap bitmap = thumbnail.getBitmap();
 
+        byte[] data = null;
         ByteArrayOutputStream stream = new ByteArrayOutputStream();
-        bitmap.compress(Bitmap.CompressFormat.PNG, 0, stream);
+        if (bitmap.compress(Bitmap.CompressFormat.PNG, 0, stream)) {
+            data = stream.toByteArray();
+        } else {
+            Log.w(LOGTAG, "Favicon compression failed.");
+        }
 
         ContentValues values = new ContentValues();
-        values.put(Thumbnails.DATA, stream.toByteArray());
+        values.put(Thumbnails.DATA, data);
         values.put(Thumbnails.URL, uri);
 
         int updated = cr.update(mThumbnailsUriWithProfile,
                                 values,
                                 Thumbnails.URL + " = ?",
                                 new String[] { uri });
 
         if (updated == 0)
--- a/mobile/android/base/widget/TopSitesView.java
+++ b/mobile/android/base/widget/TopSitesView.java
@@ -317,17 +317,18 @@ public class TopSitesView extends GridVi
                 return thumbnails;
 
             do {
                 final String url = c.getString(c.getColumnIndexOrThrow(Thumbnails.URL));
                 final byte[] b = c.getBlob(c.getColumnIndexOrThrow(Thumbnails.DATA));
                 if (b == null)
                     continue;
 
-                Bitmap thumbnail = BitmapUtils.decodeByteArray(b);
+                Bitmap thumbnail = null;
+                thumbnail = BitmapUtils.decodeByteArray(b);
                 if (thumbnail == null)
                     continue;
 
                 thumbnails.put(url, thumbnail);
             } while (c.moveToNext());
         } finally {
             if (c != null)
                 c.close();
@@ -683,17 +684,17 @@ public class TopSitesView extends GridVi
 
                         Cursor c = BrowserDB.getThumbnailsForUrls(resolver, urls);
                         if (c == null || !c.moveToFirst()) {
                             return null;
                         }
 
                         final byte[] b = c.getBlob(c.getColumnIndexOrThrow(Thumbnails.DATA));
                         Bitmap bitmap = null;
-                        if (b != null && b.length > 0) {
+                        if (b != null) {
                             bitmap = BitmapUtils.decodeByteArray(b);
                         }
                         c.close();
 
                         return bitmap;
                     }
 
                     @Override