Bug 798223 - Increase optimized favicon dimension in Places database from 16 to 32. r=mak

MozReview-Commit-ID: HTLPQR66HOn

toolkit/components/places/FaviconHelpers.cpp

@@ -321,7 +321,7 @@ OptimizeIconSize(IconData& aIcon,
   // image or a multiresolution .ico file), don't try to store more data than
   // needed.
   nsAutoCString newData, newMimeType;
-  if (aIcon.data.Length() > MAX_ICON_FILESIZE(aFaviconSvc->GetOptimizedIconDimension())) {
+  if (aIcon.data.Length() > MAX_FAVICON_FILESIZE) {
     nsresult rv = aFaviconSvc->OptimizeFaviconImage(TO_INTBUFFER(aIcon.data),
                                                     aIcon.data.Length(),
                                                     aIcon.mimeType,
@@ -613,7 +613,7 @@ AsyncFetchAndSetIconForPage::OnStopRequest(nsIRequest* aRequest,
 
   // If over the maximum size allowed, don't save data to the database to
   // avoid bloating it.
-  if (mIcon.data.Length() > nsIFaviconService::MAX_FAVICON_SIZE) {
+  if (mIcon.data.Length() > nsIFaviconService::MAX_FAVICON_BUFFER_SIZE) {
     return NS_OK;
   }
 

toolkit/components/places/PageIconProtocolHandler.js

@@ -72,7 +72,7 @@ PageIconProtocolHandler.prototype = {
       // we got all the favicon data.
       let pipe = Cc["@mozilla.org/pipe;1"]
                    .createInstance(Ci.nsIPipe);
-      pipe.init(true, true, 0, Ci.nsIFaviconService.MAX_FAVICON_SIZE);
+      pipe.init(true, true, 0, Ci.nsIFaviconService.MAX_FAVICON_BUFFER_SIZE);
 
       // Create our channel.
       let channel = Cc['@mozilla.org/network/input-stream-channel;1']

toolkit/components/places/nsAnnoProtocolHandler.cpp

@@ -336,7 +336,8 @@ nsAnnoProtocolHandler::NewFaviconChannel(nsIURI *aURI, nsIURI *aAnnotationURI,
   nsCOMPtr<nsIOutputStream> outputStream;
   nsresult rv = NS_NewPipe(getter_AddRefs(inputStream),
                            getter_AddRefs(outputStream),
-                           0, nsIFaviconService::MAX_FAVICON_SIZE, true, true);
+                           0, nsIFaviconService::MAX_FAVICON_BUFFER_SIZE,
+                           true, true);
   NS_ENSURE_SUCCESS(rv, GetDefaultIcon(aLoadInfo, _channel));
 
   // Create our channel.  We'll call SetContentType with the right type when

toolkit/components/places/nsFaviconService.cpp

@@ -36,8 +36,8 @@
 #include "imgITools.h"
 #include "imgIContainer.h"
 
-// Default value for mOptimizedIconDimension
-#define OPTIMIZED_FAVICON_DIMENSION 16
+// The target dimension, in pixels, for favicons we optimize.
+#define OPTIMIZED_FAVICON_DIMENSION 32
 
 #define MAX_FAILED_FAVICONS 256
 #define FAVICON_CACHE_REDUCE_COUNT 64
@@ -79,8 +79,7 @@ NS_IMPL_ISUPPORTS_CI(
 )
 
 nsFaviconService::nsFaviconService()
-  : mOptimizedIconDimension(OPTIMIZED_FAVICON_DIMENSION)
-  , mFailedFaviconSerial(0)
+  : mFailedFaviconSerial(0)
   , mFailedFavicons(MAX_FAILED_FAVICONS / 2)
   , mUnassociatedIcons(UNASSOCIATED_FAVICONS_LENGTH)
 {
@@ -105,10 +104,6 @@ nsFaviconService::Init()
   mDB = Database::GetDatabase();
   NS_ENSURE_STATE(mDB);
 
-  mOptimizedIconDimension = Preferences::GetInt(
-    "places.favicons.optimizeToDimension", OPTIMIZED_FAVICON_DIMENSION
-  );
-
   mExpireUnassociatedIconsTimer = do_CreateInstance("@mozilla.org/timer;1");
   NS_ENSURE_STATE(mExpireUnassociatedIconsTimer);
 
@@ -343,11 +338,11 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI,
   // 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.
-  if (aDataLen > MAX_ICON_FILESIZE(mOptimizedIconDimension)) {
+  if (aDataLen > MAX_FAVICON_FILESIZE) {
     rv = OptimizeFaviconImage(aData, aDataLen, aMimeType, iconData->data, iconData->mimeType);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (iconData->data.Length() > nsIFaviconService::MAX_FAVICON_SIZE) {
+    if (iconData->data.Length() > nsIFaviconService::MAX_FAVICON_BUFFER_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.
       mUnassociatedIcons.RemoveEntry(aFaviconURI);
@@ -660,8 +655,8 @@ nsFaviconService::OptimizeFaviconImage(const uint8_t* aData, uint32_t aDataLen,
   // scale and recompress
   nsCOMPtr<nsIInputStream> iconStream;
   rv = imgtool->EncodeScaledImage(container, aNewMimeType,
-                                  mOptimizedIconDimension,
-                                  mOptimizedIconDimension,
+                                  OPTIMIZED_FAVICON_DIMENSION,
+                                  OPTIMIZED_FAVICON_DIMENSION,
                                   EmptyString(),
                                   getter_AddRefs(iconStream));
   NS_ENSURE_SUCCESS(rv, rv);

toolkit/components/places/nsFaviconService.h

@@ -23,9 +23,9 @@
 
 #include "FaviconHelpers.h"
 
-// Most icons will be smaller than this rough estimate of the size of an
-// uncompressed 16x16 RGBA image of the same dimensions.
-#define MAX_ICON_FILESIZE(s) ((uint32_t) s*s*4)
+// Favicons bigger than this (in bytes) will not be stored in the database.  We
+// expect that most 32x32 PNG favicons will be no larger due to compression.
+#define MAX_FAVICON_FILESIZE 3072 /* 3 KiB */
 
 // forward class definitions
 class mozIStorageStatementCallback;
@@ -85,7 +85,6 @@ public:
   nsresult OptimizeFaviconImage(const uint8_t* aData, uint32_t aDataLen,
                                 const nsACString& aMimeType,
                                 nsACString& aNewData, nsACString& aNewMimeType);
-  int32_t GetOptimizedIconDimension() { return mOptimizedIconDimension; }
 
   /**
    * Obtains the favicon data asynchronously.
@@ -135,12 +134,6 @@ private:
    */
   nsCOMPtr<nsIURI> mDefaultIcon;
 
-  // The target dimension, in pixels, for favicons we optimize.
-  // If we find images that are as large or larger than an uncompressed RGBA
-  // image of this size (mOptimizedIconDimension*mOptimizedIconDimension*4),
-  // we will try to optimize it.
-  int32_t mOptimizedIconDimension;
-
   uint32_t mFailedFaviconSerial;
   nsDataHashtable<nsCStringHashKey, uint32_t> mFailedFavicons;
 

toolkit/components/places/nsIFaviconService.idl

@@ -16,10 +16,10 @@ interface nsIFaviconService : nsISupports
   const unsigned long FAVICON_LOAD_NON_PRIVATE = 2;
 
   /**
-   * Favicons bigger than this size in bytes, won't be saved to the database to
-   * avoid bloating it with large image blobs.
+   * The limit in bytes of the size of favicons in memory and passed via the
+   * favicon protocol.
    */
-  const unsigned long MAX_FAVICON_SIZE = 10240;
+  const unsigned long MAX_FAVICON_BUFFER_SIZE = 10240;
 
   /**
    * For a given icon URI, this will return a URI that will result in the image.

toolkit/components/places/nsPlacesExpiration.js

@@ -75,7 +75,7 @@ const DATABASE_TO_MEMORY_PERC = 4;
 const DATABASE_TO_DISK_PERC = 2;
 // Maximum size of the optimal database.  High-end hardware has plenty of
 // memory and disk space, but performances don't grow linearly.
-const DATABASE_MAX_SIZE = 62914560; // 60MiB
+const DATABASE_MAX_SIZE = 73400320; // 70MiB
 // If the physical memory size is bogus, fallback to this.
 const MEMSIZE_FALLBACK_BYTES = 268435456; // 256 MiB
 // If the disk available space is bogus, fallback to this.

toolkit/components/places/tests/favicons/test_favicons_conversions.js

@@ -69,33 +69,35 @@ function run_test() {
 
 add_test(function test_storing_a_normal_16x16_icon() {
   // 16x16 png, 286 bytes.
+  // optimized: no
   checkFaviconDataConversion("favicon-normal16.png", "image/png", 286,
                              false, false, run_next_test);
 });
 
 add_test(function test_storing_a_normal_32x32_icon() {
   // 32x32 png, 344 bytes.
+  // optimized: no
   checkFaviconDataConversion("favicon-normal32.png", "image/png", 344,
                              false, false, run_next_test);
 });
 
-add_test(function test_storing_an_oversize_16x16_icon() {
+add_test(function test_storing_a_big_16x16_icon() {
   //  in: 16x16 ico, 1406 bytes.
-  // out: 16x16 png
+  // optimized: no
   checkFaviconDataConversion("favicon-big16.ico", "image/x-icon", 1406,
-                             true, false, run_next_test);
+                             false, false, run_next_test);
 });
 
 add_test(function test_storing_an_oversize_4x4_icon() {
   //  in: 4x4 jpg, 4751 bytes.
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-big4.jpg", "image/jpeg", 4751,
                              true, false, run_next_test);
 });
 
 add_test(function test_storing_an_oversize_32x32_icon() {
   //  in: 32x32 jpg, 3494 bytes.
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-big32.jpg", "image/jpeg", 3494,
                              true, true, run_next_test);
 });
@@ -104,28 +106,28 @@ add_test(function test_storing_an_oversize_48x48_icon() {
   //  in: 48x48 ico, 56646 bytes.
   // (howstuffworks.com icon, contains 13 icons with sizes from 16x16 to
   // 48x48 in varying depths)
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-big48.ico", "image/x-icon", 56646,
                              true, false, run_next_test);
 });
 
 add_test(function test_storing_an_oversize_64x64_icon() {
   //  in: 64x64 png, 10698 bytes.
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-big64.png", "image/png", 10698,
                              true, false, run_next_test);
 });
 
 add_test(function test_scaling_an_oversize_160x3_icon() {
   //  in: 160x3 jpg, 5095 bytes.
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-scale160x3.jpg", "image/jpeg", 5095,
                              true, false, run_next_test);
 });
 
 add_test(function test_scaling_an_oversize_3x160_icon() {
   //  in: 3x160 jpg, 5059 bytes.
-  // out: 16x16 png
+  // optimized: yes
   checkFaviconDataConversion("favicon-scale3x160.jpg", "image/jpeg", 5059,
                              true, false, run_next_test);
 });

toolkit/components/places/tests/favicons/xpcshell.ini

@@ -3,7 +3,6 @@ head = head_favicons.js
 tail =
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 support-files =
-  expected-favicon-big16.ico.png
   expected-favicon-big32.jpg.png
   expected-favicon-big4.jpg.png
   expected-favicon-big48.ico.png