Bug 442731 - GIF favicons are not resampled in places.sqlite (large icons are stored), r=dietrich, pavlov

browser/components/places/src/nsPlacesImportExportService.cpp

@@ -1313,9 +1313,10 @@ BookmarkContentSink::SetFaviconForURI(nsIURI* aPageURI, nsIURI* aIconURI,
     serialNumber++;
   }
 
-  // save in service
-  rv = faviconService->SetFaviconDataFromDataURL(faviconURI, aData, 0);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // save the favicon data
+  // This could fail if the favicon is bigger than defined limit, in such a
+  // case data will not be saved to the db but we will still continue.
+  (void) faviconService->SetFaviconDataFromDataURL(faviconURI, aData, 0);
 
   rv = faviconService->SetFaviconUrlForPage(aPageURI, faviconURI);
   NS_ENSURE_SUCCESS(rv, rv);

modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp

@@ -174,7 +174,7 @@ NS_IMETHODIMP nsGIFDecoder2::Close()
 /* void flush (); */
 NS_IMETHODIMP nsGIFDecoder2::Flush()
 {
-    return NS_ERROR_NOT_IMPLEMENTED;
+    return NS_OK;
 }
 
 //******************************************************************************

toolkit/components/places/public/nsIFaviconService.idl

@@ -130,6 +130,11 @@ interface nsIFaviconService : nsISupports
    * notifications is very intensive. Those pages will keep the old icon
    * until they have been refreshed by other means.
    *
+   * This function tries to optimize the favicon size, if it is bigger
+   * than defined limit we will try to convert it to a 16x16 png image. If the
+   * conversion fails and favicon is bigger than our max accepted favicon size
+   * we will fail and the favicon won't be saved.
+   *
    * @param aFaviconURI
    *        URI of the favicon whose data is being set.
    * @param aData
@@ -142,6 +147,8 @@ interface nsIFaviconService : nsISupports
    * @param aExpiration
    *        Time in microseconds since the epoch when this favicon expires.
    *        Until this time, we won't try to load it again.
+   * @throws NS_ERROR_FAILURE
+   *         Thrown if the favicon is overbloated and won't be saved to the db.
    */
   void setFaviconData(in nsIURI aFaviconURI,
                       [const,array,size_is(aDataLen)] in octet aData,
@@ -152,6 +159,11 @@ interface nsIFaviconService : nsISupports
    * Stores the data of a given favicon. The data is provided by a string
    * containing a data URL.
    *
+   * This function tries to optimize the favicon size, if it is bigger
+   * than defined limit we will try to convert it to a 16x16 png image. If the
+   * conversion fails and favicon is bigger than our max accepted favicon size
+   * we will fail and the favicon won't be saved.
+   *
    * @see setFaviconData
    *
    * @param aFaviconURI
@@ -162,6 +174,8 @@ interface nsIFaviconService : nsISupports
    * @param aExpiration
    *        Time in microseconds since the epoch when this favicon expires.
    *        Until this time, we won't try to load it again.
+   * @throws NS_ERROR_FAILURE
+   *         Thrown if the favicon is overbloated and won't be saved to the db.
    */
   void setFaviconDataFromDataURL(in nsIURI aFaviconURI, in AString aDataURL,
                                  in PRTime aExpiration);

toolkit/components/places/src/nsFaviconService.cpp

@@ -76,6 +76,15 @@
 
 #define CONTENT_SNIFFING_SERVICES "content-sniffing-services"
 
+// If favicon is bigger than this size we will try to optimize it into a
+// 16x16 png. An uncompressed 16x16 RGBA image is 1024 bytes, and almost all
+// sensible 16x16 icons are under 1024 bytes.
+#define OPTIMIZED_FAVICON_SIZE 1024
+
+// Favicons bigger than this size should not be saved to the db to avoid
+// bloating it with large image blobs.
+// This still allows us to accept a favicon even if we cannot optimize it.
+#define MAX_FAVICON_SIZE 10240
 
 class FaviconLoadListener : public nsIStreamListener,
                             public nsIInterfaceRequestor,
@@ -592,15 +601,19 @@ nsFaviconService::SetFaviconData(nsIURI* aFaviconURI, const PRUint8* aData,
 
   // If the page provided a large image for the favicon (eg, a highres image
   // or a multiresolution .ico file), we don't want to store more data than
-  // needed. An uncompressed 16x16 RGBA image is 1024 bytes, and almost all
-  // sensible 16x16 icons are under 1024 bytes.
-  if (aDataLen > 1024) {
+  // needed.
+  if (aDataLen > OPTIMIZED_FAVICON_SIZE) {
     rv = OptimizeFaviconImage(aData, aDataLen, aMimeType, newData, newMimeType);
     if (NS_SUCCEEDED(rv) && newData.Length() < aDataLen) {
       data = reinterpret_cast<PRUint8*>(const_cast<char*>(newData.get())),
       dataLen = newData.Length();
       mimeType = &newMimeType;
     }
+    else if (aDataLen > MAX_FAVICON_SIZE) {
+      // We cannot optimize this favicon size and we are over the maximum size
+      // allowed, so we will not save data to the db to avoid bloating it.
+      return NS_ERROR_FAILURE;
+    }
   }
 
   mozIStorageStatement* statement;
@@ -1087,10 +1100,11 @@ FaviconLoadListener::OnStopRequest(nsIRequest *aRequest, nsISupports *aContext,
                       (PRInt64)(24 * 60 * 60) * (PRInt64)PR_USEC_PER_SEC;
 
   // save the favicon data
-  rv = mFaviconService->SetFaviconData(mFaviconURI,
+  // This could fail if the favicon is bigger than defined limit, in such a
+  // case data will not be saved to the db but we will still continue.
+  (void) mFaviconService->SetFaviconData(mFaviconURI,
                reinterpret_cast<PRUint8*>(const_cast<char*>(mData.get())),
                mData.Length(), mimeType, expiration);
-  NS_ENSURE_SUCCESS(rv, rv);
 
   // set the favicon for the page
   PRBool hasData;

toolkit/components/places/tests/queries/head_queries.js

@@ -213,9 +213,11 @@ function populateDB(aArray) {
       if (qdata.isFavicon) {
         // Not planning on doing deep testing of favIcon service so these two
         // calls should be sufficient to get favicons into the database
-        faviconsvc.setFaviconData(uri(qdata.faviconURI), qdata.favicon,
-                                  qdata.faviconLen, qdata.faviconMimeType,
-                                  qdata.faviconExpiration);
+        try {
+          faviconsvc.setFaviconData(uri(qdata.faviconURI), qdata.favicon,
+                                    qdata.faviconLen, qdata.faviconMimeType,
+                                    qdata.faviconExpiration);
+        } catch (ex) {}
         faviconsvc.setFaviconUrlForPage(uri(qdata.uri), uri(qdata.faviconURI));
       }
 

toolkit/components/places/tests/unit/test_favicons.js

@@ -67,14 +67,15 @@ function readFileData(aFile) {
  */
 function setAndGetFaviconData(aFilename, aData, aMimeType) {
   var iconURI = uri("http://places.test/" + aFilename);
-
-  iconsvc.setFaviconData(iconURI,
-                         aData, aData.length, aMimeType,
-                         Number.MAX_VALUE);
-
+  try {
+    iconsvc.setFaviconData(iconURI,
+                           aData, aData.length, aMimeType,
+                           Number.MAX_VALUE);
+  } catch (ex) {}
   var dataURL = iconsvc.getFaviconDataAsDataURL(iconURI);
-  iconsvc.setFaviconDataFromDataURL(iconURI, dataURL, Number.MAX_VALUE);
-
+  try {
+    iconsvc.setFaviconDataFromDataURL(iconURI, dataURL, Number.MAX_VALUE);
+  } catch (ex) {}
   var mimeTypeOutparam = {};
 
   var outData = iconsvc.getFaviconData(iconURI,
@@ -365,21 +366,27 @@ histsvc.addVisit(page3URI, Date.now() * 1000, null,
                  histsvc.TRANSITION_TYPED, false, 0);
 
 // set first page icon
+try {
+  iconsvc.setFaviconData(icon1URI, icon1Data, icon1Data.length,
+                         icon1MimeType, Number.MAX_VALUE);
+} catch (ex) {}
 iconsvc.setFaviconUrlForPage(page1URI, icon1URI);
-iconsvc.setFaviconData(icon1URI, icon1Data, icon1Data.length,
-                       icon1MimeType, Number.MAX_VALUE);
 var savedIcon1URI = iconsvc.getFaviconForPage(page1URI);
 
 // set second page icon
+try {
+  iconsvc.setFaviconData(icon2URI, icon2Data, icon2Data.length,
+                         icon2MimeType, Number.MAX_VALUE);
+} catch (ex) {}
 iconsvc.setFaviconUrlForPage(page2URI, icon2URI);
-iconsvc.setFaviconData(icon2URI, icon2Data, icon2Data.length,
-                       icon2MimeType, Number.MAX_VALUE);
 var savedIcon2URI = iconsvc.getFaviconForPage(page2URI);
 
 // set third page icon as the same as first page one
+try {
+  iconsvc.setFaviconData(icon1URI, icon1Data, icon1Data.length,
+                         icon1MimeType, Number.MAX_VALUE);
+} catch (ex) {}
 iconsvc.setFaviconUrlForPage(page3URI, icon1URI);
-iconsvc.setFaviconData(icon1URI, icon1Data, icon1Data.length,
-                       icon1MimeType, Number.MAX_VALUE);
 var savedIcon3URI = iconsvc.getFaviconForPage(page3URI);
 
 // check first page icon