Backed out changeset 3507329c5999 (bug 1357555o) for eslint failure in test_expire_migrated_icons.js
authorIris Hsiao <ihsiao@mozilla.com>
Thu, 20 Apr 2017 16:23:59 +0800
changeset 354032 6941882b6708a70cc3a4c2d7fdd0e8faf713b7bd
parent 354031 3507329c599976fcf7f7ab17dd9004aaf64a328d
child 354033 12f943a6d12c1fa531e2834bf6185f539c1d2072
push id31685
push userkwierso@gmail.com
push dateThu, 20 Apr 2017 21:45:29 +0000
treeherdermozilla-central@5e3dc7e1288a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone55.0a1
backs out3507329c599976fcf7f7ab17dd9004aaf64a328d
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
Backed out changeset 3507329c5999 (bug 1357555o) for eslint failure in test_expire_migrated_icons.js
toolkit/components/places/FaviconHelpers.cpp
toolkit/components/places/tests/favicons/test_expire_migrated_icons.js
toolkit/components/places/tests/favicons/xpcshell.ini
toolkit/components/places/tests/head_common.js
--- a/toolkit/components/places/FaviconHelpers.cpp
+++ b/toolkit/components/places/FaviconHelpers.cpp
@@ -154,17 +154,16 @@ 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.
@@ -515,17 +514,18 @@ 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() || PR_Now() > mIcon.expiration;
+  bool isInvalidIcon = !mIcon.payloads.Length() ||
+                       (mIcon.expiration && 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);
deleted file mode 100644
--- a/toolkit/components/places/tests/favicons/test_expire_migrated_icons.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/* 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,17 +19,16 @@ 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,27 +858,25 @@ 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, forceReload = true) {
+function setFaviconForPage(page, icon) {
   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, forceReload,
+      pageURI, iconURI, true,
       PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
       resolve,
       Services.scriptSecurityManager.getSystemPrincipal()
     );
   });
 }
 
 function getFaviconUrlForPage(page, width = 0) {