diff --git a/browser/components/places/src/nsPlacesImportExportService.cpp b/browser/components/places/src/nsPlacesImportExportService.cpp index 55bf999f0248..97a44ce2af5f 100644 --- a/browser/components/places/src/nsPlacesImportExportService.cpp +++ b/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); diff --git a/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp b/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp index a5ab12eff0a8..4f1db17a85ed 100644 --- a/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp +++ b/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; } //****************************************************************************** diff --git a/toolkit/components/places/public/nsIFaviconService.idl b/toolkit/components/places/public/nsIFaviconService.idl index c22a6de060d4..60d8edd13490 100644 --- a/toolkit/components/places/public/nsIFaviconService.idl +++ b/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); diff --git a/toolkit/components/places/src/nsFaviconService.cpp b/toolkit/components/places/src/nsFaviconService.cpp index 8b7b61ddff1e..def447f615a7 100644 --- a/toolkit/components/places/src/nsFaviconService.cpp +++ b/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(const_cast(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(const_cast(mData.get())), mData.Length(), mimeType, expiration); - NS_ENSURE_SUCCESS(rv, rv); // set the favicon for the page PRBool hasData; diff --git a/toolkit/components/places/tests/queries/head_queries.js b/toolkit/components/places/tests/queries/head_queries.js index 72f6da27e5ce..bfc4667ab121 100644 --- a/toolkit/components/places/tests/queries/head_queries.js +++ b/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)); } diff --git a/toolkit/components/places/tests/unit/test_favicons.js b/toolkit/components/places/tests/unit/test_favicons.js index 9d70b46fb21b..ae69b8d10bf9 100644 --- a/toolkit/components/places/tests/unit/test_favicons.js +++ b/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