Open Bug 2057815 Opened 24 days ago Updated 15 days ago

AsyncCopyFavicons copies and renews expired icon relations, growing moz_icons_to_pages without bound on pushState-heavy sites

Categories

(Toolkit :: Places, defect)

Firefox 153
x86_64
macOS
defect

Tracking

()

ASSIGNED

People

(Reporter: peter, Assigned: peter, NeedInfo)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:153.0) Gecko/20100101 Firefox/153.0

Steps to reproduce:

How it was found (accumulates over months):

  1. Use Firefox as a daily driver with a large history.
  2. Browse YouTube regularly. YouTube is an SPA — every Shorts/watch navigation is a pushState — and it versions its favicon URL on each deploy (https://www.youtube.com/s/desktop/<hash>/img/favicon_32.png, favicon_144.png, older https://s.ytimg.com/yts/img/favicon_*-vfl<hash>.png).
  3. After some months, check the profile: ls -lh favicons.sqlite
  4. Open new tabs and watch the Top Sites tiles render.

Deterministic repro (minutes):

  1. Serve over HTTP (copying is blocked for file://), e.g. python3 -m http.server, with spa.html:
    <link rel="icon" href="/icon.png?v=1">
    <script>
      for (let i = 0; i < 50; i++) history.pushState({}, "", "/page-" + i);
    </script>
    
  2. Load http://localhost:8000/spa.html.
  3. Bump v=1v=2 (simulates a deploy shipping a new favicon URL) and reload. Repeat ~10 times.
  4. Count the relations:
    sqlite3 "file:<profile>/favicons.sqlite?immutable=1" "SELECT count(*) FROM moz_icons_to_pages;"
    

Automated repro: three regression tests in toolkit/components/places/tests/favicons/test_copyFavicons.js (attached patch) fail on an unpatched build and pass with the fix.

Actual results:

favicons.sqlite grew to 1.2 GB, and opening a new tab intermittently froze the entire browser UI for ~10 seconds with the macOS spinning-wait cursor. Intermittent because it only stalls when the table's pages have been evicted from the OS file cache.

Breakdown by table (via dbstat) on the affected profile:

Table Rows Disk
moz_icons_to_pages 56,736,496 1.22 GB (~99% of file)
moz_icons 9,286 49 MB
moz_pages_w_icons 88,342 10 MB

All 56.7M mapping rows are distinct (icon_id, page_id) pairs — not duplicates. Forensics on that profile:

  • Two unrelated pages, https://www.youtube.com/shorts/NxEqWhTh3Go and https://www.youtube.com/watch?v=Quthm48dLyg, hold byte-identical 1,468-icon sets (100% overlap). Independent per-page accumulation cannot produce identical sets; copying can.
  • 56,659,385 of the 56,736,496 relations are already expired — only 77,111 are live.
  • The one page with 1,468 live relations has only 4 distinct expire_ms values, i.e. they were renewed in bulk rather than genuinely re-observed.
  • 41,119 pages hold ≥1000 relations each, accounting for 53.1M rows (94% of the table).

nsDocShell::CopyFavicon runs on same-document navigations — pushState/replaceState (nsDocShell.cpp:11132) and anchor navigation (nsDocShell.cpp:8278) — and reaches AsyncCopyFavicons (toolkit/components/places/FaviconHelpers.cpp), which:

  1. copies every relation of the source page, including long-expired ones;
  2. never prunes the destination's expired relations; and
  3. via ON CONFLICT (page_id, icon_id) DO UPDATE SET expire_ms = max(excluded.expire_ms, :min_expiration_ms), renews every matched row to now + 1 day, so an actively-browsed set never decays.

So each in-page navigation hands the entire accumulated icon set to the next URL, and the set only ever grows.

The existing cleanup is not a meaningful sink: QUERY_EXPIRE_OLD_FAVICON_RELATIONS (PlacesExpiration.sys.mjs) only removes relations for pages unvisited in 6 months and is capped at LIMIT 50 per idle-daily run — roughly 3,000 years to drain 56M rows.

The New Tab / Top Sites path then queries this table to resolve tile favicons (FetchMostFrecentSubPageIcon joins moz_pages_w_iconsmoz_icons_to_pagesmoz_iconsmoz_places, ordered by frecency); on a cache-cold 1.2 GB table this blocks for seconds.

Expected results:

moz_icons_to_pages should stay bounded per page — a page needs at most a handful of current icons, not every icon its domain has ever served.

Specifically, AsyncCopyFavicons should apply the same expiration hygiene that AsyncAssociateIconToPage already applies when associating an icon:

  • Relations whose expire_ms has elapsed should not be propagated to the destination.
  • The destination's own elapsed relations should be dropped before inserting.
  • Copying should carry over a relation's real expiration rather than renewing it, so every relation dies within MAX_FAVICON_EXPIRATION of its last genuine association.

That bounds each page to its domain's live icon set (at most 7 days of versions). On the affected profile this is the difference between ~1,468 and ~1 relation per page.

Additionally, opening a new tab should never block the main thread for seconds on favicon lookup, and existing bloated profiles need a remediation path — the current expiration query is far too slow to drain them.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Navigation' component, but is not confident enough to move the bug to that component.

Component: Untriaged → General
Component: General → Places
OS: Unspecified → macOS
Product: Firefox → Toolkit
Hardware: Unspecified → x86_64

nsDocShell::CopyFavicon runs on same-document navigations (pushState and
anchor navigation), so AsyncCopyFavicons handed the source page's entire
relation set to the destination. It copied elapsed relations, never pruned
the destination's own elapsed relations, and its ON CONFLICT clause renewed
every matched row to now + MIN_FAVICON_EXPIRATION, so an actively browsed
set never decayed.

A site that versions its favicon url per deploy and navigates with pushState
therefore accumulated every icon it ever served on every page it navigated
to. One profile reached 56.7M rows in moz_icons_to_pages (1.2GB, ~99% of
favicons.sqlite), with unrelated pages holding identical 1468-icon sets and
56.6M of the relations already expired.

Apply the same expiration hygiene AsyncAssociateIconToPage already applies
when associating an icon, and carry over a relation's real expiration rather
than renewing it, so a relation dies within MAX_FAVICON_EXPIRATION of its
last genuine association.

Assignee: nobody → peter
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

The severity field is not set for this bug.
:mak, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(mak)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: