diff --git a/browser/components/customizableui/CustomizableWidgets.jsm b/browser/components/customizableui/CustomizableWidgets.jsm index a057749c0c21..6b0ebcfe7f45 100644 --- a/browser/components/customizableui/CustomizableWidgets.jsm +++ b/browser/components/customizableui/CustomizableWidgets.jsm @@ -218,7 +218,6 @@ const CustomizableWidgets = [ while ((row = aResultSet.getNextRow())) { let uri = row.getResultByIndex(1); let title = row.getResultByIndex(2); - let icon = row.getResultByIndex(6); let item = doc.createElementNS(kNSXUL, "toolbarbutton"); item.setAttribute("label", title || uri); @@ -226,10 +225,7 @@ const CustomizableWidgets = [ item.setAttribute("class", "subviewbutton"); item.addEventListener("command", onItemCommand); item.addEventListener("click", onItemCommand); - if (icon) { - let iconURL = "moz-anno:favicon:" + icon; - item.setAttribute("image", iconURL); - } + item.setAttribute("image", "page-icon:" + uri); fragment.appendChild(item); } items.appendChild(fragment); diff --git a/browser/components/places/content/browserPlacesViews.js b/browser/components/places/content/browserPlacesViews.js index 7dea0891dcd8..360857e77a62 100644 --- a/browser/components/places/content/browserPlacesViews.js +++ b/browser/components/places/content/browserPlacesViews.js @@ -508,14 +508,10 @@ PlacesViewBase.prototype = { return; // Here we need the . - if (elt.localName == "menupopup") + if (elt.localName == "menupopup") { elt = elt.parentNode; - - let icon = aPlacesNode.icon; - if (!icon) - elt.removeAttribute("image"); - else if (icon != elt.getAttribute("image")) - elt.setAttribute("image", icon); + } + elt.setAttribute("image", aPlacesNode.icon); }, nodeAnnotationChanged: diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 192a2dfbf3f0..ed8b51bc5695 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -9603,7 +9603,7 @@ public: NS_IMETHOD OnComplete(nsIURI* aFaviconURI, uint32_t aDataLen, - const uint8_t* aData, const nsACString& aMimeType) override + const uint8_t* aData, const nsACString& aMimeType, uint16_t aWidth) override { // Continue only if there is an associated favicon. if (!aFaviconURI) { @@ -9662,7 +9662,7 @@ nsDocShell::CopyFavicon(nsIURI* aOldURI, new nsCopyFaviconCallback(favSvc, aNewURI, aLoadingPrincipal, aInPrivateBrowsing); - favSvc->GetFaviconURLForPage(aOldURI, callback); + favSvc->GetFaviconURLForPage(aOldURI, callback, 0); } #endif } diff --git a/testing/firefox-ui/tests/functional/locationbar/manifest.ini b/testing/firefox-ui/tests/functional/locationbar/manifest.ini index 72adfd767f2d..1cff722d39fb 100644 --- a/testing/firefox-ui/tests/functional/locationbar/manifest.ini +++ b/testing/firefox-ui/tests/functional/locationbar/manifest.ini @@ -4,6 +4,5 @@ tags = local [test_access_locationbar.py] disabled = Bug 1168727 - Timeout when opening auto-complete popup [test_escape_autocomplete.py] -[test_favicon_in_autocomplete.py] [test_suggest_bookmarks.py] diff --git a/testing/firefox-ui/tests/functional/locationbar/test_favicon_in_autocomplete.py b/testing/firefox-ui/tests/functional/locationbar/test_favicon_in_autocomplete.py deleted file mode 100644 index 6e8a5f6b1266..000000000000 --- a/testing/firefox-ui/tests/functional/locationbar/test_favicon_in_autocomplete.py +++ /dev/null @@ -1,62 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -from firefox_puppeteer import PuppeteerMixin -from marionette_driver import Wait -from marionette_harness import MarionetteTestCase - - -class TestFaviconInAutocomplete(PuppeteerMixin, MarionetteTestCase): - - PREF_SUGGEST_SEARCHES = 'browser.urlbar.suggest.searches' - PREF_SUGGEST_BOOKMARK = 'browser.urlbar.suggest.bookmark' - - def setUp(self): - super(TestFaviconInAutocomplete, self).setUp() - - # Disable suggestions for searches and bookmarks to get results only for history - self.marionette.set_pref(self.PREF_SUGGEST_SEARCHES, False) - self.marionette.set_pref(self.PREF_SUGGEST_BOOKMARK, False) - - self.puppeteer.places.remove_all_history() - - self.test_urls = [self.marionette.absolute_url('layout/mozilla.html')] - - self.test_string = 'mozilla' - self.test_favicon = 'mozilla_favicon.ico' - - self.autocomplete_results = self.browser.navbar.locationbar.autocomplete_results - - def tearDown(self): - try: - self.autocomplete_results.close(force=True) - self.marionette.clear_pref(self.PREF_SUGGEST_SEARCHES) - self.marionette.clear_pref(self.PREF_SUGGEST_BOOKMARK) - finally: - super(TestFaviconInAutocomplete, self).tearDown() - - def test_favicon_in_autocomplete(self): - # Open the test page - def load_urls(): - with self.marionette.using_context('content'): - self.marionette.navigate(self.test_urls[0]) - self.puppeteer.places.wait_for_visited(self.test_urls, load_urls) - - locationbar = self.browser.navbar.locationbar - - # Clear the location bar, type the test string, check that autocomplete list opens - locationbar.clear() - locationbar.urlbar.send_keys(self.test_string) - self.assertEqual(locationbar.value, self.test_string) - Wait(self.marionette).until(lambda _: self.autocomplete_results.is_complete) - - result = self.autocomplete_results.visible_results[1] - - result_icon = self.marionette.execute_script(""" - return arguments[0].image; - """, script_args=[result]) - - self.assertIn(self.test_favicon, result_icon) - - self.autocomplete_results.close() diff --git a/toolkit/components/alerts/nsAlertsService.cpp b/toolkit/components/alerts/nsAlertsService.cpp index dd67ad9831a9..600535b36249 100644 --- a/toolkit/components/alerts/nsAlertsService.cpp +++ b/toolkit/components/alerts/nsAlertsService.cpp @@ -49,7 +49,7 @@ public: NS_IMETHOD OnComplete(nsIURI *aIconURI, uint32_t aIconSize, const uint8_t *aIconData, - const nsACString &aMimeType) override + const nsACString &aMimeType, uint16_t aWidth) override { nsresult rv = NS_ERROR_FAILURE; if (aIconSize > 0) { @@ -111,9 +111,9 @@ ShowWithIconBackend(nsIAlertsService* aBackend, nsIAlertNotification* aAlert, nsCOMPtr callback = new IconCallback(aBackend, aAlert, aAlertListener); if (alertsIconData) { - return favicons->GetFaviconDataForPage(uri, callback); + return favicons->GetFaviconDataForPage(uri, callback, 0); } - return favicons->GetFaviconURLForPage(uri, callback); + return favicons->GetFaviconURLForPage(uri, callback, 0); #else return NS_ERROR_NOT_IMPLEMENTED; #endif // !MOZ_PLACES diff --git a/toolkit/components/places/FaviconHelpers.cpp b/toolkit/components/places/FaviconHelpers.cpp index 5082589e6390..0eb5f4c6319e 100644 --- a/toolkit/components/places/FaviconHelpers.cpp +++ b/toolkit/components/places/FaviconHelpers.cpp @@ -23,10 +23,10 @@ #include "nsISupportsPriority.h" #include "nsContentUtils.h" #include +#include #include "mozilla/gfx/2D.h" #include "imgIContainer.h" #include "ImageOps.h" -#include "imgLoader.h" #include "imgIEncoder.h" using namespace mozilla::places; @@ -38,9 +38,9 @@ namespace places { namespace { /** - * Fetches information on a page from the Places database. + * Fetches information about a page from the database. * - * @param aDBConn + * @param aDB * Database connection to history tables. * @param _page * Page that should be fetched. @@ -55,7 +55,7 @@ FetchPageInfo(const RefPtr& aDB, // This query finds the bookmarked uri we want to set the icon for, // walking up to two redirect levels. nsCString query = nsPrintfCString( - "SELECT h.id, h.favicon_id, h.guid, ( " + "SELECT h.id, h.guid, ( " "SELECT h.url FROM moz_bookmarks b WHERE b.fk = h.id " "UNION ALL " // Union not directly bookmarked pages. "SELECT url FROM moz_places WHERE id = ( " @@ -69,7 +69,9 @@ FetchPageInfo(const RefPtr& aDB, "AND EXISTS(SELECT 1 FROM moz_bookmarks b WHERE b.fk = r_place_id) " "LIMIT 1 " ") " - ") FROM moz_places h WHERE h.url_hash = hash(:page_url) AND h.url = :page_url", + ") " + "FROM moz_places h " + "WHERE h.url_hash = hash(:page_url) AND h.url = :page_url", nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT, nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY, nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT, @@ -94,21 +96,15 @@ FetchPageInfo(const RefPtr& aDB, rv = stmt->GetInt64(0, &_page.id); NS_ENSURE_SUCCESS(rv, rv); + rv = stmt->GetUTF8String(1, _page.guid); + NS_ENSURE_SUCCESS(rv, rv); + // Bookmarked url can be nullptr. bool isNull; - rv = stmt->GetIsNull(1, &isNull); - NS_ENSURE_SUCCESS(rv, rv); - // favicon_id can be nullptr. - if (!isNull) { - rv = stmt->GetInt64(1, &_page.iconId); - NS_ENSURE_SUCCESS(rv, rv); - } - rv = stmt->GetUTF8String(2, _page.guid); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->GetIsNull(3, &isNull); + rv = stmt->GetIsNull(2, &isNull); NS_ENSURE_SUCCESS(rv, rv); // The page could not be bookmarked. if (!isNull) { - rv = stmt->GetUTF8String(3, _page.bookmarkedSpec); + rv = stmt->GetUTF8String(2, _page.bookmarkedSpec); NS_ENSURE_SUCCESS(rv, rv); } @@ -136,136 +132,273 @@ FetchPageInfo(const RefPtr& aDB, } /** - * Stores information on a icon in the database. + * Stores information about an icon in the database. * - * @param aDBConn + * @param aDB * Database connection to history tables. * @param aIcon * Icon that should be stored. + * @param aMustReplace + * If set to true, the function will bail out with NS_ERROR_NOT_AVAILABLE + * if it can't find a previous stored icon to replace. + * @note Should be wrapped in a transaction. */ nsresult SetIconInfo(const RefPtr& aDB, - const IconData& aIcon) + IconData& aIcon, + bool aMustReplace = false) { MOZ_ASSERT(!NS_IsMainThread()); + MOZ_ASSERT(aIcon.payloads.Length() > 0); + MOZ_ASSERT(!aIcon.spec.IsEmpty()); - nsCOMPtr stmt = aDB->GetStatement( - "INSERT OR REPLACE INTO moz_favicons " - "(id, url, data, mime_type, expiration) " - "VALUES ((SELECT id FROM moz_favicons WHERE url = :icon_url), " - ":icon_url, :data, :mime_type, :expiration) " + // There are multiple cases possible at this point: + // 1. We must insert some payloads and no payloads exist in the table. This + // would be a straight INSERT. + // 2. The table contains the same number of payloads we are inserting. This + // would be a straight UPDATE. + // 3. The table contains more payloads than we are inserting. This would be + // an UPDATE and a DELETE. + // 4. The table contains less payloads than we are inserting. This would be + // an UPDATE and an INSERT. + // We can't just remove all the old entries and insert the new ones, cause + // we'd lose the referential integrity with pages. For the same reason we + // cannot use INSERT OR REPLACE, since it's implemented as DELETE AND INSERT. + // Thus, we follow this strategy: + // * SELECT all existing icon ids + // * For each payload, either UPDATE OR INSERT reusing icon ids. + // * If any previous icon ids is leftover, DELETE it. + + nsCOMPtr selectStmt = aDB->GetStatement( + "SELECT id FROM moz_icons " + "WHERE fixed_icon_url_hash = hash(fixup_url(:url)) " + "AND icon_url = :url " ); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); - nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("icon_url"), aIcon.spec); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindBlobByName(NS_LITERAL_CSTRING("data"), - TO_INTBUFFER(aIcon.data), aIcon.data.Length()); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("mime_type"), aIcon.mimeType); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("expiration"), aIcon.expiration); + NS_ENSURE_STATE(selectStmt); + mozStorageStatementScoper scoper(selectStmt); + nsresult rv = URIBinder::Bind(selectStmt, NS_LITERAL_CSTRING("url"), aIcon.spec); NS_ENSURE_SUCCESS(rv, rv); + std::deque ids; + bool hasResult = false; + while (NS_SUCCEEDED(selectStmt->ExecuteStep(&hasResult)) && hasResult) { + int64_t id = selectStmt->AsInt64(0); + MOZ_ASSERT(id > 0); + ids.push_back(id); + } + if (aMustReplace && ids.empty()) { + return NS_ERROR_NOT_AVAILABLE; + } - rv = stmt->Execute(); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr insertStmt = aDB->GetStatement( + "INSERT INTO moz_icons " + "(icon_url, fixed_icon_url_hash, width, expire_ms, data) " + "VALUES (:url, hash(fixup_url(:url)), :width, :expire, :data) " + ); + NS_ENSURE_STATE(insertStmt); + nsCOMPtr updateStmt = aDB->GetStatement( + "UPDATE moz_icons SET width = :width, " + "expire_ms = :expire, " + "data = :data " + "WHERE id = :id " + ); + NS_ENSURE_STATE(updateStmt); + + for (auto& payload : aIcon.payloads) { + // Sanity checks. + MOZ_ASSERT(payload.mimeType.EqualsLiteral(PNG_MIME_TYPE) || + payload.mimeType.EqualsLiteral(SVG_MIME_TYPE), + "Only png and svg payloads are supported"); + MOZ_ASSERT(!payload.mimeType.EqualsLiteral(SVG_MIME_TYPE) || + payload.width == UINT16_MAX, + "SVG payloads should have max width"); + MOZ_ASSERT(payload.width > 0, "Payload should have a width"); +#ifdef DEBUG + // Done to ensure we fetch the id. See the MOZ_ASSERT below. + payload.id = 0; +#endif + if (!ids.empty()) { + // Pop the first existing id for reuse. + int64_t id = ids.front(); + ids.pop_front(); + mozStorageStatementScoper scoper(updateStmt); + rv = updateStmt->BindInt64ByName(NS_LITERAL_CSTRING("id"), id); + NS_ENSURE_SUCCESS(rv, rv); + rv = updateStmt->BindInt32ByName(NS_LITERAL_CSTRING("width"), + payload.width); + NS_ENSURE_SUCCESS(rv, rv); + rv = updateStmt->BindInt64ByName(NS_LITERAL_CSTRING("expire"), + aIcon.expiration / 1000); + NS_ENSURE_SUCCESS(rv, rv); + rv = updateStmt->BindBlobByName(NS_LITERAL_CSTRING("data"), + TO_INTBUFFER(payload.data), + payload.data.Length()); + rv = updateStmt->Execute(); + NS_ENSURE_SUCCESS(rv, rv); + // Set the new payload id. + payload.id = id; + } else { + // Insert a new entry. + mozStorageStatementScoper scoper(insertStmt); + rv = URIBinder::Bind(insertStmt, NS_LITERAL_CSTRING("url"), aIcon.spec); + NS_ENSURE_SUCCESS(rv, rv); + rv = insertStmt->BindInt32ByName(NS_LITERAL_CSTRING("width"), + payload.width); + NS_ENSURE_SUCCESS(rv, rv); + rv = insertStmt->BindInt64ByName(NS_LITERAL_CSTRING("expire"), + aIcon.expiration / 1000); + NS_ENSURE_SUCCESS(rv, rv); + rv = insertStmt->BindBlobByName(NS_LITERAL_CSTRING("data"), + TO_INTBUFFER(payload.data), + payload.data.Length()); + NS_ENSURE_SUCCESS(rv, rv); + rv = insertStmt->Execute(); + NS_ENSURE_SUCCESS(rv, rv); + // Set the new payload id. + payload.id = nsFaviconService::sLastInsertedIconId; + } + MOZ_ASSERT(payload.id > 0, "Payload should have an id"); + } + + if (!ids.empty()) { + // Remove any old leftover payload. + nsAutoCString sql("DELETE FROM moz_icons WHERE id IN ("); + for (int64_t id : ids) { + sql.AppendInt(id); + sql.AppendLiteral(","); + } + sql.AppendLiteral(" 0)"); // Non-existing id to match the trailing comma. + nsCOMPtr stmt = aDB->GetStatement(sql); + NS_ENSURE_STATE(stmt); + mozStorageStatementScoper scoper(stmt); + rv = stmt->Execute(); + NS_ENSURE_SUCCESS(rv, rv); + } return NS_OK; } /** - * Fetches information on a icon from the Places database. + * Fetches information on a icon url from the database. * * @param aDBConn * Database connection to history tables. + * @param aPreferredWidth + * The preferred size to fetch. * @param _icon * Icon that should be fetched. */ nsresult FetchIconInfo(const RefPtr& aDB, - IconData& _icon) + uint16_t aPreferredWidth, + IconData& _icon +) { MOZ_ASSERT(_icon.spec.Length(), "Must have a non-empty spec!"); MOZ_ASSERT(!NS_IsMainThread()); if (_icon.status & ICON_STATUS_CACHED) { + // The icon data has already been set by ReplaceFaviconData. return NS_OK; } nsCOMPtr stmt = aDB->GetStatement( - "SELECT id, expiration, data, mime_type " - "FROM moz_favicons WHERE url = :icon_url" + "/* do not warn (bug no: not worth having a compound index) */ " + "SELECT id, expire_ms, data, width " + "FROM moz_icons " + "WHERE fixed_icon_url_hash = hash(fixup_url(:url)) " + "AND icon_url = :url " + "ORDER BY width ASC " ); NS_ENSURE_STATE(stmt); mozStorageStatementScoper scoper(stmt); - DebugOnly rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("icon_url"), + DebugOnly rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), _icon.spec); MOZ_ASSERT(NS_SUCCEEDED(rv)); - bool hasResult; - rv = stmt->ExecuteStep(&hasResult); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - if (!hasResult) { - // The icon does not exist yet, bail out. - return NS_OK; - } - - rv = stmt->GetInt64(0, &_icon.id); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - - // Expiration can be nullptr. - bool isNull; - rv = stmt->GetIsNull(1, &isNull); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - if (!isNull) { - rv = stmt->GetInt64(1, reinterpret_cast(&_icon.expiration)); + bool hasResult = false; + while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { + IconPayload payload; + rv = stmt->GetInt64(0, &payload.id); MOZ_ASSERT(NS_SUCCEEDED(rv)); - } - // Data can be nullptr. - rv = stmt->GetIsNull(2, &isNull); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - if (!isNull) { + // Expiration can be nullptr. + bool isNull; + rv = stmt->GetIsNull(1, &isNull); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + if (!isNull) { + int64_t expire_ms; + rv = stmt->GetInt64(1, &expire_ms); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + _icon.expiration = expire_ms * 1000; + } + uint8_t* data; uint32_t dataLen = 0; rv = stmt->GetBlob(2, &dataLen, &data); MOZ_ASSERT(NS_SUCCEEDED(rv)); - _icon.data.Adopt(TO_CHARBUFFER(data), dataLen); - // Read mime only if we have data. - rv = stmt->GetUTF8String(3, _icon.mimeType); + + payload.data.Adopt(TO_CHARBUFFER(data), dataLen); + int32_t width; + rv = stmt->GetInt32(3, &width); MOZ_ASSERT(NS_SUCCEEDED(rv)); + payload.width = width; + + if (payload.width == UINT16_MAX) { + payload.mimeType.AssignLiteral(SVG_MIME_TYPE); + } else { + payload.mimeType.AssignLiteral(PNG_MIME_TYPE); + } + + if (aPreferredWidth == 0 || _icon.payloads.Length() == 0) { + _icon.payloads.AppendElement(payload); + } else if (payload.width >= aPreferredWidth) { + // Only retain the best matching payload. + _icon.payloads.ReplaceElementAt(0, payload); + } else { + break; + } } return NS_OK; } nsresult -FetchIconURL(const RefPtr& aDB, - const nsACString& aPageSpec, - nsACString& aIconSpec) +FetchIconPerSpec(const RefPtr& aDB, + const nsACString& aPageSpec, + IconData& aIconData, + uint16_t aPreferredWidth) { MOZ_ASSERT(!aPageSpec.IsEmpty(), "Page spec must not be empty."); MOZ_ASSERT(!NS_IsMainThread()); - aIconSpec.Truncate(); - nsCOMPtr stmt = aDB->GetStatement( - "SELECT f.url " - "FROM moz_places h " - "JOIN moz_favicons f ON h.favicon_id = f.id " - "WHERE h.url_hash = hash(:page_url) AND h.url = :page_url" + "/* do not warn (bug no: not worth having a compound index) */ " + "SELECT width, icon_url " + "FROM moz_icons i " + "JOIN moz_icons_to_pages ON i.id = icon_id " + "JOIN moz_pages_w_icons p ON p.id = page_id " + "WHERE page_url_hash = hash(:url) AND page_url = :url " + "ORDER BY width DESC " ); NS_ENSURE_STATE(stmt); mozStorageStatementScoper scoper(stmt); - nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), + nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), aPageSpec); NS_ENSURE_SUCCESS(rv, rv); + // Return the biggest icon close to the preferred width. It may be bigger + // or smaller if the preferred width isn't found. bool hasResult; - if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { - rv = stmt->GetUTF8String(0, aIconSpec); + while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { + int32_t width; + rv = stmt->GetInt32(0, &width); + NS_ENSURE_SUCCESS(rv, rv); + if (width < aPreferredWidth && !aIconData.spec.IsEmpty()) { + break; + } + rv = stmt->GetUTF8String(1, aIconData.spec); NS_ENSURE_SUCCESS(rv, rv); } @@ -307,40 +440,6 @@ GetExpirationTimeFromChannel(nsIChannel* aChannel) : expiration; } -/** - * Checks the icon and evaluates if it needs to be optimized. In such a case it - * will try to reduce its size through OptimizeFaviconImage method of the - * favicons service. - * - * @param aIcon - * The icon to be evaluated. - * @param aFaviconSvc - * Pointer to the favicons service. - */ -nsresult -OptimizeIconSize(IconData& aIcon, - nsFaviconService* aFaviconSvc) -{ - MOZ_ASSERT(NS_IsMainThread()); - - // Even if the page provides a large image for the favicon (eg, a highres - // image or a multiresolution .ico file), don't try to store more data than - // needed. - nsAutoCString newData, newMimeType; - if (aIcon.data.Length() > MAX_FAVICON_FILESIZE) { - nsresult rv = aFaviconSvc->OptimizeFaviconImage(TO_INTBUFFER(aIcon.data), - aIcon.data.Length(), - aIcon.mimeType, - newData, - newMimeType); - if (NS_SUCCEEDED(rv) && newData.Length() < aIcon.data.Length()) { - aIcon.data = newData; - aIcon.mimeType = newMimeType; - } - } - return NS_OK; -} - } // namespace //////////////////////////////////////////////////////////////////////////////// @@ -379,10 +478,10 @@ AsyncFetchAndSetIconForPage::Run() // Try to fetch the icon from the database. RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); - nsresult rv = FetchIconInfo(DB, mIcon); + nsresult rv = FetchIconInfo(DB, 0, mIcon); NS_ENSURE_SUCCESS(rv, rv); - bool isInvalidIcon = mIcon.data.IsEmpty() || + bool isInvalidIcon = !mIcon.payloads.Length() || (mIcon.expiration && PR_Now() > mIcon.expiration); bool fetchIconFromNetwork = mIcon.fetchMode == FETCH_ALWAYS || (mIcon.fetchMode == FETCH_IF_MISSING && isInvalidIcon); @@ -392,9 +491,8 @@ AsyncFetchAndSetIconForPage::Run() // directly proceed with association. RefPtr event = new AsyncAssociateIconToPage(mIcon, mPage, mCallback); - DB->DispatchToAsyncThread(event); - - return NS_OK; + // We're already on the async thread. + return event->Run(); } // Fetch the icon from the network, the request starts from the main-thread. @@ -413,10 +511,10 @@ AsyncFetchAndSetIconForPage::FetchFromNetwork() { } // Ensure data is cleared, since it's going to be overwritten. - if (mIcon.data.Length() > 0) { - mIcon.data.Truncate(0); - mIcon.mimeType.Truncate(0); - } + mIcon.payloads.Clear(); + + IconPayload payload; + mIcon.payloads.AppendElement(payload); nsCOMPtr iconURI; nsresult rv = NS_NewURI(getter_AddRefs(iconURI), mIcon.spec); @@ -489,9 +587,11 @@ AsyncFetchAndSetIconForPage::OnDataAvailable(nsIRequest* aRequest, uint64_t aOffset, uint32_t aCount) { - const size_t kMaxFaviconDownloadSize = 1 * 1024 * 1024; - if (mIcon.data.Length() + aCount > kMaxFaviconDownloadSize) { - mIcon.data.Truncate(); + MOZ_ASSERT(mIcon.payloads.Length() == 1); + // Limit downloads to 500KB. + const size_t kMaxDownloadSize = 500 * 1024; + if (mIcon.payloads[0].data.Length() + aCount > kMaxDownloadSize) { + mIcon.payloads.Clear(); return NS_ERROR_FILE_TOO_BIG; } @@ -501,8 +601,8 @@ AsyncFetchAndSetIconForPage::OnDataAvailable(nsIRequest* aRequest, return rv; } - if (!mIcon.data.Append(buffer, fallible)) { - mIcon.data.Truncate(); + if (!mIcon.payloads[0].data.Append(buffer, fallible)) { + mIcon.payloads.Clear(); return NS_ERROR_OUT_OF_MEMORY; } @@ -551,7 +651,7 @@ AsyncFetchAndSetIconForPage::OnStopRequest(nsIRequest* aRequest, nsresult rv; // If fetching the icon failed, add it to the failed cache. - if (NS_FAILED(aStatusCode) || mIcon.data.Length() == 0) { + if (NS_FAILED(aStatusCode) || mIcon.payloads.Length() == 0) { nsCOMPtr iconURI; rv = NS_NewURI(getter_AddRefs(iconURI), mIcon.spec); NS_ENSURE_SUCCESS(rv, rv); @@ -564,19 +664,23 @@ AsyncFetchAndSetIconForPage::OnStopRequest(nsIRequest* aRequest, // aRequest should always QI to nsIChannel. MOZ_ASSERT(channel); + MOZ_ASSERT(mIcon.payloads.Length() == 1); + IconPayload& payload = mIcon.payloads[0]; + nsAutoCString contentType; channel->GetContentType(contentType); - // Bug 366324 - can't sniff SVG yet, so rely on server-specified type - if (contentType.EqualsLiteral("image/svg+xml")) { - mIcon.mimeType.AssignLiteral("image/svg+xml"); + // Bug 366324 - We don't want to sniff for SVG, so rely on server-specified type. + if (contentType.EqualsLiteral(SVG_MIME_TYPE)) { + payload.mimeType.AssignLiteral(SVG_MIME_TYPE); + payload.width = UINT16_MAX; } else { NS_SniffContent(NS_DATA_SNIFFER_CATEGORY, aRequest, - TO_INTBUFFER(mIcon.data), mIcon.data.Length(), - mIcon.mimeType); + TO_INTBUFFER(payload.data), payload.data.Length(), + payload.mimeType); } // If the icon does not have a valid MIME type, add it to the failed cache. - if (mIcon.mimeType.IsEmpty()) { + if (payload.mimeType.IsEmpty()) { nsCOMPtr iconURI; rv = NS_NewURI(getter_AddRefs(iconURI), mIcon.spec); NS_ENSURE_SUCCESS(rv, rv); @@ -589,37 +693,36 @@ AsyncFetchAndSetIconForPage::OnStopRequest(nsIRequest* aRequest, // Telemetry probes to measure the favicon file sizes for each different file type. // This allow us to measure common file sizes while also observing each type popularity. - if (mIcon.mimeType.EqualsLiteral("image/png")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_PNG_SIZES, mIcon.data.Length()); + if (payload.mimeType.EqualsLiteral(PNG_MIME_TYPE)) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_PNG_SIZES, payload.data.Length()); } - else if (mIcon.mimeType.EqualsLiteral("image/x-icon") || - mIcon.mimeType.EqualsLiteral("image/vnd.microsoft.icon")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_ICO_SIZES, mIcon.data.Length()); + else if (payload.mimeType.EqualsLiteral("image/x-icon") || + payload.mimeType.EqualsLiteral("image/vnd.microsoft.icon")) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_ICO_SIZES, payload.data.Length()); } - else if (mIcon.mimeType.EqualsLiteral("image/jpeg") || - mIcon.mimeType.EqualsLiteral("image/pjpeg")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_JPEG_SIZES, mIcon.data.Length()); + else if (payload.mimeType.EqualsLiteral("image/jpeg") || + payload.mimeType.EqualsLiteral("image/pjpeg")) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_JPEG_SIZES, payload.data.Length()); } - else if (mIcon.mimeType.EqualsLiteral("image/gif")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_GIF_SIZES, mIcon.data.Length()); + else if (payload.mimeType.EqualsLiteral("image/gif")) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_GIF_SIZES, payload.data.Length()); } - else if (mIcon.mimeType.EqualsLiteral("image/bmp") || - mIcon.mimeType.EqualsLiteral("image/x-windows-bmp")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_BMP_SIZES, mIcon.data.Length()); + else if (payload.mimeType.EqualsLiteral("image/bmp") || + payload.mimeType.EqualsLiteral("image/x-windows-bmp")) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_BMP_SIZES, payload.data.Length()); } - else if (mIcon.mimeType.EqualsLiteral("image/svg+xml")) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_SVG_SIZES, mIcon.data.Length()); + else if (payload.mimeType.EqualsLiteral(SVG_MIME_TYPE)) { + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_SVG_SIZES, payload.data.Length()); } else { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_OTHER_SIZES, mIcon.data.Length()); + mozilla::Telemetry::Accumulate(mozilla::Telemetry::PLACES_FAVICON_OTHER_SIZES, payload.data.Length()); } - rv = OptimizeIconSize(mIcon, favicons); + rv = favicons->OptimizeIconSizes(mIcon); NS_ENSURE_SUCCESS(rv, rv); - // If over the maximum size allowed, don't save data to the database to - // avoid bloating it. - if (mIcon.data.Length() > nsIFaviconService::MAX_FAVICON_BUFFER_SIZE) { + // If there's not valid payload, don't store the icon into to the database. + if (mIcon.payloads.Length() == 0) { return NS_OK; } @@ -645,6 +748,7 @@ AsyncAssociateIconToPage::AsyncAssociateIconToPage( , mIcon(aIcon) , mPage(aPage) { + // May be created in both threads. } NS_IMETHODIMP @@ -666,59 +770,82 @@ AsyncAssociateIconToPage::Run() NS_ENSURE_SUCCESS(rv, rv); } + bool shouldUpdateIcon = mIcon.status & ICON_STATUS_CHANGED; + if (!shouldUpdateIcon) { + for (const auto& payload : mIcon.payloads) { + // If the entry is missing from the database, we should add it. + if (payload.id == 0) { + shouldUpdateIcon = true; + break; + } + } + } + mozStorageTransaction transaction(DB->MainConn(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE); - // If there is no entry for this icon, or the entry is obsolete, replace it. - if (mIcon.id == 0 || (mIcon.status & ICON_STATUS_CHANGED)) { + if (shouldUpdateIcon) { rv = SetIconInfo(DB, mIcon); NS_ENSURE_SUCCESS(rv, rv); - // Get the new icon id. Do this regardless mIcon.id, since other code - // could have added a entry before us. Indeed we interrupted the thread - // after the previous call to FetchIconInfo. mIcon.status = (mIcon.status & ~(ICON_STATUS_CACHED)) | ICON_STATUS_SAVED; - rv = FetchIconInfo(DB, mIcon); - NS_ENSURE_SUCCESS(rv, rv); } // If the page does not have an id, don't try to insert a new one, cause we // don't know where the page comes from. Not doing so we may end adding // a page that otherwise we'd explicitly ignore, like a POST or an error page. if (mPage.id == 0) { + rv = transaction.Commit(); + NS_ENSURE_SUCCESS(rv, rv); return NS_OK; } - // Otherwise just associate the icon to the page, if needed. - if (mPage.iconId != mIcon.id) { + // First we need to create the page entry. + { nsCOMPtr stmt; - if (mPage.id) { - stmt = DB->GetStatement( - "UPDATE moz_places SET favicon_id = :icon_id WHERE id = :page_id" - ); - NS_ENSURE_STATE(stmt); - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id); - NS_ENSURE_SUCCESS(rv, rv); - } - else { - stmt = DB->GetStatement( - "UPDATE moz_places SET favicon_id = :icon_id " - "WHERE url_hash = hash(:page_url) AND url = :page_url" - ); - NS_ENSURE_STATE(stmt); - rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPage.spec); - NS_ENSURE_SUCCESS(rv, rv); - } - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("icon_id"), mIcon.id); - NS_ENSURE_SUCCESS(rv, rv); - + stmt = DB->GetStatement( + "INSERT OR IGNORE INTO moz_pages_w_icons (id, page_url, page_url_hash) " + "VALUES (:page_id, :page_url, hash(:page_url)) " + ); + NS_ENSURE_STATE(stmt); mozStorageStatementScoper scoper(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id); + NS_ENSURE_SUCCESS(rv, rv); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPage.spec); + NS_ENSURE_SUCCESS(rv, rv); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); - - mIcon.status |= ICON_STATUS_ASSOCIATED; } + // Then we can create the relations. + nsCOMPtr stmt; + stmt = DB->GetStatement( + "INSERT OR IGNORE INTO moz_icons_to_pages (page_id, icon_id) " + "VALUES (:page_id, :icon_id) " + ); + NS_ENSURE_STATE(stmt); + mozStorageStatementScoper scoper(stmt); + nsCOMPtr paramsArray; + rv = stmt->NewBindingParamsArray(getter_AddRefs(paramsArray)); + NS_ENSURE_SUCCESS(rv, rv); + for (const auto& payload : mIcon.payloads) { + nsCOMPtr params; + rv = paramsArray->NewBindingParams(getter_AddRefs(params)); + NS_ENSURE_SUCCESS(rv, rv); + rv = params->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id); + NS_ENSURE_SUCCESS(rv, rv); + rv = params->BindInt64ByName(NS_LITERAL_CSTRING("icon_id"), payload.id); + NS_ENSURE_SUCCESS(rv, rv); + rv = paramsArray->AddParams(params); + NS_ENSURE_SUCCESS(rv, rv); + } + rv = stmt->BindParameters(paramsArray); + NS_ENSURE_SUCCESS(rv, rv); + rv = stmt->Execute(); + NS_ENSURE_SUCCESS(rv, rv); + + mIcon.status |= ICON_STATUS_ASSOCIATED; + rv = transaction.Commit(); NS_ENSURE_SUCCESS(rv, rv); @@ -735,8 +862,10 @@ AsyncAssociateIconToPage::Run() AsyncGetFaviconURLForPage::AsyncGetFaviconURLForPage( const nsACString& aPageSpec +, uint16_t aPreferredWidth , nsIFaviconDataCallback* aCallback -) : mCallback(new nsMainThreadPtrHolder(aCallback)) +) : mPreferredWidth(aPreferredWidth == 0 ? UINT16_MAX : aPreferredWidth) + , mCallback(new nsMainThreadPtrHolder(aCallback)) { MOZ_ASSERT(NS_IsMainThread()); mPageSpec.Assign(aPageSpec); @@ -749,14 +878,11 @@ AsyncGetFaviconURLForPage::Run() RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); - nsAutoCString iconSpec; - nsresult rv = FetchIconURL(DB, mPageSpec, iconSpec); + IconData iconData; + nsresult rv = FetchIconPerSpec(DB, mPageSpec, iconData, mPreferredWidth); NS_ENSURE_SUCCESS(rv, rv); // Now notify our callback of the icon spec we retrieved, even if empty. - IconData iconData; - iconData.spec.Assign(iconSpec); - PageData pageData; pageData.spec.Assign(mPageSpec); @@ -773,8 +899,10 @@ AsyncGetFaviconURLForPage::Run() AsyncGetFaviconDataForPage::AsyncGetFaviconDataForPage( const nsACString& aPageSpec +, uint16_t aPreferredWidth , nsIFaviconDataCallback* aCallback -) : mCallback(new nsMainThreadPtrHolder(aCallback)) +) : mPreferredWidth(aPreferredWidth == 0 ? UINT16_MAX : aPreferredWidth) + , mCallback(new nsMainThreadPtrHolder(aCallback)) { MOZ_ASSERT(NS_IsMainThread()); mPageSpec.Assign(aPageSpec); @@ -787,23 +915,20 @@ AsyncGetFaviconDataForPage::Run() RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); - nsAutoCString iconSpec; - nsresult rv = FetchIconURL(DB, mPageSpec, iconSpec); + IconData iconData; + nsresult rv = FetchIconPerSpec(DB, mPageSpec, iconData, mPreferredWidth); NS_ENSURE_SUCCESS(rv, rv); - IconData iconData; - iconData.spec.Assign(iconSpec); - - PageData pageData; - pageData.spec.Assign(mPageSpec); - - if (!iconSpec.IsEmpty()) { - rv = FetchIconInfo(DB, iconData); + if (!iconData.spec.IsEmpty()) { + rv = FetchIconInfo(DB, mPreferredWidth, iconData); if (NS_FAILED(rv)) { iconData.spec.Truncate(); } } + PageData pageData; + pageData.spec.Assign(mPageSpec); + nsCOMPtr event = new NotifyIconObservers(iconData, pageData, mCallback); rv = NS_DispatchToMainThread(event); @@ -817,6 +942,7 @@ AsyncGetFaviconDataForPage::Run() AsyncReplaceFaviconData::AsyncReplaceFaviconData(const IconData &aIcon) : mIcon(aIcon) { + MOZ_ASSERT(NS_IsMainThread()); } NS_IMETHODIMP @@ -826,16 +952,16 @@ AsyncReplaceFaviconData::Run() RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); - IconData dbIcon; - dbIcon.spec.Assign(mIcon.spec); - nsresult rv = FetchIconInfo(DB, dbIcon); - NS_ENSURE_SUCCESS(rv, rv); - if (!dbIcon.id) { + mozStorageTransaction transaction(DB->MainConn(), false, + mozIStorageConnection::TRANSACTION_IMMEDIATE); + nsresult rv = SetIconInfo(DB, mIcon, true); + if (rv == NS_ERROR_NOT_AVAILABLE) { + // There's no previous icon to replace, we don't need to do anything. return NS_OK; } - - rv = SetIconInfo(DB, mIcon); + NS_ENSURE_SUCCESS(rv, rv); + rv = transaction.Commit(); NS_ENSURE_SUCCESS(rv, rv); // We can invalidate the cache version since we now persist the icon. @@ -896,12 +1022,18 @@ NotifyIconObservers::Run() } } - if (mCallback) { - (void)mCallback->OnComplete(iconURI, mIcon.data.Length(), - TO_INTBUFFER(mIcon.data), mIcon.mimeType); + if (!mCallback) { + return NS_OK; } - return NS_OK; + if (mIcon.payloads.Length() > 0) { + IconPayload& payload = mIcon.payloads[0]; + return mCallback->OnComplete(iconURI, payload.data.Length(), + TO_INTBUFFER(payload.data), payload.mimeType, + payload.width); + } + return mCallback->OnComplete(iconURI, 0, TO_INTBUFFER(EmptyCString()), + EmptyCString(), 0); } void diff --git a/toolkit/components/places/FaviconHelpers.h b/toolkit/components/places/FaviconHelpers.h index 1bc82d88b170..f2fd69526d55 100644 --- a/toolkit/components/places/FaviconHelpers.h +++ b/toolkit/components/places/FaviconHelpers.h @@ -15,6 +15,7 @@ #include "nsProxyRelease.h" #include "imgITools.h" #include "imgIContainer.h" +#include "imgLoader.h" class nsIPrincipal; @@ -59,25 +60,41 @@ enum AsyncFaviconFetchMode { }; /** - * Data cache for a icon entry. + * Represents one of the payloads (frames) of an icon entry. + */ +struct IconPayload +{ + IconPayload() + : id(0) + , width(0) + { + data.SetIsVoid(true); + mimeType.SetIsVoid(true); + } + + int64_t id; + uint16_t width; + nsCString data; + nsCString mimeType; +}; + +/** + * Represents an icon entry. */ struct IconData { IconData() - : id(0) - , expiration(0) + : expiration(0) , fetchMode(FETCH_NEVER) , status(ICON_STATUS_UNKNOWN) { } - int64_t id; nsCString spec; - nsCString data; - nsCString mimeType; PRTime expiration; enum AsyncFaviconFetchMode fetchMode; uint16_t status; // This is a bitset, see ICON_STATUS_* defines above. + nsTArray payloads; }; /** @@ -88,7 +105,6 @@ struct PageData PageData() : id(0) , canAddToHistory(true) - , iconId(0) { guid.SetIsVoid(true); } @@ -98,7 +114,6 @@ struct PageData nsCString bookmarkedSpec; nsString revHost; bool canAddToHistory; // False for disabled history and unsupported schemas. - int64_t iconId; nsCString guid; }; @@ -199,11 +214,15 @@ public: * URL of the page whose favicon's URL we're fetching * @param aCallback * function to be called once finished + * @param aPreferredWidth + * The preferred size for the icon */ AsyncGetFaviconURLForPage(const nsACString& aPageSpec, + uint16_t aPreferredWidth, nsIFaviconDataCallback* aCallback); private: + uint16_t mPreferredWidth; nsMainThreadPtrHandle mCallback; nsCString mPageSpec; }; @@ -223,13 +242,18 @@ public: * * @param aPageSpec * URL of the page whose favicon URL and data we're fetching + * @param aPreferredWidth + * The preferred size of the icon. We will try to return an icon close + * to this size. * @param aCallback * function to be called once finished */ AsyncGetFaviconDataForPage(const nsACString& aPageSpec, + uint16_t aPreferredWidth, nsIFaviconDataCallback* aCallback); private: + uint16_t mPreferredWidth; nsMainThreadPtrHandle mCallback; nsCString mPageSpec; }; diff --git a/toolkit/components/places/History.jsm b/toolkit/components/places/History.jsm index 6094fb054ae3..21143afb6e8d 100644 --- a/toolkit/components/places/History.jsm +++ b/toolkit/components/places/History.jsm @@ -160,9 +160,7 @@ this.History = Object.freeze({ * * @throws (Error) * If the `url` specified was for a protocol that should not be - * stored (e.g. "chrome:", "mailbox:", "about:", "imap:", "news:", - * "moz-anno:", "view-source:", "resource:", "data:", "wyciwyg:", - * "javascript:", "blob:"). + * stored (@see nsNavHistory::CanAddURI). * @throws (Error) * If `pageInfo` has an unexpected type. * @throws (Error) @@ -216,9 +214,7 @@ this.History = Object.freeze({ * * @throws (Error) * If the `url` specified was for a protocol that should not be - * stored (e.g. "chrome:", "mailbox:", "about:", "imap:", "news:", - * "moz-anno:", "view-source:", "resource:", "data:", "wyciwyg:", - * "javascript:", "blob:"). + * stored (@see nsNavHistory::CanAddURI). * @throws (Error) * If `pageInfos` has an unexpected type. * @throws (Error) @@ -706,9 +702,10 @@ var cleanupPages = Task.async(function*(db, pages) { yield db.executeCached(`DELETE FROM moz_updatehosts_temp`); // Expire orphans. - yield db.executeCached(` - DELETE FROM moz_favicons WHERE NOT EXISTS - (SELECT 1 FROM moz_places WHERE favicon_id = moz_favicons.id)`); + yield db.executeCached(`DELETE FROM moz_pages_w_icons + WHERE page_url_hash NOT IN (SELECT url_hash FROM moz_places)`); + yield db.executeCached(`DELETE FROM moz_icons + WHERE id NOT IN (SELECT icon_id FROM moz_icons_to_pages)`); yield db.execute(`DELETE FROM moz_annos WHERE place_id IN ( ${ idsList } )`); yield db.execute(`DELETE FROM moz_inputhistory diff --git a/toolkit/components/places/PageIconProtocolHandler.js b/toolkit/components/places/PageIconProtocolHandler.js index e530b9663e32..07bf90b984d8 100644 --- a/toolkit/components/places/PageIconProtocolHandler.js +++ b/toolkit/components/places/PageIconProtocolHandler.js @@ -41,6 +41,16 @@ function streamDefaultFavicon(uri, loadInfo, outputStream) { } } +function serveIcon(pipe, data, len) { + // Pass the icon data to the output stream. + let stream = Cc["@mozilla.org/binaryoutputstream;1"] + .createInstance(Ci.nsIBinaryOutputStream); + stream.setOutputStream(pipe.outputStream); + stream.writeByteArray(data, len); + stream.close(); + pipe.outputStream.close(); +} + function PageIconProtocolHandler() { } @@ -83,25 +93,16 @@ PageIconProtocolHandler.prototype = { channel.loadInfo = loadInfo; let pageURI = NetUtil.newURI(uri.path); - PlacesUtils.favicons.getFaviconDataForPage(pageURI, (iconuri, len, data, mime) => { + PlacesUtils.favicons.getFaviconDataForPage(pageURI, (iconURI, len, data, mimeType) => { + channel.contentType = mimeType; if (len == 0) { - channel.contentType = "image/png"; - streamDefaultFavicon(uri, loadInfo, pipe.outputStream); - return; - } - - try { - channel.contentType = mime; - // Pass the icon data to the output stream. - let stream = Cc["@mozilla.org/binaryoutputstream;1"] - .createInstance(Ci.nsIBinaryOutputStream); - stream.setOutputStream(pipe.outputStream); - stream.writeByteArray(data, len); - stream.close(); - pipe.outputStream.close(); - } catch (ex) { - channel.contentType = "image/png"; streamDefaultFavicon(uri, loadInfo, pipe.outputStream); + } else { + try { + serveIcon(pipe, data, len); + } catch (ex) { + streamDefaultFavicon(uri, loadInfo, pipe.outputStream); + } } }); diff --git a/toolkit/components/places/PlacesDBUtils.jsm b/toolkit/components/places/PlacesDBUtils.jsm index 2070570de7d9..c10cc643cec0 100644 --- a/toolkit/components/places/PlacesDBUtils.jsm +++ b/toolkit/components/places/PlacesDBUtils.jsm @@ -593,13 +593,17 @@ this.PlacesDBUtils = { fixEmptyNamedTags.params["tags_folder"] = PlacesUtils.tagsFolderId; cleanupStatements.push(fixEmptyNamedTags); - // MOZ_FAVICONS - // E.1 remove orphan icons + // MOZ_ICONS + // E.1 remove orphan icon entries. + let deleteOrphanIconPages = DBConn.createAsyncStatement( + `DELETE FROM moz_pages_w_icons WHERE page_url_hash NOT IN ( + SELECT url_hash FROM moz_places + )`); + cleanupStatements.push(deleteOrphanIconPages); + let deleteOrphanIcons = DBConn.createAsyncStatement( - `DELETE FROM moz_favicons WHERE id IN ( - SELECT id FROM moz_favicons f - WHERE NOT EXISTS - (SELECT id FROM moz_places WHERE favicon_id = f.id LIMIT 1) + `DELETE FROM moz_icons WHERE id NOT IN ( + SELECT icon_id FROM moz_icons_to_pages )`); cleanupStatements.push(deleteOrphanIcons); @@ -654,16 +658,6 @@ this.PlacesDBUtils = { cleanupStatements.push(deleteUnusedKeywords); // MOZ_PLACES - // L.1 fix wrong favicon ids - let fixInvalidFaviconIds = DBConn.createAsyncStatement( - `UPDATE moz_places SET favicon_id = NULL WHERE id IN ( - SELECT id FROM moz_places h - WHERE favicon_id NOT NULL - AND NOT EXISTS - (SELECT id FROM moz_favicons WHERE id = h.favicon_id LIMIT 1) - )`); - cleanupStatements.push(fixInvalidFaviconIds); - // L.2 recalculate visit_count and last_visit_date let fixVisitStats = DBConn.createAsyncStatement( `UPDATE moz_places diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index 3267bdb54737..d85f9026176e 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1869,7 +1869,11 @@ this.PlacesUtils = { JOIN descendants ON b2.parent = descendants.id AND b2.id <> :tags_folder) SELECT d.level, d.id, d.guid, d.parent, d.parentGuid, d.type, d.position AS [index], d.title, d.dateAdded, d.lastModified, - h.url, f.url AS iconuri, + h.url, (SELECT icon_url FROM moz_icons i + JOIN moz_icons_to_pages ON icon_id = i.id + JOIN moz_pages_w_icons pi ON page_id = pi.id + WHERE pi.page_url_hash = hash(h.url) AND pi.page_url = h.url + ORDER BY width DESC LIMIT 1) AS iconuri, (SELECT GROUP_CONCAT(t.title, ',') FROM moz_bookmarks b2 JOIN moz_bookmarks t ON t.id = +b2.parent AND t.parent = :tags_folder @@ -1884,7 +1888,6 @@ this.PlacesUtils = { FROM descendants d LEFT JOIN moz_bookmarks b3 ON b3.id = d.parent LEFT JOIN moz_places h ON h.id = d.fk - LEFT JOIN moz_favicons f ON f.id = h.favicon_id ORDER BY d.level, d.parent, d.position`; diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index aa56f81e3a0b..0a6d9f9dfbe5 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -94,15 +94,14 @@ const REGEXP_SPACES = /\s+/; const QUERYINDEX_QUERYTYPE = 0; const QUERYINDEX_URL = 1; const QUERYINDEX_TITLE = 2; -const QUERYINDEX_ICONURL = 3; -const QUERYINDEX_BOOKMARKED = 4; -const QUERYINDEX_BOOKMARKTITLE = 5; -const QUERYINDEX_TAGS = 6; -const QUERYINDEX_VISITCOUNT = 7; -const QUERYINDEX_TYPED = 8; -const QUERYINDEX_PLACEID = 9; -const QUERYINDEX_SWITCHTAB = 10; -const QUERYINDEX_FRECENCY = 11; +const QUERYINDEX_BOOKMARKED = 3; +const QUERYINDEX_BOOKMARKTITLE = 4; +const QUERYINDEX_TAGS = 5; +const QUERYINDEX_VISITCOUNT = 6; +const QUERYINDEX_TYPED = 7; +const QUERYINDEX_PLACEID = 8; +const QUERYINDEX_SWITCHTAB = 9; +const QUERYINDEX_FRECENCY = 10; // This SQL query fragment provides the following: // - whether the entry is bookmarked (QUERYINDEX_BOOKMARKED) @@ -125,10 +124,9 @@ const SQL_BOOKMARK_TAGS_FRAGMENT = // and "tags" queries for bookmarked entries. function defaultQuery(conditions = "") { let query = - `SELECT :query_type, h.url, h.title, f.url, ${SQL_BOOKMARK_TAGS_FRAGMENT}, + `SELECT :query_type, h.url, h.title, ${SQL_BOOKMARK_TAGS_FRAGMENT}, h.visit_count, h.typed, h.id, t.open_count, h.frecency FROM moz_places h - LEFT JOIN moz_favicons f ON f.id = h.favicon_id LEFT JOIN moz_openpages_temp t ON t.url = h.url AND t.userContextId = :userContextId @@ -150,7 +148,7 @@ function defaultQuery(conditions = "") { } const SQL_SWITCHTAB_QUERY = - `SELECT :query_type, t.url, t.url, NULL, NULL, NULL, NULL, NULL, NULL, NULL, + `SELECT :query_type, t.url, t.url, NULL, NULL, NULL, NULL, NULL, NULL, t.open_count, NULL FROM moz_openpages_temp t LEFT JOIN moz_places h ON h.url_hash = hash(t.url) AND h.url = t.url @@ -164,7 +162,7 @@ const SQL_SWITCHTAB_QUERY = const SQL_ADAPTIVE_QUERY = `/* do not warn (bug 487789) */ - SELECT :query_type, h.url, h.title, f.url, ${SQL_BOOKMARK_TAGS_FRAGMENT}, + SELECT :query_type, h.url, h.title, ${SQL_BOOKMARK_TAGS_FRAGMENT}, h.visit_count, h.typed, h.id, t.open_count, h.frecency FROM ( SELECT ROUND(MAX(use_count) * (1 + (input = :search_string)), 1) AS rank, @@ -174,7 +172,6 @@ const SQL_ADAPTIVE_QUERY = GROUP BY place_id ) AS i JOIN moz_places h ON h.id = i.place_id - LEFT JOIN moz_favicons f ON f.id = h.favicon_id LEFT JOIN moz_openpages_temp t ON t.url = h.url AND t.userContextId = :userContextId @@ -189,12 +186,7 @@ const SQL_ADAPTIVE_QUERY = function hostQuery(conditions = "") { let query = `/* do not warn (bug NA): not worth to index on (typed, frecency) */ - SELECT :query_type, host || '/', IFNULL(prefix, '') || host || '/', - ( SELECT f.url FROM moz_favicons f - JOIN moz_places h ON h.favicon_id = f.id - WHERE rev_host = get_unreversed_host(host || '.') || '.' - OR rev_host = get_unreversed_host(host || '.') || '.www.' - ) AS favicon_url, + SELECT :query_type, host || '/', IFNULL(prefix, 'http://') || host || '/', NULL, NULL, NULL, NULL, NULL, NULL, NULL, frecency FROM moz_hosts WHERE host BETWEEN :searchString AND :searchString || X'FFFF' @@ -212,12 +204,7 @@ const SQL_TYPED_HOST_QUERY = hostQuery("AND typed = 1"); function bookmarkedHostQuery(conditions = "") { let query = `/* do not warn (bug NA): not worth to index on (typed, frecency) */ - SELECT :query_type, host || '/', IFNULL(prefix, '') || host || '/', - ( SELECT f.url FROM moz_favicons f - JOIN moz_places h ON h.favicon_id = f.id - WHERE rev_host = get_unreversed_host(host || '.') || '.' - OR rev_host = get_unreversed_host(host || '.') || '.www.' - ) AS favicon_url, + SELECT :query_type, host || '/', IFNULL(prefix, 'http://') || host || '/', ( SELECT foreign_count > 0 FROM moz_places WHERE rev_host = get_unreversed_host(host || '.') || '.' OR rev_host = get_unreversed_host(host || '.') || '.www.' @@ -238,11 +225,10 @@ const SQL_BOOKMARKED_TYPED_HOST_QUERY = bookmarkedHostQuery("AND typed = 1"); function urlQuery(conditions = "") { return `/* do not warn (bug no): cannot use an index to sort */ - SELECT :query_type, h.url, NULL, f.url AS favicon_url, + SELECT :query_type, h.url, NULL, foreign_count > 0 AS bookmarked, NULL, NULL, NULL, NULL, NULL, NULL, h.frecency FROM moz_places h - LEFT JOIN moz_favicons f ON h.favicon_id = f.id WHERE (rev_host = :revHost OR rev_host = :revHost || "www.") AND h.frecency <> 0 AND fixup_url(h.url) BETWEEN :searchString AND :searchString || X'FFFF' @@ -1392,7 +1378,12 @@ Search.prototype = { // The title will end up being "host: queryString" let comment = entry.url.host; - this._addMatch({ value, comment, style, frecency: FRECENCY_DEFAULT }); + this._addMatch({ + value, + comment, + icon: "page-icon:" + url, + style, + frecency: FRECENCY_DEFAULT }); return true; }, @@ -1533,12 +1524,7 @@ Search.prototype = { // It's rare that Sync supplies the icon for the page (but if it does, it // is a string URL) if (!icon) { - try { - let favicon = yield PlacesUtils.promiseFaviconLinkUrl(url); - if (favicon) { - icon = favicon.spec; - } - } catch (ex) {} // no favicon for this URL. + icon = "page-icon:" + url; } else { icon = PlacesUtils.favicons .getFaviconLinkForIcon(NetUtil.newURI(icon)).spec; @@ -1621,16 +1607,9 @@ Search.prototype = { comment: displayURL, style: "action visiturl", frecency: 0, + icon: "page-icon:" + escapedURL }; - try { - let favicon = yield PlacesUtils.promiseFaviconLinkUrl(uri); - if (favicon) - match.icon = favicon.spec; - } catch (e) { - // It's possible we don't have a favicon for this - and that's ok. - } - this._addMatch(match); return true; }, @@ -1784,9 +1763,9 @@ Search.prototype = { _processHostRow(row) { let match = {}; let strippedHost = row.getResultByIndex(QUERYINDEX_URL); - let unstrippedHost = row.getResultByIndex(QUERYINDEX_TITLE); + let url = row.getResultByIndex(QUERYINDEX_TITLE); + let unstrippedHost = stripHttpAndTrim(url, false); let frecency = row.getResultByIndex(QUERYINDEX_FRECENCY); - let faviconUrl = row.getResultByIndex(QUERYINDEX_ICONURL); // If the unfixup value doesn't preserve the user's input just // ignore it and complete to the found host. @@ -1797,10 +1776,8 @@ Search.prototype = { match.value = this._strippedPrefix + strippedHost; match.finalCompleteValue = unstrippedHost; - if (faviconUrl) { - match.icon = PlacesUtils.favicons - .getFaviconLinkForIcon(NetUtil.newURI(faviconUrl)).spec; - } + match.icon = "page-icon:" + url; + // Although this has a frecency, this query is executed before any other // queries that would result in frecency matches. match.frecency = frecency; @@ -1813,7 +1790,6 @@ Search.prototype = { let strippedUrl = stripPrefix(url); let prefix = url.substr(0, url.length - strippedUrl.length); let frecency = row.getResultByIndex(QUERYINDEX_FRECENCY); - let faviconUrl = row.getResultByIndex(QUERYINDEX_ICONURL); // We must complete the URL up to the next separator (which is /, ? or #). let searchString = stripPrefix(this._trimmedOriginalSearchString); @@ -1835,17 +1811,14 @@ Search.prototype = { style: "autofill" }; - if (faviconUrl) { - match.icon = PlacesUtils.favicons - .getFaviconLinkForIcon(NetUtil.newURI(faviconUrl)).spec; - } - // Complete to the found url only if its untrimmed value preserves the // user's input. if (url.toLowerCase().includes(this._trimmedOriginalSearchString.toLowerCase())) { match.finalCompleteValue = prefix + strippedUrl; } + match.icon = "page-icon:" + (match.finalCompleteValue || match.value); + return match; }, @@ -1855,7 +1828,6 @@ Search.prototype = { let escapedURL = row.getResultByIndex(QUERYINDEX_URL); let openPageCount = row.getResultByIndex(QUERYINDEX_SWITCHTAB) || 0; let historyTitle = row.getResultByIndex(QUERYINDEX_TITLE) || ""; - let iconurl = row.getResultByIndex(QUERYINDEX_ICONURL) || ""; let bookmarked = row.getResultByIndex(QUERYINDEX_BOOKMARKED); let bookmarkTitle = bookmarked ? row.getResultByIndex(QUERYINDEX_BOOKMARKTITLE) : null; @@ -1911,10 +1883,7 @@ Search.prototype = { match.value = url; match.comment = title; - if (iconurl) { - match.icon = PlacesUtils.favicons - .getFaviconLinkForIcon(NetUtil.newURI(iconurl)).spec; - } + match.icon = "page-icon:" + escapedURL; match.frecency = frecency; return match; diff --git a/toolkit/components/places/mozIAsyncFavicons.idl b/toolkit/components/places/mozIAsyncFavicons.idl index f1be18278fad..7b9fb6870b55 100644 --- a/toolkit/components/places/mozIAsyncFavicons.idl +++ b/toolkit/components/places/mozIAsyncFavicons.idl @@ -63,6 +63,7 @@ interface mozIAsyncFavicons : nsISupports in unsigned long aFaviconLoadType, [optional] in nsIFaviconDataCallback aCallback, [optional] in nsIPrincipal aLoadingPrincipal); + /** * Sets the data for a given favicon URI either by replacing existing data in * the database or taking the place of otherwise fetched icon data when @@ -143,6 +144,8 @@ interface mozIAsyncFavicons : nsISupports * This callback is always invoked to notify the result of the lookup. * The aURI parameter will be the favicon URI, or null when no favicon * is associated with the page or an error occurred while fetching it. + * @param aPreferredWidth + * The preferred icon width, 0 for the biggest available. * * @note When the callback is invoked, aDataLen will be always 0, aData will * be an empty array, and aMimeType will be an empty string, regardless @@ -151,10 +154,13 @@ interface mozIAsyncFavicons : nsISupports * @see nsIFaviconDataCallback in nsIFaviconService.idl. */ void getFaviconURLForPage(in nsIURI aPageURI, - in nsIFaviconDataCallback aCallback); + in nsIFaviconDataCallback aCallback, + [optional] in unsigned short aPreferredWidth); /** * Retrieves the favicon URI and data associated to the given page, if any. + * If the page icon is not available, it will try to return the root domain + * icon data, when it's known. * * @param aPageURI * URI of the page whose favicon URI and data we're looking up. @@ -166,9 +172,12 @@ interface mozIAsyncFavicons : nsISupports * However, if no favicon data is currently associated with the favicon * URI, aDataLen will be 0, aData will be an empty array, and aMimeType * will be an empty string. + * @param aPreferredWidth + * The preferred icon width, 0 for the biggest available. * * @see nsIFaviconDataCallback in nsIFaviconService.idl. */ void getFaviconDataForPage(in nsIURI aPageURI, - in nsIFaviconDataCallback aCallback); + in nsIFaviconDataCallback aCallback, + [optional] in unsigned short aPreferredWidth); }; diff --git a/toolkit/components/places/nsAnnoProtocolHandler.cpp b/toolkit/components/places/nsAnnoProtocolHandler.cpp index 2b4a084b03da..99ccb7a7f965 100644 --- a/toolkit/components/places/nsAnnoProtocolHandler.cpp +++ b/toolkit/components/places/nsAnnoProtocolHandler.cpp @@ -33,6 +33,7 @@ #include "mozilla/ScopeExit.h" #include "mozilla/storage.h" #include "Helpers.h" +#include "FaviconHelpers.h" using namespace mozilla; using namespace mozilla::places; @@ -65,22 +66,21 @@ namespace { * HandleResult, and on HandleCompletion, we'll close our output stream which * will close the original channel for the favicon request. * - * However, if an error occurs at any point, we do not set mReturnDefaultIcon to - * false, so we will open up another channel to get the default favicon, and - * pass that along to our output stream in HandleCompletion. If anything - * happens at that point, the world must be against us, so we return nothing. + * However, if an error occurs at any point and we don't have mData, we will + * just fallback to the default favicon. If anything happens at that point, the + * world must be against us, so we can do nothing. */ class faviconAsyncLoader : public AsyncStatementCallback { public: - faviconAsyncLoader(nsIChannel *aChannel, nsIStreamListener *aListener) : - mChannel(aChannel) + faviconAsyncLoader(nsIChannel *aChannel, nsIStreamListener *aListener) + : mChannel(aChannel) , mListener(aListener) { - NS_ASSERTION(aChannel, - "Not providing a channel will result in crashes!"); - NS_ASSERTION(aListener, - "Not providing a stream listener will result in crashes!"); + MOZ_ASSERT(aChannel, "Not providing a channel will result in crashes!"); + MOZ_ASSERT(aListener, "Not providing a stream listener will result in crashes!"); + // Set the default content type. + Unused << mChannel->SetContentType(NS_LITERAL_CSTRING(PNG_MIME_TYPE)); } ////////////////////////////////////////////////////////////////////////////// @@ -88,77 +88,73 @@ public: NS_IMETHOD HandleResult(mozIStorageResultSet *aResultSet) override { - // We will only get one row back in total, so we do not need to loop. nsCOMPtr row; - nsresult rv = aResultSet->GetNextRow(getter_AddRefs(row)); - NS_ENSURE_SUCCESS(rv, rv); + while (NS_SUCCEEDED(aResultSet->GetNextRow(getter_AddRefs(row))) && row) { + // TODO: For now just return the biggest icon, that is the first one. + // Later this should allow to return a specific size. + if (!mData.IsEmpty()) { + return NS_OK; + } - // We do not allow favicons without a MIME type, so we'll return the default - // icon. - nsAutoCString mimeType; - (void)row->GetUTF8String(1, mimeType); - NS_ENSURE_FALSE(mimeType.IsEmpty(), NS_OK); + int32_t width; + nsresult rv = row->GetInt32(1, &width); + NS_ENSURE_SUCCESS(rv, rv); - // Set our mimeType now that we know it. - rv = mChannel->SetContentType(mimeType); - NS_ENSURE_SUCCESS(rv, rv); + // Eventually override the default mimeType for svg. + if (width == UINT16_MAX) { + rv = mChannel->SetContentType(NS_LITERAL_CSTRING(SVG_MIME_TYPE)); + NS_ENSURE_SUCCESS(rv, rv); + } - // Obtain the binary blob that contains our favicon data. - uint8_t *favicon; - uint32_t size = 0; - rv = row->GetBlob(0, &size, &favicon); - NS_ENSURE_SUCCESS(rv, rv); - - nsCOMPtr stream; - rv = NS_NewByteInputStream(getter_AddRefs(stream), - reinterpret_cast(favicon), - size, NS_ASSIGNMENT_ADOPT); - if (NS_FAILED(rv)) { - free(favicon); - return rv; + // Obtain the binary blob that contains our favicon data. + uint8_t *data; + uint32_t dataLen; + rv = row->GetBlob(0, &dataLen, &data); + NS_ENSURE_SUCCESS(rv, rv); + mData.Adopt(TO_CHARBUFFER(data), dataLen); } - RefPtr pump; - rv = nsInputStreamPump::Create(getter_AddRefs(pump), stream, -1, -1, 0, 0, - true); - NS_ENSURE_SUCCESS(rv, rv); - - MOZ_DIAGNOSTIC_ASSERT(mListener); - NS_ENSURE_TRUE(mListener, NS_ERROR_UNEXPECTED); - - rv = pump->AsyncRead(mListener, nullptr); - NS_ENSURE_SUCCESS(rv, rv); - - mListener = nullptr; return NS_OK; } NS_IMETHOD HandleCompletion(uint16_t aReason) override { - // If we've already written our icon data to the channel, there's nothing - // more to do. If we didn't, then return the default icon instead. - if (!mListener) - return NS_OK; + MOZ_DIAGNOSTIC_ASSERT(mListener); + NS_ENSURE_TRUE(mListener, NS_ERROR_UNEXPECTED); + nsresult rv; + // Ensure we'll break possible cycles with the listener. auto cleanup = MakeScopeExit([&] () { mListener = nullptr; }); + if (!mData.IsEmpty()) { + nsCOMPtr stream; + rv = NS_NewCStringInputStream(getter_AddRefs(stream), mData); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + if (NS_SUCCEEDED(rv)) { + RefPtr pump; + rv = nsInputStreamPump::Create(getter_AddRefs(pump), stream, -1, -1, 0, 0, + true); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + if (NS_SUCCEEDED(rv)) { + return pump->AsyncRead(mListener, nullptr); + } + } + } + + // Fallback to the default favicon. // we should pass the loadInfo of the original channel along // to the new channel. Note that mChannel can not be null, // constructor checks that. nsCOMPtr loadInfo = mChannel->GetLoadInfo(); nsCOMPtr newChannel; - nsresult rv = GetDefaultIcon(loadInfo, getter_AddRefs(newChannel)); - + rv = GetDefaultIcon(loadInfo, getter_AddRefs(newChannel)); if (NS_FAILED(rv)) { mListener->OnStartRequest(mChannel, nullptr); mListener->OnStopRequest(mChannel, nullptr, rv); return rv; } - - mChannel->SetContentType(NS_LITERAL_CSTRING("image/png")); - return newChannel->AsyncOpen2(mListener); } @@ -168,6 +164,7 @@ protected: private: nsCOMPtr mChannel; nsCOMPtr mListener; + nsCString mData; }; } // namespace diff --git a/toolkit/components/places/nsFaviconService.cpp b/toolkit/components/places/nsFaviconService.cpp index 3c9444ef3c38..ab807e87d946 100644 --- a/toolkit/components/places/nsFaviconService.cpp +++ b/toolkit/components/places/nsFaviconService.cpp @@ -118,32 +118,31 @@ nsFaviconService::Init() NS_IMETHODIMP nsFaviconService::ExpireAllFavicons() { - nsCOMPtr unlinkIconsStmt = mDB->GetAsyncStatement( - "UPDATE moz_places " - "SET favicon_id = NULL " - "WHERE favicon_id NOT NULL" + NS_ENSURE_STATE(mDB); + + nsCOMPtr removePagesStmt = mDB->GetAsyncStatement( + "DELETE FROM moz_pages_w_icons" ); - NS_ENSURE_STATE(unlinkIconsStmt); + NS_ENSURE_STATE(removePagesStmt); nsCOMPtr removeIconsStmt = mDB->GetAsyncStatement( - "DELETE FROM moz_favicons WHERE id NOT IN (" - "SELECT favicon_id FROM moz_places WHERE favicon_id NOT NULL " - ")" + "DELETE FROM moz_icons" ); NS_ENSURE_STATE(removeIconsStmt); + nsCOMPtr unlinkIconsStmt = mDB->GetAsyncStatement( + "DELETE FROM moz_icons_to_pages" + ); + NS_ENSURE_STATE(unlinkIconsStmt); mozIStorageBaseStatement* stmts[] = { - unlinkIconsStmt.get() + removePagesStmt.get() , removeIconsStmt.get() + , unlinkIconsStmt.get() }; nsCOMPtr ps; RefPtr callback = new ExpireFaviconsStatementCallbackNotifier(); - nsresult rv = mDB->MainConn()->ExecuteAsync( - stmts, ArrayLength(stmts), callback, getter_AddRefs(ps) - ); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; + return mDB->MainConn()->ExecuteAsync(stmts, ArrayLength(stmts), + callback, getter_AddRefs(ps)); } //////////////////////////////////////////////////////////////////////////////// @@ -224,10 +223,12 @@ nsFaviconService::SetAndFetchFaviconForPage(nsIURI* aPageURI, nsresult rv = IsFailedFavicon(aFaviconURI, &previouslyFailed); NS_ENSURE_SUCCESS(rv, rv); if (previouslyFailed) { - if (aForceReload) + if (aForceReload) { RemoveFailedFavicon(aFaviconURI); - else + } + else { return NS_OK; + } } nsCOMPtr loadingPrincipal = aLoadingPrincipal; @@ -248,15 +249,14 @@ nsFaviconService::SetAndFetchFaviconForPage(nsIURI* aPageURI, } NS_ENSURE_TRUE(loadingPrincipal, NS_ERROR_FAILURE); - // Check if the icon already exists and fetch it from the network, if needed. - // Finally associate the icon to the requested page if not yet associated. bool loadPrivate = aFaviconLoadType == nsIFaviconService::FAVICON_LOAD_PRIVATE; + // Build page data. PageData page; rv = aPageURI->GetSpec(page.spec); NS_ENSURE_SUCCESS(rv, rv); // URIs can arguably miss a host. - (void)GetReversedHostname(aPageURI, page.revHost); + Unused << GetReversedHostname(aPageURI, page.revHost); bool canAddToHistory; nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY); @@ -264,7 +264,9 @@ nsFaviconService::SetAndFetchFaviconForPage(nsIURI* aPageURI, NS_ENSURE_SUCCESS(rv, rv); page.canAddToHistory = !!canAddToHistory && !loadPrivate; + // Build icon data. IconData icon; + // If we have an in-memory icon payload, it overwrites the actual request. UnassociatedIconHashKey* iconKey = mUnassociatedIcons.GetEntry(aFaviconURI); if (iconKey) { icon = iconKey->iconData; @@ -276,8 +278,8 @@ nsFaviconService::SetAndFetchFaviconForPage(nsIURI* aPageURI, } // If the page url points to an image, the icon's url will be the same. - // In future evaluate to store a resample of the image. For now avoid that - // for database size concerns. + // TODO (Bug 403651): store a resample of the image. For now avoid that + // for database size and UX concerns. // Don't store favicons for error pages too. if (icon.spec.Equals(page.spec) || icon.spec.Equals(FAVICON_ERRORPAGE_URL)) { @@ -310,8 +312,11 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI, MOZ_ASSERT(NS_IsMainThread()); NS_ENSURE_ARG(aFaviconURI); NS_ENSURE_ARG(aData); - NS_ENSURE_TRUE(aDataLen > 0, NS_ERROR_INVALID_ARG); - NS_ENSURE_TRUE(aMimeType.Length() > 0, NS_ERROR_INVALID_ARG); + NS_ENSURE_ARG(aDataLen > 0); + NS_ENSURE_ARG(aMimeType.Length() > 0); + NS_ENSURE_ARG(imgLoader::SupportImageWithMimeType(PromiseFlatCString(aMimeType).get(), + AcceptedMimeTypes::IMAGES)); + if (aExpiration == 0) { aExpiration = PR_Now() + MAX_FAVICON_EXPIRATION; } @@ -339,22 +344,22 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI, nsresult rv = aFaviconURI->GetSpec(iconData->spec); NS_ENSURE_SUCCESS(rv, rv); - // 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_FAVICON_FILESIZE) { - rv = OptimizeFaviconImage(aData, aDataLen, aMimeType, iconData->data, iconData->mimeType); - NS_ENSURE_SUCCESS(rv, rv); + IconPayload payload; + payload.mimeType = aMimeType; + payload.data.Assign(TO_CHARBUFFER(aData), aDataLen); + // There may already be a previous payload, so ensure to only have one. + iconData->payloads.Clear(); + iconData->payloads.AppendElement(payload); - 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); - return NS_ERROR_FAILURE; - } - } else { - iconData->mimeType.Assign(aMimeType); - iconData->data.Assign(TO_CHARBUFFER(aData), aDataLen); + rv = OptimizeIconSizes(*iconData); + NS_ENSURE_SUCCESS(rv, rv); + + // If there's not valid payload, don't store the icon into to the database. + if ((*iconData).payloads.Length() == 0) { + // 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); + return NS_ERROR_FAILURE; } // If the database contains an icon at the given url, we will update the @@ -464,7 +469,8 @@ nsFaviconService::ReplaceFaviconDataFromDataURL(nsIURI* aFaviconURI, NS_IMETHODIMP nsFaviconService::GetFaviconURLForPage(nsIURI *aPageURI, - nsIFaviconDataCallback* aCallback) + nsIFaviconDataCallback* aCallback, + uint16_t aPreferredWidth) { MOZ_ASSERT(NS_IsMainThread()); NS_ENSURE_ARG(aPageURI); @@ -475,7 +481,7 @@ nsFaviconService::GetFaviconURLForPage(nsIURI *aPageURI, NS_ENSURE_SUCCESS(rv, rv); RefPtr event = - new AsyncGetFaviconURLForPage(pageSpec, aCallback); + new AsyncGetFaviconURLForPage(pageSpec, aPreferredWidth, aCallback); RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); @@ -486,7 +492,8 @@ nsFaviconService::GetFaviconURLForPage(nsIURI *aPageURI, NS_IMETHODIMP nsFaviconService::GetFaviconDataForPage(nsIURI* aPageURI, - nsIFaviconDataCallback* aCallback) + nsIFaviconDataCallback* aCallback, + uint16_t aPreferredWidth) { MOZ_ASSERT(NS_IsMainThread()); NS_ENSURE_ARG(aPageURI); @@ -497,7 +504,7 @@ nsFaviconService::GetFaviconDataForPage(nsIURI* aPageURI, NS_ENSURE_SUCCESS(rv, rv); RefPtr event = - new AsyncGetFaviconDataForPage(pageSpec, aCallback); + new AsyncGetFaviconDataForPage(pageSpec, aPreferredWidth, aCallback); RefPtr DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); DB->DispatchToAsyncThread(event); @@ -587,13 +594,7 @@ nsFaviconService::GetFaviconLinkForIconString(const nsCString& aSpec, nsIURI** aOutput) { if (aSpec.IsEmpty()) { - // default icon for empty strings - if (! mDefaultIcon) { - nsresult rv = NS_NewURI(getter_AddRefs(mDefaultIcon), - NS_LITERAL_CSTRING(FAVICON_DEFAULT_URL)); - NS_ENSURE_SUCCESS(rv, rv); - } - return mDefaultIcon->Clone(aOutput); + return GetDefaultFavicon(aOutput); } if (StringBeginsWith(aSpec, NS_LITERAL_CSTRING("chrome:"))) { @@ -627,44 +628,91 @@ nsFaviconService::GetFaviconSpecForIconString(const nsCString& aSpec, } } - -// nsFaviconService::OptimizeFaviconImage -// -// Given a blob of data (a image file already read into a buffer), optimize -// its size by recompressing it as a 16x16 PNG. +/** + * Checks the icon and evaluates if it needs to be optimized. + * + * @param aIcon + * The icon to be evaluated. + */ nsresult -nsFaviconService::OptimizeFaviconImage(const uint8_t* aData, uint32_t aDataLen, - const nsACString& aMimeType, - nsACString& aNewData, - nsACString& aNewMimeType) +nsFaviconService::OptimizeIconSizes(IconData& aIcon) { - nsresult rv; + // TODO (bug 1346139): move optimization to the async thread. + MOZ_ASSERT(NS_IsMainThread()); + // There should only be a single payload at this point, it may have to be + // split though, if it's an ico file. + MOZ_ASSERT(aIcon.payloads.Length() == 1); + + // Even if the page provides a large image for the favicon (eg, a highres + // image or a multiresolution .ico file), don't try to store more data than + // needed. + IconPayload payload = aIcon.payloads[0]; + if (payload.mimeType.EqualsLiteral(SVG_MIME_TYPE)) { + // Nothing to optimize, but check the payload size. + if (payload.data.Length() >= nsIFaviconService::MAX_FAVICON_BUFFER_SIZE) { + aIcon.payloads.Clear(); + } + return NS_OK; + } + + // Make space for the optimized payloads. + aIcon.payloads.Clear(); nsCOMPtr stream; - rv = NS_NewByteInputStream(getter_AddRefs(stream), - reinterpret_cast(aData), aDataLen, - NS_ASSIGNMENT_DEPEND); + nsresult rv = NS_NewByteInputStream(getter_AddRefs(stream), + payload.data.get(), + payload.data.Length(), + NS_ASSIGNMENT_DEPEND); NS_ENSURE_SUCCESS(rv, rv); // decode image nsCOMPtr container; - rv = GetImgTools()->DecodeImageData(stream, aMimeType, getter_AddRefs(container)); + rv = GetImgTools()->DecodeImageData(stream, payload.mimeType, + getter_AddRefs(container)); NS_ENSURE_SUCCESS(rv, rv); - aNewMimeType.AssignLiteral(PNG_MIME_TYPE); - - // scale and recompress - nsCOMPtr iconStream; - rv = GetImgTools()->EncodeScaledImage(container, aNewMimeType, - DEFAULT_FAVICON_SIZE, - DEFAULT_FAVICON_SIZE, - EmptyString(), - getter_AddRefs(iconStream)); + IconPayload newPayload; + newPayload.mimeType = NS_LITERAL_CSTRING(PNG_MIME_TYPE); + // TODO: for ico files we should extract every single payload. + int32_t width; + rv = container->GetWidth(&width); NS_ENSURE_SUCCESS(rv, rv); - - // Read the stream into a new buffer. - rv = NS_ConsumeStream(iconStream, UINT32_MAX, aNewData); + int32_t height; + rv = container->GetHeight(&height); NS_ENSURE_SUCCESS(rv, rv); + // For non-square images, pick the largest side. + int32_t originalSize = std::max(width, height); + newPayload.width = originalSize; + for (uint16_t size : sFaviconSizes) { + if (size <= originalSize) { + newPayload.width = size; + break; + } + } + + // If the original payload is png and the size is the same, no reason to + // rescale the image. + if (newPayload.mimeType.Equals(payload.mimeType) && + newPayload.width == originalSize) { + newPayload.data = payload.data; + } else { + // scale and recompress + nsCOMPtr iconStream; + rv = GetImgTools()->EncodeScaledImage(container, + newPayload.mimeType, + newPayload.width, + newPayload.width, + EmptyString(), + getter_AddRefs(iconStream)); + NS_ENSURE_SUCCESS(rv, rv); + // Read the stream into the new buffer. + rv = NS_ConsumeStream(iconStream, UINT32_MAX, newPayload.data); + NS_ENSURE_SUCCESS(rv, rv); + } + + if (newPayload.data.Length() < nsIFaviconService::MAX_FAVICON_BUFFER_SIZE) { + aIcon.payloads.AppendElement(newPayload); + } return NS_OK; } @@ -673,18 +721,20 @@ nsresult nsFaviconService::GetFaviconDataAsync(nsIURI* aFaviconURI, mozIStorageStatementCallback *aCallback) { - NS_ASSERTION(aCallback, "Doesn't make sense to call this without a callback"); + MOZ_ASSERT(aCallback, "Doesn't make sense to call this without a callback"); + nsCOMPtr stmt = mDB->GetAsyncStatement( - "SELECT f.data, f.mime_type FROM moz_favicons f WHERE url = :icon_url" + "SELECT data, width FROM moz_icons " + "WHERE fixed_icon_url_hash = hash(fixup_url(:url)) AND icon_url = :url " + "ORDER BY width DESC" ); NS_ENSURE_STATE(stmt); // Ignore the ref part of the URI before querying the database because // we may have added a media fragment for rendering purposes. - nsAutoCString faviconURI; aFaviconURI->GetSpecIgnoringRef(faviconURI); - nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("icon_url"), faviconURI); + nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), faviconURI); NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr pendingStatement; diff --git a/toolkit/components/places/nsFaviconService.h b/toolkit/components/places/nsFaviconService.h index 70220a9bf4c4..f506e7024a2a 100644 --- a/toolkit/components/places/nsFaviconService.h +++ b/toolkit/components/places/nsFaviconService.h @@ -28,12 +28,6 @@ static uint16_t sFaviconSizes[8] = { 256, 192, 144, 96, 64, 48, 32, 16 }; -// Default size when preferred size is unknown, doubled for hi-dpi. -#define DEFAULT_FAVICON_SIZE 32 - -// 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; @@ -95,9 +89,7 @@ public: nsresult GetFaviconLinkForIconString(const nsCString& aIcon, nsIURI** aOutput); void GetFaviconSpecForIconString(const nsCString& aIcon, nsACString& aOutput); - nsresult OptimizeFaviconImage(const uint8_t* aData, uint32_t aDataLen, - const nsACString& aMimeType, - nsACString& aNewData, nsACString& aNewMimeType); + nsresult OptimizeIconSizes(mozilla::places::IconData& aIcon); /** * Obtains the favicon data asynchronously. diff --git a/toolkit/components/places/nsIFaviconService.idl b/toolkit/components/places/nsIFaviconService.idl index 25339d64b5e2..12d187f32ae1 100644 --- a/toolkit/components/places/nsIFaviconService.idl +++ b/toolkit/components/places/nsIFaviconService.idl @@ -19,7 +19,7 @@ interface nsIFaviconService : nsISupports * The limit in bytes of the size of favicons in memory and passed via the * favicon protocol. */ - const unsigned long MAX_FAVICON_BUFFER_SIZE = 10240; + const unsigned long MAX_FAVICON_BUFFER_SIZE = 35840; /** * For a given icon URI, this will return a URI that will result in the image. @@ -121,6 +121,9 @@ interface nsIFaviconDataCallback : nsISupports * Icon data, or an empty array if aDataLen is 0. * @param aMimeType * Mime type of the icon, or an empty string if aDataLen is 0. + * @param aWidth + * Width of the icon. 0 if the width is unknown or if the icon is + * vectorial. * * @note If you want to open a network channel to access the favicon, it's * recommended that you call the getFaviconLinkForIcon method to convert @@ -129,7 +132,8 @@ interface nsIFaviconDataCallback : nsISupports void onComplete(in nsIURI aFaviconURI, in unsigned long aDataLen, [const,array,size_is(aDataLen)] in octet aData, - in AUTF8String aMimeType); + in AUTF8String aMimeType, + in unsigned short aWidth); }; %{C++ diff --git a/toolkit/components/places/nsNavBookmarks.cpp b/toolkit/components/places/nsNavBookmarks.cpp index 31a470a4afa6..c1f0b03725c9 100644 --- a/toolkit/components/places/nsNavBookmarks.cpp +++ b/toolkit/components/places/nsNavBookmarks.cpp @@ -1108,12 +1108,11 @@ nsNavBookmarks::GetDescendantChildren(int64_t aFolderId, // item_child, and folder_child from moz_bookmarks. nsCOMPtr stmt = mDB->GetStatement( "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, " - "h.last_visit_date, f.url, b.id, b.dateAdded, b.lastModified, " + "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, " "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, " "b.guid, b.position, b.type, b.fk, b.syncStatus " "FROM moz_bookmarks b " "LEFT JOIN moz_places h ON b.fk = h.id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE b.parent = :parent " "ORDER BY b.position ASC" ); @@ -2147,12 +2146,11 @@ nsNavBookmarks::QueryFolderChildren( // item_child, and folder_child from moz_bookmarks. nsCOMPtr stmt = mDB->GetStatement( "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, " - "h.last_visit_date, f.url, b.id, b.dateAdded, b.lastModified, " + "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, " "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, " "b.guid, b.position, b.type, b.fk " "FROM moz_bookmarks b " "LEFT JOIN moz_places h ON b.fk = h.id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE b.parent = :parent " "ORDER BY b.position ASC" ); @@ -2285,12 +2283,11 @@ nsNavBookmarks::QueryFolderChildrenAsync( // item_child, and folder_child from moz_bookmarks. nsCOMPtr stmt = mDB->GetAsyncStatement( "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, " - "h.last_visit_date, f.url, b.id, b.dateAdded, b.lastModified, " + "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, " "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, " "b.guid, b.position, b.type, b.fk " "FROM moz_bookmarks b " "LEFT JOIN moz_places h ON b.fk = h.id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE b.parent = :parent " "ORDER BY b.position ASC" ); diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp index 4ba3726e8ddc..c966d7ebcf8f 100644 --- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -1280,7 +1280,7 @@ nsNavHistory::ExecuteQueries(nsINavHistoryQuery** aQueries, uint32_t aQueryCount if (!rootNode) { // Either this is not a folder shortcut, or is a broken one. In both cases // just generate a query node. - rootNode = new nsNavHistoryQueryResultNode(EmptyCString(), EmptyCString(), + rootNode = new nsNavHistoryQueryResultNode(EmptyCString(), queries, options); } @@ -1517,11 +1517,10 @@ PlacesSQLQueryBuilder::SelectAsURI() mQueryString = NS_LITERAL_CSTRING( "SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, " - "h.last_visit_date, f.url, null, null, null, null, ") + + "h.last_visit_date, null, null, null, null, null, ") + tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null " "FROM moz_places h " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " // WHERE 1 is a no-op since additonal conditions will start with AND. "WHERE 1 " "{QUERY_OPTIONS_VISITS} {QUERY_OPTIONS_PLACES} " @@ -1543,7 +1542,7 @@ PlacesSQLQueryBuilder::SelectAsURI() mQueryString = NS_LITERAL_CSTRING( "SELECT b2.fk, h.url, COALESCE(b2.title, h.title) AS page_title, " - "h.rev_host, h.visit_count, h.last_visit_date, f.url, b2.id, " + "h.rev_host, h.visit_count, h.last_visit_date, null, b2.id, " "b2.dateAdded, b2.lastModified, b2.parent, ") + tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null, b2.guid, b2.position, b2.type, b2.fk " @@ -1554,7 +1553,6 @@ PlacesSQLQueryBuilder::SelectAsURI() "WHERE b.type = 1 {ADDITIONAL_CONDITIONS} " ") AS seed ON b2.fk = seed.fk " "JOIN moz_places h ON h.id = b2.fk " - "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE NOT EXISTS ( " "SELECT id FROM moz_bookmarks WHERE id = b2.parent AND parent = ") + nsPrintfCString("%" PRId64, history->GetTagsFolder()) + @@ -1568,13 +1566,12 @@ PlacesSQLQueryBuilder::SelectAsURI() tagsSqlFragment); mQueryString = NS_LITERAL_CSTRING( "SELECT b.fk, h.url, COALESCE(b.title, h.title) AS page_title, " - "h.rev_host, h.visit_count, h.last_visit_date, f.url, b.id, " + "h.rev_host, h.visit_count, h.last_visit_date, null, b.id, " "b.dateAdded, b.lastModified, b.parent, ") + tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid," "null, null, null, b.guid, b.position, b.type, b.fk " "FROM moz_bookmarks b " "JOIN moz_places h ON b.fk = h.id " - "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE NOT EXISTS " "(SELECT id FROM moz_bookmarks " "WHERE id = b.parent AND parent = ") + @@ -1602,12 +1599,11 @@ PlacesSQLQueryBuilder::SelectAsVisit() tagsSqlFragment); mQueryString = NS_LITERAL_CSTRING( "SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, " - "v.visit_date, f.url, null, null, null, null, ") + + "v.visit_date, null, null, null, null, null, ") + tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "v.id, v.from_visit, v.visit_type " "FROM moz_places h " "JOIN moz_historyvisits v ON h.id = v.place_id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " // WHERE 1 is a no-op since additonal conditions will start with AND. "WHERE 1 " "{QUERY_OPTIONS_VISITS} {QUERY_OPTIONS_PLACES} " @@ -2131,11 +2127,10 @@ nsNavHistory::ConstructQueryString( // smart bookmark. queryString = NS_LITERAL_CSTRING( "SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, h.last_visit_date, " - "f.url, null, null, null, null, ") + + "null, null, null, null, null, ") + tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null " "FROM moz_places h " - "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE h.hidden = 0 " "AND EXISTS (SELECT id FROM moz_historyvisits WHERE place_id = h.id " "AND visit_type NOT IN ") + @@ -3797,11 +3792,6 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow, uint32_t accessCount = aRow->AsInt32(kGetInfoIndex_VisitCount); PRTime time = aRow->AsInt64(kGetInfoIndex_VisitDate); - // favicon - nsAutoCString favicon; - rv = aRow->GetUTF8String(kGetInfoIndex_FaviconURL, favicon); - NS_ENSURE_SUCCESS(rv, rv); - // itemId int64_t itemId = aRow->AsInt64(kGetInfoIndex_ItemId); int64_t parentId = -1; @@ -3842,7 +3832,7 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow, } RefPtr resultNode; - rv = QueryRowToResult(itemId, guid, url, title, accessCount, time, favicon, + rv = QueryRowToResult(itemId, guid, url, title, accessCount, time, getter_AddRefs(resultNode)); NS_ENSURE_SUCCESS(rv, rv); @@ -3864,7 +3854,7 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow, } else if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_URI || aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) { RefPtr resultNode = - new nsNavHistoryResultNode(url, title, accessCount, time, favicon); + new nsNavHistoryResultNode(url, title, accessCount, time); if (itemId != -1) { resultNode->mItemId = itemId; @@ -3896,7 +3886,7 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow, if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_VISIT) { RefPtr resultNode = - new nsNavHistoryResultNode(url, title, accessCount, time, favicon); + new nsNavHistoryResultNode(url, title, accessCount, time); nsAutoString tags; rv = aRow->GetString(kGetInfoIndex_ItemTags, tags); @@ -3937,8 +3927,8 @@ nsNavHistory::QueryRowToResult(int64_t itemId, const nsACString& aBookmarkGuid, const nsACString& aURI, const nsACString& aTitle, - uint32_t aAccessCount, PRTime aTime, - const nsACString& aFavicon, + uint32_t aAccessCount, + PRTime aTime, nsNavHistoryResultNode** aNode) { MOZ_ASSERT((itemId != -1 && !aBookmarkGuid.IsEmpty()) || @@ -3981,8 +3971,7 @@ nsNavHistory::QueryRowToResult(int64_t itemId, } else { // This is a regular query. - resultNode = new nsNavHistoryQueryResultNode(aTitle, EmptyCString(), - aTime, queries, options); + resultNode = new nsNavHistoryQueryResultNode(aTitle, aTime, queries, options); resultNode->mItemId = itemId; } } @@ -3992,7 +3981,7 @@ nsNavHistory::QueryRowToResult(int64_t itemId, // This is a broken query, that either did not parse or points to not // existing data. We don't want to return failure since that will kill the // whole result. Instead make a generic empty query node. - resultNode = new nsNavHistoryQueryResultNode(aTitle, aFavicon, aURI); + resultNode = new nsNavHistoryQueryResultNode(aTitle, aURI); resultNode->mItemId = itemId; // This is a perf hack to generate an empty query that skips filtering. resultNode->GetAsQuery()->Options()->SetExcludeItems(true); @@ -4026,12 +4015,11 @@ nsNavHistory::VisitIdToResultNode(int64_t visitId, // Should match kGetInfoIndex_* (see GetQueryResults) statement = mDB->GetStatement(NS_LITERAL_CSTRING( "SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, " - "v.visit_date, f.url, null, null, null, null, " + "v.visit_date, null, null, null, null, null, " ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "v.id, v.from_visit, v.visit_type " "FROM moz_places h " "JOIN moz_historyvisits v ON h.id = v.place_id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE v.id = :visit_id ") ); break; @@ -4041,12 +4029,11 @@ nsNavHistory::VisitIdToResultNode(int64_t visitId, // Should match kGetInfoIndex_* (see GetQueryResults) statement = mDB->GetStatement(NS_LITERAL_CSTRING( "SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, " - "h.last_visit_date, f.url, null, null, null, null, " + "h.last_visit_date, null, null, null, null, null, " ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null " "FROM moz_places h " "JOIN moz_historyvisits v ON h.id = v.place_id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE v.id = :visit_id ") ); break; @@ -4087,13 +4074,12 @@ nsNavHistory::BookmarkIdToResultNode(int64_t aBookmarkId, nsNavHistoryQueryOptio // Should match kGetInfoIndex_* nsCOMPtr stmt = mDB->GetStatement(NS_LITERAL_CSTRING( "SELECT b.fk, h.url, COALESCE(b.title, h.title), " - "h.rev_host, h.visit_count, h.last_visit_date, f.url, b.id, " + "h.rev_host, h.visit_count, h.last_visit_date, null, b.id, " "b.dateAdded, b.lastModified, b.parent, " ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null, b.guid, b.position, b.type, b.fk " "FROM moz_bookmarks b " "JOIN moz_places h ON b.fk = h.id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE b.id = :item_id ") ); NS_ENSURE_STATE(stmt); @@ -4128,13 +4114,12 @@ nsNavHistory::URIToResultNode(nsIURI* aURI, // Should match kGetInfoIndex_* nsCOMPtr stmt = mDB->GetStatement(NS_LITERAL_CSTRING( "SELECT h.id, :page_url, COALESCE(b.title, h.title), " - "h.rev_host, h.visit_count, h.last_visit_date, f.url, " + "h.rev_host, h.visit_count, h.last_visit_date, null, " "b.id, b.dateAdded, b.lastModified, b.parent, " ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, " "null, null, null, b.guid, b.position, b.type, b.fk " "FROM moz_places h " "LEFT JOIN moz_bookmarks b ON b.fk = h.id " - "LEFT JOIN moz_favicons f ON h.favicon_id = f.id " "WHERE h.url_hash = hash(:page_url) AND h.url = :page_url ") ); NS_ENSURE_STATE(stmt); diff --git a/toolkit/components/places/nsNavHistory.h b/toolkit/components/places/nsNavHistory.h index bfe13b13dd10..160052c8b1a3 100644 --- a/toolkit/components/places/nsNavHistory.h +++ b/toolkit/components/places/nsNavHistory.h @@ -258,8 +258,8 @@ public: const nsACString& aBookmarkGuid, const nsACString& aURI, const nsACString& aTitle, - uint32_t aAccessCount, PRTime aTime, - const nsACString& aFavicon, + uint32_t aAccessCount, + PRTime aTime, nsNavHistoryResultNode** aNode); nsresult VisitIdToResultNode(int64_t visitId, diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index c59338948281..82f70bd16831 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -88,14 +88,13 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(nsNavHistoryResultNode) nsNavHistoryResultNode::nsNavHistoryResultNode( const nsACString& aURI, const nsACString& aTitle, uint32_t aAccessCount, - PRTime aTime, const nsACString& aIconURI) : + PRTime aTime) : mParent(nullptr), mURI(aURI), mTitle(aTitle), mAreTagsSorted(false), mAccessCount(aAccessCount), mTime(aTime), - mFaviconURI(aIconURI), mBookmarkIndex(-1), mItemId(-1), mFolderId(-1), @@ -115,14 +114,13 @@ nsNavHistoryResultNode::nsNavHistoryResultNode( NS_IMETHODIMP nsNavHistoryResultNode::GetIcon(nsACString& aIcon) { - if (mFaviconURI.IsEmpty()) { - aIcon.Truncate(); + if (this->IsContainer() || mURI.IsEmpty()) { return NS_OK; } - nsFaviconService* faviconService = nsFaviconService::GetFaviconService(); - NS_ENSURE_TRUE(faviconService, NS_ERROR_OUT_OF_MEMORY); - faviconService->GetFaviconSpecForIconString(mFaviconURI, aIcon); + aIcon.AppendLiteral("page-icon:"); + aIcon.Append(mURI); + return NS_OK; } @@ -345,10 +343,9 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(nsNavHistoryContainerResultNod NS_INTERFACE_MAP_END_INHERITING(nsNavHistoryResultNode) nsNavHistoryContainerResultNode::nsNavHistoryContainerResultNode( - const nsACString& aURI, const nsACString& aTitle, - const nsACString& aIconURI, uint32_t aContainerType, + const nsACString& aURI, const nsACString& aTitle, uint32_t aContainerType, nsNavHistoryQueryOptions* aOptions) : - nsNavHistoryResultNode(aURI, aTitle, 0, 0, aIconURI), + nsNavHistoryResultNode(aURI, aTitle, 0, 0), mResult(nullptr), mContainerType(aContainerType), mExpanded(false), @@ -359,10 +356,9 @@ nsNavHistoryContainerResultNode::nsNavHistoryContainerResultNode( nsNavHistoryContainerResultNode::nsNavHistoryContainerResultNode( const nsACString& aURI, const nsACString& aTitle, - PRTime aTime, - const nsACString& aIconURI, uint32_t aContainerType, + PRTime aTime, uint32_t aContainerType, nsNavHistoryQueryOptions* aOptions) : - nsNavHistoryResultNode(aURI, aTitle, 0, aTime, aIconURI), + nsNavHistoryResultNode(aURI, aTitle, 0, aTime), mResult(nullptr), mContainerType(aContainerType), mExpanded(false), @@ -1763,9 +1759,8 @@ NS_IMPL_ISUPPORTS_INHERITED(nsNavHistoryQueryResultNode, nsINavHistoryQueryResultNode) nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( - const nsACString& aTitle, const nsACString& aIconURI, - const nsACString& aQueryURI) : - nsNavHistoryContainerResultNode(aQueryURI, aTitle, aIconURI, + const nsACString& aTitle, const nsACString& aQueryURI) : + nsNavHistoryContainerResultNode(aQueryURI, aTitle, nsNavHistoryResultNode::RESULT_TYPE_QUERY, nullptr), mLiveUpdate(QUERYUPDATE_COMPLEX_WITH_BOOKMARKS), @@ -1776,10 +1771,9 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( } nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( - const nsACString& aTitle, const nsACString& aIconURI, - const nsCOMArray& aQueries, + const nsACString& aTitle, const nsCOMArray& aQueries, nsNavHistoryQueryOptions* aOptions) : - nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aIconURI, + nsNavHistoryContainerResultNode(EmptyCString(), aTitle, nsNavHistoryResultNode::RESULT_TYPE_QUERY, aOptions), mQueries(aQueries), @@ -1808,11 +1802,11 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( } nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( - const nsACString& aTitle, const nsACString& aIconURI, + const nsACString& aTitle, PRTime aTime, const nsCOMArray& aQueries, nsNavHistoryQueryOptions* aOptions) : - nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aTime, aIconURI, + nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aTime, nsNavHistoryResultNode::RESULT_TYPE_QUERY, aOptions), mQueries(aQueries), @@ -2715,12 +2709,9 @@ nsNavHistoryQueryResultNode::OnClearHistory() static nsresult setFaviconCallback(nsNavHistoryResultNode* aNode, - const void* aClosure, + const void * aClosure, const nsNavHistoryResult* aResult) { - const nsCString* newFavicon = static_cast(aClosure); - aNode->mFaviconURI = *newFavicon; - if (aResult && (!aNode->mParent || aNode->mParent->AreChildrenVisible())) NOTIFY_RESULT_OBSERVERS(aResult, NodeIconChanged(aNode)); @@ -2740,13 +2731,12 @@ nsNavHistoryQueryResultNode::OnPageChanged(nsIURI* aURI, switch (aChangedAttribute) { case nsINavHistoryObserver::ATTRIBUTE_FAVICON: { - NS_ConvertUTF16toUTF8 newFavicon(aNewValue); bool onlyOneEntry = (mOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_URI || mOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS); UpdateURIs(true, onlyOneEntry, false, spec, setFaviconCallback, - &newFavicon); + nullptr); break; } default: @@ -3031,7 +3021,7 @@ NS_IMPL_ISUPPORTS_INHERITED(nsNavHistoryFolderResultNode, nsNavHistoryFolderResultNode::nsNavHistoryFolderResultNode( const nsACString& aTitle, nsNavHistoryQueryOptions* aOptions, int64_t aFolderId) : - nsNavHistoryContainerResultNode(EmptyCString(), aTitle, EmptyCString(), + nsNavHistoryContainerResultNode(EmptyCString(), aTitle, nsNavHistoryResultNode::RESULT_TYPE_FOLDER, aOptions), mContentsValid(false), @@ -3766,7 +3756,6 @@ nsNavHistoryResultNode::OnItemChanged(int64_t aItemId, NOTIFY_RESULT_OBSERVERS(result, NodeURIChanged(this, mURI)); } else if (aProperty.EqualsLiteral("favicon")) { - mFaviconURI = aNewValue; if (shouldNotify) NOTIFY_RESULT_OBSERVERS(result, NodeIconChanged(this)); } @@ -4007,7 +3996,7 @@ nsNavHistoryFolderResultNode::OnItemMoved(int64_t aItemId, */ nsNavHistorySeparatorResultNode::nsNavHistorySeparatorResultNode() : nsNavHistoryResultNode(EmptyCString(), EmptyCString(), - 0, 0, EmptyCString()) + 0, 0) { } diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index 6b6f96284a30..faaba2b475d8 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -257,8 +257,7 @@ class nsNavHistoryResultNode : public nsINavHistoryResultNode { public: nsNavHistoryResultNode(const nsACString& aURI, const nsACString& aTitle, - uint32_t aAccessCount, PRTime aTime, - const nsACString& aIconURI); + uint32_t aAccessCount, PRTime aTime); NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYRESULTNODE_IID) @@ -367,7 +366,6 @@ public: bool mAreTagsSorted; uint32_t mAccessCount; int64_t mTime; - nsCString mFaviconURI; int32_t mBookmarkIndex; int64_t mItemId; int64_t mFolderId; @@ -436,13 +434,10 @@ class nsNavHistoryContainerResultNode : public nsNavHistoryResultNode, public: nsNavHistoryContainerResultNode( const nsACString& aURI, const nsACString& aTitle, - const nsACString& aIconURI, uint32_t aContainerType, - nsNavHistoryQueryOptions* aOptions); + uint32_t aContainerType, nsNavHistoryQueryOptions* aOptions); nsNavHistoryContainerResultNode( const nsACString& aURI, const nsACString& aTitle, - PRTime aTime, - const nsACString& aIconURI, uint32_t aContainerType, - nsNavHistoryQueryOptions* aOptions); + PRTime aTime, uint32_t aContainerType, nsNavHistoryQueryOptions* aOptions); virtual nsresult Refresh(); @@ -625,14 +620,11 @@ class nsNavHistoryQueryResultNode final : public nsNavHistoryContainerResultNode { public: nsNavHistoryQueryResultNode(const nsACString& aTitle, - const nsACString& aIconURI, const nsACString& aQueryURI); nsNavHistoryQueryResultNode(const nsACString& aTitle, - const nsACString& aIconURI, const nsCOMArray& aQueries, nsNavHistoryQueryOptions* aOptions); nsNavHistoryQueryResultNode(const nsACString& aTitle, - const nsACString& aIconURI, PRTime aTime, const nsCOMArray& aQueries, nsNavHistoryQueryOptions* aOptions); diff --git a/toolkit/components/places/nsPlacesExpiration.js b/toolkit/components/places/nsPlacesExpiration.js index 77d486f6e12c..4a19c6263b72 100644 --- a/toolkit/components/places/nsPlacesExpiration.js +++ b/toolkit/components/places/nsPlacesExpiration.js @@ -269,15 +269,24 @@ const EXPIRATION_QUERIES = { ACTION.DEBUG }, + // Expire orphan pages from the icons database. + QUERY_EXPIRE_FAVICONS_PAGES: { + sql: `DELETE FROM moz_pages_w_icons + WHERE page_url_hash NOT IN ( + SELECT url_hash FROM moz_places + )`, + actions: ACTION.TIMED_OVERLIMIT | ACTION.CLEAR_HISTORY | + ACTION.SHUTDOWN_DIRTY | ACTION.IDLE_DIRTY | ACTION.IDLE_DAILY | + ACTION.DEBUG + }, + // Expire orphan icons from the database. QUERY_EXPIRE_FAVICONS: { - sql: `DELETE FROM moz_favicons WHERE id IN ( - SELECT f.id FROM moz_favicons f - LEFT JOIN moz_places h ON f.id = h.favicon_id - WHERE h.favicon_id IS NULL - LIMIT :limit_favicons + sql: `DELETE FROM moz_icons + WHERE id NOT IN ( + SELECT icon_id FROM moz_icons_to_pages )`, - actions: ACTION.TIMED | ACTION.TIMED_OVERLIMIT | ACTION.CLEAR_HISTORY | + actions: ACTION.TIMED_OVERLIMIT | ACTION.CLEAR_HISTORY | ACTION.SHUTDOWN_DIRTY | ACTION.IDLE_DIRTY | ACTION.IDLE_DAILY | ACTION.DEBUG }, @@ -996,9 +1005,6 @@ nsPlacesExpiration.prototype = { case "QUERY_SILENT_EXPIRE_ORPHAN_URIS": params.limit_uris = baseLimit; break; - case "QUERY_EXPIRE_FAVICONS": - params.limit_favicons = baseLimit; - break; case "QUERY_EXPIRE_ANNOS": // Each page may have multiple annos. params.limit_annos = baseLimit * EXPIRE_AGGRESSIVITY_MULTIPLIER; diff --git a/toolkit/components/places/tests/browser/browser_favicon_setAndFetchFaviconForPage_failures.js b/toolkit/components/places/tests/browser/browser_favicon_setAndFetchFaviconForPage_failures.js index cc775f364ae3..4c8ef6133eb7 100644 --- a/toolkit/components/places/tests/browser/browser_favicon_setAndFetchFaviconForPage_failures.js +++ b/toolkit/components/places/tests/browser/browser_favicon_setAndFetchFaviconForPage_failures.js @@ -38,7 +38,7 @@ function test() { }); function checkFavIconsDBCount(aCallback) { - let stmt = DBConn().createAsyncStatement("SELECT url FROM moz_favicons"); + let stmt = DBConn().createAsyncStatement("SELECT icon_url FROM moz_icons"); stmt.executeAsync({ handleResult: function final_handleResult(aResultSet) { while (aResultSet.getNextRow()) { @@ -50,7 +50,7 @@ function test() { }, handleCompletion: function final_handleCompletion(aReason) { // begin testing - info("Previous records in moz_favicons: " + favIconsResultCount); + info("Previous records in moz_icons: " + favIconsResultCount); if (aCallback) { aCallback(); } @@ -173,11 +173,11 @@ function test() { function final_callback() { // Check that only one record corresponding to the last favicon is present. let resultCount = 0; - let stmt = DBConn().createAsyncStatement("SELECT url FROM moz_favicons"); + let stmt = DBConn().createAsyncStatement("SELECT icon_url FROM moz_icons"); stmt.executeAsync({ handleResult: function final_handleResult(aResultSet) { - // If the moz_favicons DB had been previously loaded (before our + // If the moz_icons DB had been previously loaded (before our // test began), we should focus only in the URI we are testing and // skip the URIs not related to our test. if (favIconsResultCount > 0) { diff --git a/toolkit/components/places/tests/chrome/chrome.ini b/toolkit/components/places/tests/chrome/chrome.ini index 5ac753e7306f..e770461d4025 100644 --- a/toolkit/components/places/tests/chrome/chrome.ini +++ b/toolkit/components/places/tests/chrome/chrome.ini @@ -1,4 +1,5 @@ [DEFAULT] +support-files = head.js [test_303567.xul] [test_341972a.xul] @@ -9,4 +10,4 @@ [test_favicon_annotations.xul] [test_reloadLivemarks.xul] [test_browser_disableglobalhistory.xul] -support-files = browser_disableglobalhistory.xul \ No newline at end of file +support-files = browser_disableglobalhistory.xul diff --git a/toolkit/components/places/tests/chrome/head.js b/toolkit/components/places/tests/chrome/head.js new file mode 100644 index 000000000000..4ce78a715ed9 --- /dev/null +++ b/toolkit/components/places/tests/chrome/head.js @@ -0,0 +1,10 @@ +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); + +XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils", + "resource://testing-common/PlacesTestUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils", + "resource://gre/modules/PlacesUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Task", + "resource://gre/modules/Task.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil", + "resource://gre/modules/NetUtil.jsm"); diff --git a/toolkit/components/places/tests/chrome/test_favicon_annotations.xul b/toolkit/components/places/tests/chrome/test_favicon_annotations.xul index 48b61032aef5..18278cb235ae 100644 --- a/toolkit/components/places/tests/chrome/test_favicon_annotations.xul +++ b/toolkit/components/places/tests/chrome/test_favicon_annotations.xul @@ -17,6 +17,7 @@ src="chrome://mochikit/content/tests/SimpleTest/WindowSnapshot.js"/>