commit 596d8db2a1a0c7864ca3ada9eb810490d0d2babb
parent b8e2b3b04fd931a2ba0db57510b76a39ab030e2e
Author: James Teow <jteow@mozilla.com>
Date: Tue, 11 Nov 2025 02:37:13 +0000
Bug 1986285 - Part 3: Replace fixed frecency thresholds with places API in New Tab code - r=mlplyler,home-newtab-reviewers,thecount
Originally, I wanted to have this patch land prior to frecency graduation but the issue
was setting a threshold that kept the exact same behaviour for old frecency and had
similar results for new frecency.
To minimize the impact to New Tab / Top Sites, I chose a threshold that prevents
certain first page visits (e.g. clicking a link on a website, the browser
automatically opening a URL on a new build) from passing the threshold.
Differential Revision: https://phabricator.services.mozilla.com/D264737
Diffstat:
7 files changed, 156 insertions(+), 31 deletions(-)
diff --git a/browser/components/topsites/TopSites.sys.mjs b/browser/components/topsites/TopSites.sys.mjs
@@ -1,6 +1,10 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+// eslint-disable-next-line mozilla/use-static-import
+const { AppConstants } = ChromeUtils.importESModule(
+ "resource://gre/modules/AppConstants.sys.mjs"
+);
import {
getDomain,
@@ -33,9 +37,23 @@ ChromeUtils.defineLazyGetter(lazy, "log", () => {
return new Logger("TopSites");
});
+ChromeUtils.defineLazyGetter(lazy, "pageFrecencyThreshold", () => {
+ // @backward-compat { version 147 }
+ // Frecency was changed in 147 Nightly. This is a pre-cautionary measure
+ // for train-hopping.
+ if (Services.vc.compare(AppConstants.MOZ_APP_VERSION, "147.0a1") >= 0) {
+ // 30 days ago, 5 visits. The threshold avoids one non-typed visit from
+ // immediately being included in recent history to mimic the original
+ // threshold which aimed to prevent first-run visits from being included in
+ // Top Sites.
+ return lazy.PlacesUtils.history.pageFrecencyThreshold(30, 5, false);
+ }
+ // The old threshold used for classic frecency: Slightly over one visit.
+ return 101;
+});
+
export const DEFAULT_TOP_SITES = [];
-const FRECENCY_THRESHOLD = 100 + 1; // 1 visit (skip first-run/one-time pages)
const MIN_FAVICON_SIZE = 96;
const PINNED_FAVICON_PROPS_TO_MIGRATE = [
"favicon",
@@ -619,7 +637,7 @@ class _TopSites {
cache = await this.frecentCache.request({
// We need to overquery due to the top 5 alexa search + default search possibly being removed
numItems: numItems + SEARCH_FILTERS.length + 1,
- topsiteFrecency: FRECENCY_THRESHOLD,
+ topsiteFrecency: lazy.pageFrecencyThreshold,
});
} catch (ex) {
cache = [];
diff --git a/browser/components/topsites/test/unit/test_top_sites.js b/browser/components/topsites/test/unit/test_top_sites.js
@@ -21,7 +21,8 @@ ChromeUtils.defineESModuleGetters(this, {
const FAKE_FAVICON = "data987";
const FAKE_FAVICON_SIZE = 128;
-const FAKE_FRECENCY = 200;
+// Two visits on the same day.
+const FAKE_FRECENCY = PlacesUtils.history.pageFrecencyThreshold(0, 2, false);
const FAKE_LINKS = new Array(2 * TOP_SITES_MAX_SITES_PER_ROW)
.fill(null)
.map((v, i) => ({
diff --git a/browser/extensions/newtab/lib/TopSitesFeed.sys.mjs b/browser/extensions/newtab/lib/TopSitesFeed.sys.mjs
@@ -2,6 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+// We use importESModule here instead of static import so that
+// the Karma test environment won't choke on this module. This
+// is because the Karma test environment already stubs out
+// AppConstants, and overrides importESModule to be a no-op (which
+// can't be done for a static import statement).
+
+// eslint-disable-next-line mozilla/use-static-import
+const { AppConstants } = ChromeUtils.importESModule(
+ "resource://gre/modules/AppConstants.sys.mjs"
+);
+
import {
actionCreators as ac,
actionTypes as at,
@@ -31,6 +42,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
ObliviousHTTP: "resource://gre/modules/ObliviousHTTP.sys.mjs",
PageThumbs: "resource://gre/modules/PageThumbs.sys.mjs",
PersistentCache: "resource://newtab/lib/PersistentCache.sys.mjs",
+ PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
Region: "resource://gre/modules/Region.sys.mjs",
RemoteSettings: "resource://services-settings/remote-settings.sys.mjs",
Sampling: "resource://gre/modules/components-utils/Sampling.sys.mjs",
@@ -44,10 +56,23 @@ ChromeUtils.defineLazyGetter(lazy, "log", () => {
return new Logger("TopSitesFeed");
});
+ChromeUtils.defineLazyGetter(lazy, "pageFrecencyThreshold", () => {
+ // @backward-compat { version 147 }
+ // Frecency was changed in 147 Nightly.
+ if (Services.vc.compare(AppConstants.MOZ_APP_VERSION, "147.0a1") >= 0) {
+ // 30 days ago, 5 visits. The threshold avoids one non-typed visit from
+ // immediately being included in recent history to mimic the original
+ // threshold which aimed to prevent first-run visits from being included in
+ // Top Sites.
+ return lazy.PlacesUtils.history.pageFrecencyThreshold(30, 5, false);
+ }
+ // The old threshold used for classic frecency: Slightly over one visit.
+ return 101;
+});
+
const DEFAULT_SITES_PREF = "default.sites";
const SHOWN_ON_NEWTAB_PREF = "feeds.topsites";
export const DEFAULT_TOP_SITES = [];
-const FRECENCY_THRESHOLD = 100 + 1; // 1 visit (skip first-run/one-time pages)
const MIN_FAVICON_SIZE = 96;
const CACHED_LINK_PROPS_TO_MIGRATE = ["screenshot", "customScreenshot"];
const PINNED_FAVICON_PROPS_TO_MIGRATE = [
@@ -1328,7 +1353,7 @@ export class TopSitesFeed {
const cache = await this.frecentCache.request({
// We need to overquery due to the top 5 alexa search + default search possibly being removed
numItems: numFetch + SEARCH_FILTERS.length + 1,
- topsiteFrecency: FRECENCY_THRESHOLD,
+ topsiteFrecency: lazy.pageFrecencyThreshold,
});
for (let link of cache) {
const hostname = lazy.NewTabUtils.shortURL(link);
diff --git a/browser/extensions/newtab/test/unit/unit-entry.js b/browser/extensions/newtab/test/unit/unit-entry.js
@@ -198,6 +198,7 @@ const TEST_GLOBAL = {
insert() {},
markPageAsTyped() {},
removeObserver() {},
+ pageFrecencyThreshold() {},
},
"@mozilla.org/io/string-input-stream;1": {
createInstance() {
@@ -445,6 +446,20 @@ const TEST_GLOBAL = {
createNullPrincipal() {},
getSystemPrincipal() {},
},
+ vc: {
+ compare(a, b) {
+ // Rather than re-write Services.vc.compare completely, do
+ // a simple comparison of the major version.
+ // This means this function will give the wrong output for differences
+ // in minor versions, but should be sufficient for unit tests.
+ let majorA = parseInt(a, 10);
+ let majorB = parseInt(b, 10);
+ if (majorA === majorB) {
+ return 0;
+ }
+ return majorA > majorB ? 1 : -1;
+ },
+ },
wm: {
getMostRecentWindow: () => window,
getMostRecentBrowserWindow: () => window,
diff --git a/browser/extensions/newtab/test/xpcshell/test_TopSitesFeed.js b/browser/extensions/newtab/test/xpcshell/test_TopSitesFeed.js
@@ -12,6 +12,7 @@ ChromeUtils.defineESModuleGetters(this, {
NewTabUtils: "resource://gre/modules/NewTabUtils.sys.mjs",
NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
PageThumbs: "resource://gre/modules/PageThumbs.sys.mjs",
+ PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
sinon: "resource://testing-common/Sinon.sys.mjs",
Screenshots: "resource://newtab/lib/Screenshots.sys.mjs",
Sampling: "resource://gre/modules/components-utils/Sampling.sys.mjs",
@@ -23,7 +24,7 @@ ChromeUtils.defineESModuleGetters(this, {
const FAKE_FAVICON = "data987";
const FAKE_FAVICON_SIZE = 128;
-const FAKE_FRECENCY = 200;
+const FAKE_FRECENCY = PlacesUtils.history.pageFrecencyThreshold(0, 2, false);
const FAKE_LINKS = new Array(2 * TOP_SITES_MAX_SITES_PER_ROW)
.fill(null)
.map((v, i) => ({
diff --git a/toolkit/modules/NewTabUtils.sys.mjs b/toolkit/modules/NewTabUtils.sys.mjs
@@ -8,6 +8,11 @@ import {
SEARCH_SHORTCUTS_EXPERIMENT,
} from "moz-src:///toolkit/components/search/SearchShortcuts.sys.mjs";
+// eslint-disable-next-line mozilla/use-static-import
+const { AppConstants } = ChromeUtils.importESModule(
+ "resource://gre/modules/AppConstants.sys.mjs"
+);
+
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
@@ -48,7 +53,18 @@ const LINKS_GET_LINKS_LIMIT = 100;
const TOPIC_GATHER_TELEMETRY = "gather-telemetry";
// Some default frecency threshold for Activity Stream requests
-const ACTIVITY_STREAM_DEFAULT_FRECENCY = 150;
+ChromeUtils.defineLazyGetter(lazy, "pageFrecencyThreshold", () => {
+ // @backward-compat { version 147 }
+ // Frecency was graduated in 147 Nightly.
+ if (Services.vc.compare(AppConstants.MOZ_APP_VERSION, "147.0a1") >= 0) {
+ // 30 days ago, 7 visits. The threshold avoids one non-typed visit from
+ // immediately being included in recent history and is slightly higher than
+ // Top Sites to mimic how this had a higher threshold.
+ return lazy.PlacesUtils.history.pageFrecencyThreshold(30, 7, false);
+ }
+ // The old threshold used for classic frecency.
+ return 150;
+});
// Some default query limit for Activity Stream requests
const ACTIVITY_STREAM_DEFAULT_LIMIT = 12;
@@ -999,7 +1015,7 @@ var ActivityStreamProvider = {
{
ignoreBlocked: false,
numItems: ACTIVITY_STREAM_DEFAULT_LIMIT,
- topsiteFrecency: ACTIVITY_STREAM_DEFAULT_FRECENCY,
+ topsiteFrecency: lazy.pageFrecencyThreshold,
onePerDomain: true,
includeFavicon: true,
hideWithSearchParam: Services.prefs.getCharPref(
@@ -1127,6 +1143,9 @@ var ActivityStreamProvider = {
url => url.match(/:\/\/(?:www\.)?([^\/]+)/),
// Combine frecencies when deduping these links
(targetLink, otherLink) => {
+ // TODO: Experiment with max() vs sum() for frecency combination.
+ // Current additive approach may bias toward sites requiring
+ // deduplication.
targetLink.frecency = link.frecency + otherLink.frecency;
}
);
diff --git a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -13,6 +13,14 @@ const { PlacesUtils } = ChromeUtils.importESModule(
"resource://gre/modules/PlacesUtils.sys.mjs"
);
+// 0 days ago, 1 visit. This is to mimic the expectation of the original test,
+// which is that a single visit should not be filtered when running tests.
+const FRECENCY_THRESHOLD = PlacesUtils.history.pageFrecencyThreshold(
+ 0,
+ 1,
+ false
+);
+
// const SEARCH_SHORTCUTS_EXPERIMENT_PREF =
// "browser.newtabpage.activity-stream.improvesearch.topSiteSearchShortcuts";
// Services.prefs.setBoolPref(SEARCH_SHORTCUTS_EXPERIMENT_PREF, false);
@@ -93,6 +101,22 @@ function getHistorySize() {
);
}
+async function getFrecencyForUrl(url) {
+ let db = await PlacesUtils.promiseDBConnection();
+ let rows = await db.executeCached(
+ `
+ SELECT frecency
+ FROM moz_places
+ WHERE url = :url
+ `,
+ {
+ url,
+ }
+ );
+ Assert.equal(rows?.length, 1, "Should have found a result.");
+ return rows[0].getResultByName("frecency");
+}
+
add_task(async function validCacheMidPopulation() {
let expectedLinks = makeLinks(0, 3, 1);
@@ -720,7 +744,9 @@ add_task(async function getTopFrecentSites() {
await setUpActivityStreamTest();
let provider = NewTabUtils.activityStreamLinks;
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.equal(links.length, 0, "empty history yields empty links");
// add a visit
@@ -734,7 +760,7 @@ add_task(async function getTopFrecentSites() {
"adding a single visit doesn't exceed default threshold"
);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "adding a visit yields a link");
Assert.equal(links[0].url, testURI, "added visit corresponds to added url");
});
@@ -757,7 +783,9 @@ add_task(
await PlacesTestUtils.addVisits(testURI);
let provider = NewTabUtils.activityStreamLinks;
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.equal(
links.length,
1,
@@ -777,7 +805,9 @@ add_task(async function getTopFrecentSites_no_dedup() {
await setUpActivityStreamTest();
let provider = NewTabUtils.activityStreamLinks;
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.equal(links.length, 0, "empty history yields empty links");
// Add a visits in reverse order they will be returned in when not deduped.
@@ -794,7 +824,7 @@ add_task(async function getTopFrecentSites_no_dedup() {
"adding a single visit doesn't exceed default threshold"
);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "adding a visit yields a link");
// Plain domain is returned when deduped.
Assert.equal(
@@ -804,7 +834,7 @@ add_task(async function getTopFrecentSites_no_dedup() {
);
links = await provider.getTopSites({
- topsiteFrecency: 100,
+ topsiteFrecency: FRECENCY_THRESHOLD,
onePerDomain: false,
});
Assert.equal(links.length, 2, "adding a visit yields a link");
@@ -825,55 +855,67 @@ add_task(async function getTopFrecentSites_dedupeWWW() {
let provider = NewTabUtils.activityStreamLinks;
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.equal(links.length, 0, "empty history yields empty links");
// add a visit without www
- let testURI = "http://mozilla.com";
+ let testURI = "http://mozilla.com/";
await PlacesTestUtils.addVisits(testURI);
+ let frecency1 = await getFrecencyForUrl(testURI);
// add a visit with www
- testURI = "http://www.mozilla.com";
+ testURI = "http://www.mozilla.com/";
await PlacesTestUtils.addVisits(testURI);
+ let frecency2 = await getFrecencyForUrl(testURI);
// Test combined frecency score
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
- Assert.equal(links[0].frecency, 200, "frecency scores are combined");
+ Assert.equal(
+ links[0].frecency,
+ frecency1 + frecency2,
+ "frecency scores are combined"
+ );
// add another page visit with www and without www
let noWWW = "http://mozilla.com/page";
await PlacesTestUtils.addVisits(noWWW);
+ let noWWWFrecency = await getFrecencyForUrl(noWWW);
let withWWW = "http://www.mozilla.com/page";
await PlacesTestUtils.addVisits(withWWW);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let withWWWFrecency = await getFrecencyForUrl(withWWW);
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
Assert.equal(
links[0].frecency,
- 200,
+ noWWWFrecency + withWWWFrecency,
"frecency scores are combined ignoring extra pages"
);
// add another visit with www
await PlacesTestUtils.addVisits(withWWW);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ withWWWFrecency = await getFrecencyForUrl(withWWW);
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "still yields one link");
Assert.equal(links[0].url, withWWW, "more frecent www link is used");
Assert.equal(
links[0].frecency,
- 300,
+ noWWWFrecency + withWWWFrecency,
"frecency scores are combined ignoring extra pages"
);
// add a couple more visits to the no-www page
await PlacesTestUtils.addVisits(noWWW);
await PlacesTestUtils.addVisits(noWWW);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ noWWWFrecency = await getFrecencyForUrl(noWWW);
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "still yields one link");
Assert.equal(links[0].url, noWWW, "now more frecent no-www link is used");
Assert.equal(
links[0].frecency,
- 500,
+ noWWWFrecency + withWWWFrecency,
"frecency scores are combined ignoring extra pages"
);
});
@@ -890,7 +932,9 @@ add_task(async function getTopFrencentSites_maxLimit() {
await PlacesTestUtils.addVisits(testURI);
}
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.less(
links.length,
MANY_LINKS,
@@ -908,21 +952,23 @@ add_task(async function getTopFrencentSites_allowedProtocols() {
let testURI = "file:///some/file/path.png";
await PlacesTestUtils.addVisits(testURI);
- let links = await provider.getTopSites({ topsiteFrecency: 100 });
+ let links = await provider.getTopSites({
+ topsiteFrecency: FRECENCY_THRESHOLD,
+ });
Assert.equal(links.length, 0, "don't get sites with the file:// protocol");
// now add a site with an allowed protocol
testURI = "http://www.mozilla.com";
await PlacesTestUtils.addVisits(testURI);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(links.length, 1, "http:// is an allowed protocol");
// and just to be sure, add a visit to a site with ftp:// protocol
testURI = "ftp://bad/example";
await PlacesTestUtils.addVisits(testURI);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(
links.length,
1,
@@ -933,7 +979,7 @@ add_task(async function getTopFrencentSites_allowedProtocols() {
testURI = "https://https";
await PlacesTestUtils.addVisits(testURI);
- links = await provider.getTopSites({ topsiteFrecency: 100 });
+ links = await provider.getTopSites({ topsiteFrecency: FRECENCY_THRESHOLD });
Assert.equal(
links.length,
2,
@@ -1115,7 +1161,7 @@ add_task(async function getTopFrecentSites_hideWithSearchParam() {
JSON.stringify(hideWithSearchParam)
);
- let options = { topsiteFrecency: 100 };
+ let options = { topsiteFrecency: FRECENCY_THRESHOLD };
if (hideWithSearchParam !== undefined) {
options = { ...options, hideWithSearchParam };
}
