Bug 1592976 - Expire some favicons more aggressively. r=Standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 06 Mar 2020 15:39:39 +0000
changeset 517250 02f45e7e36b060b86d22bf8cecb30fce592bfb30
parent 517249 ed7eaba537577cff4eca4306efafee753910d71f
child 517251 22c4b944eb92d862f4d83fff50efe599286a2e1a
push id37188
push userncsoregi@mozilla.com
push dateFri, 06 Mar 2020 20:10:41 +0000
treeherdermozilla-central@8ab81c8e93ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1592976
milestone75.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
Bug 1592976 - Expire some favicons more aggressively. r=Standard8 Expire favicons older than 6 months when: * they are for permanently redirecting urls, that are unlikely to receive updated favicons * they are for urls with refs (often mail, docs) that have a fallback root favicon for their origin Expiration happens in chunks, mostly on idle-daily. Differential Revision: https://phabricator.services.mozilla.com/D65308
toolkit/components/places/PlacesExpiration.jsm
toolkit/components/places/tests/expiration/test_debug_expiration.js
--- a/toolkit/components/places/PlacesExpiration.jsm
+++ b/toolkit/components/places/PlacesExpiration.jsm
@@ -249,21 +249,61 @@ const EXPIRATION_QUERIES = {
       ACTION.TIMED |
       ACTION.TIMED_OVERLIMIT |
       ACTION.SHUTDOWN_DIRTY |
       ACTION.IDLE_DIRTY |
       ACTION.IDLE_DAILY |
       ACTION.DEBUG,
   },
 
+  // Expire old favicons for:
+  //  - urls that permanently redirect.
+  //  - urls with ref, when the origin has a root favicon that can be used as
+  //    a fallback.
+  // This deletes pages instead of icons, because icons may be referenced by
+  // multiple pages. The moz_pages_to_icons entries are removed by the table's
+  // FOREIGN KEY, while orphan icons are removed by one of the next queries.
+  QUERY_EXPIRE_OLD_FAVICONS: {
+    sql: `DELETE FROM moz_pages_w_icons WHERE id IN (
+            SELECT DISTINCT page_id FROM moz_icons i
+            JOIN moz_icons_to_pages ON icon_id = i.id
+            JOIN moz_pages_w_icons p ON page_id = p.id
+            JOIN moz_places h ON h.url_hash = page_url_hash
+            JOIN moz_origins o ON o.id = h.origin_id
+            WHERE root = 0
+              AND h.foreign_count = 0
+              AND expire_ms BETWEEN 1 AND strftime('%s','now','localtime','start of day','-180 days','utc') * 1000
+              AND (
+              h.id IN (
+                SELECT v.place_id
+                FROM moz_historyvisits v
+                JOIN moz_historyvisits v_dest on v_dest.from_visit = v.id
+                WHERE v_dest.visit_type = 5
+              )
+              OR (
+                INSTR(page_url, '#') >= 0
+                AND EXISTS(SELECT id FROM moz_icons WHERE root = 1 AND fixed_icon_url_hash = hash(fixup_url(o.host) || '/favicon.ico'))
+              )
+            )
+            LIMIT 100
+          )`,
+    actions:
+      ACTION.SHUTDOWN_DIRTY |
+      ACTION.IDLE_DIRTY |
+      ACTION.IDLE_DAILY |
+      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
+          ) OR id NOT IN (
+            SELECT DISTINCT page_id FROM moz_icons_to_pages
           )`,
     actions:
       ACTION.TIMED_OVERLIMIT |
       ACTION.SHUTDOWN_DIRTY |
       ACTION.IDLE_DIRTY |
       ACTION.IDLE_DAILY |
       ACTION.DEBUG,
   },
@@ -346,17 +386,18 @@ const EXPIRATION_QUERIES = {
       ACTION.IDLE_DAILY |
       ACTION.DEBUG,
   },
 
   // Select entries for notifications.
   // If p_id is set whole_entry = 1, then we have expired the full page.
   // Either p_id or v_id are always set.
   QUERY_SELECT_NOTIFICATIONS: {
-    sql: `SELECT url, guid, MAX(visit_date) AS visit_date,
+    sql: `/* do not warn (bug no): temp table has no index */
+          SELECT url, guid, MAX(visit_date) AS visit_date,
                  MAX(IFNULL(MIN(p_id, 1), MIN(v_id, 0))) AS whole_entry,
                  MAX(expected_results) AS expected_results,
                  (SELECT MAX(visit_date) FROM expiration_notify
                   WHERE reason = "expired" AND url = n.url AND p_id ISNULL
                  ) AS most_recent_expired_visit
           FROM expiration_notify n
           GROUP BY url`,
     actions:
