AsyncCopyFavicons copies and renews expired icon relations, growing moz_icons_to_pages without bound on pushState-heavy sites
Categories
(Toolkit :: Places, defect)
Tracking
()
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):
- Use Firefox as a daily driver with a large history.
- 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, olderhttps://s.ytimg.com/yts/img/favicon_*-vfl<hash>.png). - After some months, check the profile:
ls -lh favicons.sqlite - Open new tabs and watch the Top Sites tiles render.
Deterministic repro (minutes):
- Serve over HTTP (copying is blocked for
file://), e.g.python3 -m http.server, withspa.html:<link rel="icon" href="/icon.png?v=1"> <script> for (let i = 0; i < 50; i++) history.pushState({}, "", "/page-" + i); </script> - Load
http://localhost:8000/spa.html. - Bump
v=1→v=2(simulates a deploy shipping a new favicon URL) and reload. Repeat ~10 times. - 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/NxEqWhTh3Goandhttps://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_msvalues, 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:
- copies every relation of the source page, including long-expired ones;
- never prunes the destination's expired relations; and
- via
ON CONFLICT (page_id, icon_id) DO UPDATE SET expire_ms = max(excluded.expire_ms, :min_expiration_ms), renews every matched row tonow + 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_icons ⨝ moz_icons_to_pages ⨝ moz_icons ⨝ moz_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_mshas 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_EXPIRATIONof 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.
Comment 1•24 days ago
|
||
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.
| Assignee | ||
Updated•24 days ago
|
| Assignee | ||
Comment 2•24 days ago
|
||
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.
Updated•24 days ago
|
Comment 3•15 days ago
|
||
The severity field is not set for this bug.
:mak, could you have a look please?
For more information, please visit BugBot documentation.
Description
•