author | Marco Bonardo <mbonardo@mozilla.com> |
Tue, 18 Apr 2017 23:51:05 +0200 | |
changeset 354013 | a6bbb991c0d2d13c34d675df39eec6db9155fbbf |
parent 354012 | ecc42c31cc283151f6a452af171045687023cf75 |
child 354014 | ad7d2ae91ce67d5cdc71e36293d8978235269c10 |
push id | 41142 |
push user | mak77@bonardo.net |
push date | Thu, 20 Apr 2017 09:40:41 +0000 |
treeherder | autoland@a6bbb991c0d2 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | adw |
bugs | 1357555 |
milestone | 55.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/toolkit/components/places/FaviconHelpers.cpp +++ b/toolkit/components/places/FaviconHelpers.cpp @@ -154,16 +154,17 @@ FetchPageInfo(const RefPtr<Database>& aD nsresult SetIconInfo(const RefPtr<Database>& aDB, IconData& aIcon, bool aMustReplace = false) { MOZ_ASSERT(!NS_IsMainThread()); MOZ_ASSERT(aIcon.payloads.Length() > 0); MOZ_ASSERT(!aIcon.spec.IsEmpty()); + MOZ_ASSERT(aIcon.expiration > 0); // 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. @@ -514,18 +515,17 @@ AsyncFetchAndSetIconForPage::Run() MOZ_ASSERT(!NS_IsMainThread()); // Try to fetch the icon from the database. RefPtr<Database> DB = Database::GetDatabase(); NS_ENSURE_STATE(DB); nsresult rv = FetchIconInfo(DB, 0, mIcon); NS_ENSURE_SUCCESS(rv, rv); - bool isInvalidIcon = !mIcon.payloads.Length() || - (mIcon.expiration && PR_Now() > mIcon.expiration); + bool isInvalidIcon = !mIcon.payloads.Length() || PR_Now() > mIcon.expiration; bool fetchIconFromNetwork = mIcon.fetchMode == FETCH_ALWAYS || (mIcon.fetchMode == FETCH_IF_MISSING && isInvalidIcon); if (!fetchIconFromNetwork) { // There is already a valid icon or we don't want to fetch a new one, // directly proceed with association. RefPtr<AsyncAssociateIconToPage> event = new AsyncAssociateIconToPage(mIcon, mPage, mCallback);
new file mode 100644 --- /dev/null +++ b/toolkit/components/places/tests/favicons/test_expire_migrated_icons.js @@ -0,0 +1,27 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * This file tests that favicons migrated from a previous profile, having a 0 + * expiration, will be properly expired when fetching new ones. + */ + +add_task(function* test_storing_a_normal_16x16_icon() { + const PAGE_URL = "http://places.test"; + yield PlacesTestUtils.addVisits(PAGE_URL); + yield setFaviconForPage(PAGE_URL, SMALLPNG_DATA_URI); + + // Now set expiration to 0 and change the payload. + do_print("Set expiration to 0 and replace favicon data"); + yield PlacesUtils.withConnectionWrapper("Change favicons payload", db => { + return db.execute(`UPDATE moz_icons SET expire_ms = 0, data = "test"`); + }); + + let {data, mimeType} = yield getFaviconDataForPage(PAGE_URL); + Assert.equal(mimeType, "image/png"); + Assert.deepEqual(data, "test".split("").map(c => c.charCodeAt(0))); + + do_print("Refresh favicon"); + yield setFaviconForPage(PAGE_URL, SMALLPNG_DATA_URI, false); + yield compareFavicons("page-icon:" + PAGE_URL, SMALLPNG_DATA_URI); +});
--- a/toolkit/components/places/tests/favicons/xpcshell.ini +++ b/toolkit/components/places/tests/favicons/xpcshell.ini @@ -19,16 +19,17 @@ support-files = favicon-multi-frame32.png favicon-multi-frame64.png favicon-normal16.png favicon-normal32.png favicon-scale160x3.jpg favicon-scale3x160.jpg [test_expireAllFavicons.js] +[test_expire_migrated_icons.js] [test_expire_on_new_icons.js] [test_favicons_conversions.js] [test_favicons_protocols_ref.js] [test_getFaviconDataForPage.js] [test_getFaviconURLForPage.js] [test_moz-anno_favicon_mime_type.js] [test_multiple_frames.js] [test_page-icon_protocol.js]
--- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -858,25 +858,27 @@ function sortBy(array, prop) { } /** * Asynchronously set the favicon associated with a page. * @param page * The page's URL * @param icon * The URL of the favicon to be set. + * @param [optional] forceReload + * Whether to enforce reloading the icon. */ -function setFaviconForPage(page, icon) { +function setFaviconForPage(page, icon, forceReload = true) { let pageURI = page instanceof Ci.nsIURI ? page : NetUtil.newURI(new URL(page).href); let iconURI = icon instanceof Ci.nsIURI ? icon : NetUtil.newURI(new URL(icon).href); return new Promise(resolve => { PlacesUtils.favicons.setAndFetchFaviconForPage( - pageURI, iconURI, true, + pageURI, iconURI, forceReload, PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, resolve, Services.scriptSecurityManager.getSystemPrincipal() ); }); } function getFaviconUrlForPage(page, width = 0) {