tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit f0e0295fbc6abf83ab26b2e781922c51abc8d8bb
parent 2cf6abd54c1cbb56ad123ceebac555f38752b09b
Author: Marco Bonardo <mbonardo@mozilla.com>
Date:   Wed,  3 Dec 2025 15:22:04 +0000

Bug 1959694 - Shrink favicons database by removing favicon relations expired months ago. r=daisuke,places-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D274669

Diffstat:
Mtoolkit/components/places/FaviconHelpers.cpp | 30++++++++++++++++--------------
Mtoolkit/components/places/FaviconHelpers.h | 7+++----
Mtoolkit/components/places/PlacesExpiration.sys.mjs | 33+++++++++++++++++++++++++++++++++
Mtoolkit/components/places/tests/expiration/test_debug_expiration.js | 53+++++++++++++++++++++++++++++++++++++++++++++++++----
4 files changed, 101 insertions(+), 22 deletions(-)

diff --git a/toolkit/components/places/FaviconHelpers.cpp b/toolkit/components/places/FaviconHelpers.cpp @@ -690,14 +690,12 @@ AsyncAssociateIconToPage::Run() { MOZ_ASSERT(mPage.canAddToHistory || !mPage.bookmarkedSpec.IsEmpty(), "The page should be addable to history or a bookmark"); - bool shouldUpdateIcon = mIcon.status & ICON_STATUS_CHANGED; - if (!shouldUpdateIcon) { - for (const auto& payload : mIcon.payloads) { - // If the entry is missing from the database, we should add it. - if (payload.id == 0) { - shouldUpdateIcon = true; - break; - } + bool shouldUpdateIcon = false; + for (const auto& payload : mIcon.payloads) { + // If the entry is missing from the database, we should add it. + if (payload.id == 0) { + shouldUpdateIcon = true; + break; } } @@ -787,10 +785,9 @@ AsyncAssociateIconToPage::Run() { stmt = DB->GetStatement( "INSERT INTO moz_icons_to_pages (page_id, icon_id, expire_ms) " "VALUES ((SELECT id from moz_pages_w_icons WHERE page_url_hash = " - "hash(:page_url) AND page_url = :page_url), " - ":icon_id, :expire) " + " hash(:page_url) AND page_url = :page_url), :icon_id, :expire) " "ON CONFLICT(page_id, icon_id) DO " - "UPDATE SET expire_ms = :expire "); + " UPDATE SET expire_ms = :expire "); NS_ENSURE_STATE(stmt); // For some reason using BindingParamsArray here fails execution, so we must @@ -1036,11 +1033,13 @@ NS_IMETHODIMP AsyncTryCopyFaviconsRunnable::Run() { // Create the relations. nsCOMPtr<mozIStorageStatement> stmt = DB->GetStatement( - "INSERT OR IGNORE INTO moz_icons_to_pages (page_id, icon_id, expire_ms) " + "INSERT INTO moz_icons_to_pages (page_id, icon_id, expire_ms) " "SELECT :id, icon_id, expire_ms " "FROM moz_icons_to_pages " - "WHERE page_id = (SELECT id FROM moz_pages_w_icons WHERE page_url_hash = " - "hash(:url) AND page_url = :url) "); + "WHERE page_id = (SELECT id FROM moz_pages_w_icons " + " WHERE page_url_hash = hash(:url) AND page_url = :url) " + "ON CONFLICT (page_id, icon_id) DO " + " UPDATE SET expire_ms = max(excluded.expire_ms, :min_expiration_ms)"); NS_ENSURE_STATE(stmt); mozStorageStatementScoper scoper(stmt); rv = stmt->BindInt64ByName("id"_ns, toPageData.id); @@ -1049,6 +1048,9 @@ NS_IMETHODIMP AsyncTryCopyFaviconsRunnable::Run() { mFromPageURI->GetSpec(fromPageSpec); rv = URIBinder::Bind(stmt, "url"_ns, fromPageSpec); NS_ENSURE_SUCCESS(rv, rv); + rv = stmt->BindInt64ByName("min_expiration_ms"_ns, + (PR_Now() + MIN_FAVICON_EXPIRATION) / 1000); + NS_ENSURE_SUCCESS(rv, rv); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); diff --git a/toolkit/components/places/FaviconHelpers.h b/toolkit/components/places/FaviconHelpers.h @@ -23,10 +23,9 @@ class nsIPrincipal; #include "mozilla/ipc/IPCCore.h" #define ICON_STATUS_UNKNOWN 0 -#define ICON_STATUS_CHANGED 1 << 0 -#define ICON_STATUS_SAVED 1 << 1 -#define ICON_STATUS_ASSOCIATED 1 << 2 -#define ICON_STATUS_CACHED 1 << 3 +#define ICON_STATUS_SAVED 1 << 0 +#define ICON_STATUS_ASSOCIATED 1 << 1 +#define ICON_STATUS_CACHED 1 << 2 #define TO_CHARBUFFER(_buffer) \ reinterpret_cast<char*>(const_cast<uint8_t*>(_buffer)) diff --git a/toolkit/components/places/PlacesExpiration.sys.mjs b/toolkit/components/places/PlacesExpiration.sys.mjs @@ -254,6 +254,39 @@ const EXPIRATION_QUERIES = { ACTION.DEBUG, }, + // Expire icon relations for pages not visited in the last six months. + // The six-month threshold is arbitrary: the history view groups visits older + // than six months under a "Older than 6 months" entry, so losing icons in + // such a large list would be acceptable. + // We only remove relations whose expire_ms is at least six months older than + // the page's last visit, indicating the relation has not been refreshed by + // visits for a long time and is likely stale. + // We also exclude bookmarked pages and root icons. + // We proceed cautiously because these icons might still be in use; + // they are only potentially stale. + // Orphaned icons are removed by subsequent expiration queries. + QUERY_EXPIRE_OLD_FAVICON_RELATIONS: { + sql: ` + DELETE FROM moz_icons_to_pages + WHERE (page_id, icon_id) IN ( + SELECT page_id, icon_id + FROM moz_icons_to_pages ip + JOIN moz_icons i ON i.id = icon_id + JOIN moz_pages_w_icons pi ON pi.id = page_id + JOIN moz_places ON url_hash = page_url_hash + WHERE + last_visit_date BETWEEN + strftime('%s', ip.expire_ms / 1000, 'unixepoch', '+6 months', 'localtime', 'utc') * 1000000 + AND strftime('%s', 'now', 'localtime', '-6 months', 'utc') * 1000000 + AND root = 0 + AND foreign_count = 0 + ORDER BY last_visit_date ASC + LIMIT 100 + ) + `, + actions: ACTION.IDLE_DIRTY | ACTION.IDLE_DAILY | ACTION.DEBUG, + }, + // Expire from favicons any page that has only relations older than 180 days, // if the page is not bookmarked, and we have a root icon that can be used // as a placeholder until the page is visited again. diff --git a/toolkit/components/places/tests/expiration/test_debug_expiration.js b/toolkit/components/places/tests/expiration/test_debug_expiration.js @@ -287,6 +287,7 @@ add_task(async function test_expire_icons() { "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAA" + "AAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg=="; + const now = new Date(); const entries = [ { desc: "Not expired because recent", @@ -342,11 +343,31 @@ add_task(async function test_expire_icons() { iconExpired: false, removed: true, }, + { + desc: "Expired because old relation and no recent visit", + page: "https://oldrelation.expired.org/", + icon: "https://oldrelation.expired.org/test_icon.png", + removed: true, + visitDate: new Date(now.setMonth(now.getMonth() - 7)), + relationDate: new Date(now.setMonth(now.getMonth() - 14)), + }, + { + desc: "Not expired even if old relation because bookmark", + page: "https://oldrelation.notexpired.org/", + icon: "https://oldrelation.notexpired.org/test_icon.png", + removed: false, + bookmarked: true, + visitDate: new Date(now.setMonth(now.getMonth() - 7)), + relationDate: new Date(now.setMonth(now.getMonth() - 14)), + }, ]; for (let entry of entries) { if (!entry.skipHistory) { - await PlacesTestUtils.addVisits(entry.page); + await PlacesTestUtils.addVisits({ + url: entry.page, + visitDate: entry.visitDate || new Date(), + }); } if (entry.bookmarked) { await PlacesUtils.bookmarks.insert({ @@ -394,6 +415,7 @@ add_task(async function test_expire_icons() { } }); } + if (entry.icon) { let favicon = await PlacesTestUtils.getFaviconForPage(entry.page); Assert.equal( @@ -402,14 +424,37 @@ add_task(async function test_expire_icons() { "Sanity check the initial icon value" ); } + + if (entry.relationDate) { + // Set an old date on the icon-page relation. + await PlacesUtils.withConnectionWrapper( + "setFaviconPageRelationDate", + async db => { + await db.execute( + `UPDATE moz_icons_to_pages + SET expire_ms = :date + WHERE page_id = (SELECT id FROM moz_pages_w_icons + WHERE page_url_hash = hash(:page_url) + AND page_url = :page_url) + AND icon_id = (SELECT id FROM moz_icons WHERE icon_url = :icon_url) + `, + { + date: entry.relationDate.getTime(), + page_url: entry.page, + icon_url: entry.icon, + } + ); + } + ); + } } - info("Run expiration"); - await promiseForceExpirationStep(-1); + info("Run expiration, but don't expire pages"); + await promiseForceExpirationStep(0); info("Check expiration"); for (let entry of entries) { - Assert.ok(page_in_database(entry.page)); + Assert.ok(page_in_database(entry.page), `Page in database: ${entry.page}`); let favicon = await PlacesTestUtils.getFaviconForPage(entry.page); if (!entry.removed) {