--- a/toolkit/components/places/tests/expiration/test_debug_expiration.js
+++ b/toolkit/components/places/tests/expiration/test_debug_expiration.js
@@ -217,16 +217,136 @@ add_task(async function test_expire_unli
   Assert.ok(!page_in_database("http://old.mozilla.org/"));
   Assert.ok(!page_in_database("http://download.mozilla.org/"));
   Assert.ok(!page_in_database("http://new.mozilla.org/"));
 
   // Clean up.
   await PlacesUtils.history.clear();
 });
 
+add_task(async function test_expire_icons() {
+  const dataUrl =
+    "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAA" +
+    "AAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==";
+
+  const entries = [
+    {
+      desc: "Expired because it redirects",
+      page: "http://source.old.org/",
+      icon: "http://source.old.org/test_icon.png",
+      expire: new Date().setYear(2018) * 1000,
+      redirect: "http://dest.old.org/",
+      removed: true,
+    },
+    {
+      desc: "Not expired because recent",
+      page: "http://source.new.org/",
+      icon: "http://source.new.org/test_icon.png",
+      expire: 0,
+      redirect: "http://dest.new.org/",
+      removed: false,
+    },
+    {
+      desc: "Not expired because does not match, even if old",
+      page: "http://stay.moz.org/",
+      icon: "http://stay.moz.org/test_icon.png",
+      expire: new Date().setYear(2018) * 1000,
+      removed: false,
+    },
+    {
+      desc: "Not expired because does not have a root icon, even if old",
+      page: "http://noroot.ref.org/#test",
+      icon: "http://noroot.ref.org/test_icon.png",
+      expire: new Date().setYear(2018) * 1000,
+      removed: false,
+    },
+    {
+      desc: "Expired because has a root icon",
+      page: "http://root.ref.org/#test",
+      icon: "http://root.ref.org/test_icon.png",
+      root: "http://root.ref.org/favicon.ico",
+      expire: new Date().setYear(2018) * 1000,
+      removed: true,
+    },
+    {
+      desc: "Not expired because recent",
+      page: "http://new.ref.org/#test",
+      icon: "http://new.ref.org/test_icon.png",
+      expire: 0,
+      root: "http://new.ref.org/favicon.ico",
+      removed: false,
+    },
+  ];
+
+  for (let entry of entries) {
+    if (entry.redirect) {
+      await PlacesTestUtils.addVisits(entry.page);
+      await PlacesTestUtils.addVisits({
+        uri: entry.redirect,
+        transition: TRANSITION_REDIRECT_PERMANENT,
+        referrer: entry.page,
+      });
+    } else {
+      await PlacesTestUtils.addVisits(entry.page);
+    }
+
+    PlacesUtils.favicons.replaceFaviconDataFromDataURL(
+      Services.io.newURI(entry.icon),
+      dataUrl,
+      entry.expire,
+      Services.scriptSecurityManager.getSystemPrincipal()
+    );
+    await PlacesTestUtils.addFavicons(new Map([[entry.page, entry.icon]]));
+    Assert.equal(
+      await getFaviconUrlForPage(entry.page),
+      entry.icon,
+      "Sanity check the icon exists"
+    );
+
+    if (entry.root) {
+      PlacesUtils.favicons.replaceFaviconDataFromDataURL(
+        Services.io.newURI(entry.root),
+        dataUrl,
+        entry.expire,
+        Services.scriptSecurityManager.getSystemPrincipal()
+      );
+      await PlacesTestUtils.addFavicons(new Map([[entry.page, entry.root]]));
+    }
+  }
+
+  info("Run expiration");
+  await promiseForceExpirationStep(-1);
+
+  info("Check expiration");
+  // Remove the root icons before checking the associated icons have been expired.
+  await PlacesUtils.withConnectionWrapper("test_debug_expiration.js", db =>
+    db.execute(`DELETE FROM moz_icons WHERE root = 1`)
+  );
+  for (let entry of entries) {
+    Assert.ok(page_in_database(entry.page));
+
+    if (entry.removed) {
+      await Assert.rejects(
+        getFaviconUrlForPage(entry.page),
+        /Unable to find an icon/,
+        entry.desc
+      );
+    } else {
+      Assert.equal(
+        await getFaviconUrlForPage(entry.page),
+        entry.icon,
+        entry.desc
+      );
+    }
+  }
+
+  // Clean up.
+  await PlacesUtils.history.clear();
+});
+
 function run_test() {
   // Set interval to a large value so we don't expire on it.
   setInterval(3600); // 1h
   // Set maxPages to a low value, so it's easy to go over it.
   setMaxPages(1);
 
   run_next_test();
 }