commit b0713995861498c3e1ebdcad663b005042dd003c parent f740b05da24b2994f0e915889ec7e43ec0dfcccf Author: James Teow <jteow@mozilla.com> Date: Thu, 27 Nov 2025 18:11:16 +0000 Bug 2001284 - Improve origin frecency calculation with daily averages and percentile-based threshold - r=mak,places-reviewers,urlbar-reviewers Differential Revision: https://phabricator.services.mozilla.com/D274095 Diffstat:
15 files changed, 359 insertions(+), 124 deletions(-)
diff --git a/browser/components/urlbar/tests/unit/head.js b/browser/components/urlbar/tests/unit/head.js @@ -1229,3 +1229,9 @@ async function checkOriginsOrder(host, prefixOrder) { Assert.deepEqual(prefixes, prefixOrder); }); } + +function daysAgo(days) { + let date = new Date(); + date.setDate(date.getDate() - days); + return date; +} diff --git a/browser/components/urlbar/tests/unit/test_autofill_adaptiveHistory.js b/browser/components/urlbar/tests/unit/test_autofill_adaptiveHistory.js @@ -1054,7 +1054,6 @@ const TEST_DATA = [ ], userInput: "example.com/test", expected: { - autofilled: "example.com/test", completed: "http://example.com/test", results: [ context => @@ -1080,13 +1079,10 @@ const TEST_DATA = [ frecency: 0, userInput: "exa", expected: { - autofilled: "example.com/", - completed: "http://example.com/", results: [ context => - makeVisitResult(context, { - uri: "http://example.com/", - fallbackTitle: UrlbarTestUtils.trimURL("http://example.com/"), + makeSearchResult(context, { + engineName: "Suggestions", heuristic: true, }), context => @@ -1123,7 +1119,7 @@ const TEST_DATA = [ visitHistory: ["http://example.com/test"], inputHistory: [{ uri: "http://example.com/test", input: "exa" }], bookmarks: [{ uri: "http://example.com/test", title: "test bookmark" }], - frecency: 0, + frecency: 5, userInput: "exa", expected: { autofilled: "example.com/", @@ -1135,6 +1131,11 @@ const TEST_DATA = [ fallbackTitle: UrlbarTestUtils.trimURL("http://example.com/"), heuristic: true, }), + context => + makeVisitResult(context, { + uri: "http://example.com/test", + title: "test bookmark", + }), ], }, }, @@ -1145,7 +1146,7 @@ const TEST_DATA = [ source: UrlbarUtils.RESULT_SOURCE.HISTORY, visitHistory: ["http://example.com/test"], inputHistory: [{ uri: "http://example.com/test", input: "exa" }], - frecency: 0, + frecency: 5, userInput: "exa", expected: { autofilled: "example.com/", diff --git a/browser/components/urlbar/tests/unit/test_autofill_bookmarked.js b/browser/components/urlbar/tests/unit/test_autofill_bookmarked.js @@ -24,18 +24,26 @@ add_task(async function () { // Add one visit to http to give it a realistic origin frecency score instead // of the default value of 1. This ensures the threshold test doesn't use an // artificially low baseline. - await PlacesTestUtils.addVisits(`http://${host}`); + await PlacesTestUtils.addVisits({ + uri: `http://${host}`, + visitDate: daysAgo(90), + }); - for (let i = 0; i < 3; i++) { - await PlacesTestUtils.addVisits(`https://${host}`); - } - // ensure both fall below the threshold. - for (let i = 0; i < 15; i++) { - await PlacesTestUtils.addVisits({ - url: `https://not-${host}`, - transition: PlacesUtils.history.TRANSITION_TYPED, - }); - } + await PlacesTestUtils.addVisits({ + uri: `https://${host}`, + visitDate: daysAgo(30), + }); + + await PlacesTestUtils.addVisits({ + uri: `https://fakedomain1.com/`, + }); + await PlacesTestUtils.addVisits({ + uri: `https://fakedomain2.com/`, + }); + await PlacesTestUtils.addVisits({ + url: `https://not-${host}/`, + transition: PlacesUtils.history.TRANSITION_TYPED, + }); async function check_autofill() { await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies(); @@ -57,6 +65,12 @@ add_task(async function () { httpsOriginFrecency, "Http origin frecency should be below the https origin frecency" ); + let not = await getOriginFrecency("https://", "not-example.com"); + Assert.less( + httpsOriginFrecency, + not, + "Http origin frecency should be below not example.com" + ); // The http version should be filled because it's bookmarked, but with the // https prefix that is more frecent. @@ -107,8 +121,6 @@ add_task(async function () { url: `http://${host}`, parentGuid: PlacesUtils.bookmarks.unfiledGuid, }); - // Add a visit to prevent origin frecency from being too low. - await PlacesTestUtils.addVisits(`http://${host}`); await checkOriginsOrder(host, ["https://", "http://"]); diff --git a/browser/components/urlbar/tests/unit/test_autofill_origins.js b/browser/components/urlbar/tests/unit/test_autofill_origins.js @@ -331,46 +331,41 @@ add_task(async function groupByHost() { // both so that alone, neither http nor https would be autofilled, but added // together they should be. await PlacesTestUtils.addVisits([ - { uri: "http://example.com/" }, + { uri: "http://example.com/", visitDate: daysAgo(30) }, - { uri: "https://example.com/" }, - { uri: "https://example.com/" }, + // Have a higher frecency by being more recent. But not so recent that it + // has a higher frecency than other visits that bump the origins threshold. + { uri: "https://example.com/", visitDate: daysAgo(7) }, { uri: "https://mozilla.org/", transition: PlacesUtils.history.TRANSITION_TYPED, }, { - uri: "https://mozilla.org/", - transition: PlacesUtils.history.TRANSITION_TYPED, - }, - { - uri: "https://mozilla.org/", - transition: PlacesUtils.history.TRANSITION_TYPED, - }, - { - uri: "https://mozilla.org/", + uri: "https://mozilla.org/1", transition: PlacesUtils.history.TRANSITION_TYPED, + visitDate: daysAgo(1), }, + + // Add more origins to make the threshold higher + { uri: "https://mozilla.com/" }, + { uri: "https://mozilla.ca/" }, ]); - let httpFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { url: "http://example.com/" } - ); - let httpsFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { url: "https://example.com/" } + let httpFrec = await getOriginFrecency("http://", "example.com"); + let httpsFrec = await getOriginFrecency("https://", "example.com"); + let otherFrec = await getOriginFrecency("https://", "mozilla.org"); + + Assert.less( + httpFrec, + httpsFrec, + "Frecency http://example.com is less than https://example.com" ); - let otherFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { url: "https://mozilla.org/" } + Assert.less( + httpsFrec, + otherFrec, + "Frecency of https://example.com is less than https://mozilla.org" ); - Assert.less(httpFrec, httpsFrec, "Sanity check"); - Assert.less(httpsFrec, otherFrec, "Sanity check"); // Make sure the frecencies of the three origins are as expected in relation // to the threshold. @@ -418,45 +413,22 @@ add_task(async function groupByHostNonDefaultStddevMultiplier() { ); await PlacesTestUtils.addVisits([ - { uri: "http://example.com/" }, - { uri: "http://example.com/" }, + { uri: "http://example.com/", visitDate: daysAgo(30) }, - { uri: "https://example.com/" }, - { uri: "https://example.com/" }, - { uri: "https://example.com/" }, + { uri: "https://example.com/", visitDate: daysAgo(7) }, - { uri: "https://foo.com/" }, - { uri: "https://foo.com/" }, - { uri: "https://foo.com/" }, - - { uri: "https://mozilla.org/" }, - { uri: "https://mozilla.org/" }, - { uri: "https://mozilla.org/" }, - { uri: "https://mozilla.org/" }, { uri: "https://mozilla.org/" }, + { uri: "https://mozilla.org/1", visitDate: daysAgo(1) }, + { uri: "https://mozilla.org/2", visitDate: daysAgo(2) }, + + // Add more origins to make the threshold higher + { uri: "https://mozilla.com/" }, + { uri: "https://mozilla.ca/" }, ]); - let httpFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { - url: "http://example.com/", - } - ); - let httpsFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { - url: "https://example.com/", - } - ); - let otherFrec = await PlacesTestUtils.getDatabaseValue( - "moz_places", - "frecency", - { - url: "https://mozilla.org/", - } - ); + let httpFrec = await getOriginFrecency("http://", "example.com"); + let httpsFrec = await getOriginFrecency("https://", "example.com"); + let otherFrec = await getOriginFrecency("https://", "mozilla.org"); Assert.less(httpFrec, httpsFrec, "Sanity check"); Assert.less(httpsFrec, otherFrec, "Sanity check"); diff --git a/browser/components/urlbar/tests/unit/test_autofill_originsAndQueries.js b/browser/components/urlbar/tests/unit/test_autofill_originsAndQueries.js @@ -581,6 +581,7 @@ add_autofill_task(async function frecency() { await PlacesTestUtils.addVisits([ { uri: "http://" + url, + visitDate: daysAgo(30), }, ]); let context = createContext(search, { isPrivate: false }); @@ -598,9 +599,9 @@ add_autofill_task(async function frecency() { }); // Add two https visits. https should now be completed. - for (let i = 0; i < 2; i++) { - await PlacesTestUtils.addVisits([{ uri: "https://" + url }]); - } + await PlacesTestUtils.addVisits([ + { uri: "https://" + url, visitDate: daysAgo(29) }, + ]); context = createContext(search, { isPrivate: false }); await check_results({ context, @@ -617,9 +618,10 @@ add_autofill_task(async function frecency() { // Add two more http visits, three total. http should now be completed // again. - for (let i = 0; i < 2; i++) { - await PlacesTestUtils.addVisits([{ uri: "http://" + url }]); - } + await PlacesTestUtils.addVisits([ + { uri: "http://" + url, visitDate: daysAgo(28) }, + { uri: "http://" + url, visitDate: daysAgo(27) }, + ]); context = createContext(search, { isPrivate: false }); await check_results({ context, @@ -641,7 +643,9 @@ add_autofill_task(async function frecency() { // Add four www https visits. www https should now be completed. for (let i = 0; i < 4; i++) { - await PlacesTestUtils.addVisits([{ uri: "https://www." + url }]); + await PlacesTestUtils.addVisits([ + { uri: "https://www." + url, visitDate: daysAgo(i) }, + ]); } context = createContext(search, { isPrivate: false }); await check_results({ @@ -726,9 +730,16 @@ add_autofill_task(async function frecency() { ], }); - // Now add 10 more visits to the different host so that the frecency of + // Now add more visits to the different host so that the frecency of // https://example.com/ falls below the autofill threshold. It should not // be autofilled now. + await PlacesTestUtils.addVisits([ + { uri: "https://other-site.com/1" }, + { uri: "https://other-site.com/2" }, + { uri: "https://other-site.com/3" }, + { uri: "https://other-site.com/4" }, + ]); + for (let i = 0; i < 10; i++) { await PlacesTestUtils.addVisits([{ uri: "https://not-" + url }]); } @@ -980,7 +991,7 @@ add_autofill_task(async function zeroThreshold() { let originFrecency = await getOriginFrecency("http://", host); Assert.equal(originFrecency, 1, "Check expected origin's frecency"); let threshold = await getOriginAutofillThreshold(); - Assert.equal(threshold, 1, "Check expected origins threshold"); + Assert.equal(threshold, 2, "Check expected origins threshold"); let context = createContext(search, { isPrivate: false }); await check_results({ @@ -2085,10 +2096,15 @@ add_autofill_task(async function suggestBookmarkFalse_visitedBookmarkBelow() { return; } // First, make sure that `url` is below the autofill threshold. - await PlacesTestUtils.addVisits("http://" + url); - for (let i = 0; i < 3; i++) { - await PlacesTestUtils.addVisits("http://some-other-" + url); - } + await PlacesTestUtils.addVisits({ + uri: "http://" + url, + visitDate: daysAgo(30), + }); + await PlacesTestUtils.addVisits({ + uri: "http://some-other-" + url, + }); + await PlacesTestUtils.addVisits("http://other-website.com"); + let context = createContext(search, { isPrivate: false }); await check_results({ context, @@ -2159,10 +2175,13 @@ add_autofill_task( return; } // First, make sure that `url` is below the autofill threshold. - await PlacesTestUtils.addVisits("http://" + url); - for (let i = 0; i < 3; i++) { - await PlacesTestUtils.addVisits("http://some-other-" + url); - } + await PlacesTestUtils.addVisits({ + uri: "http://" + url, + visitDate: daysAgo(30), + }); + await PlacesTestUtils.addVisits("http://some-other-" + url); + await PlacesTestUtils.addVisits("http://other-website.com"); + let context = createContext("http://" + search, { isPrivate: false }); await check_results({ context, diff --git a/browser/components/urlbar/tests/unit/xpcshell.toml b/browser/components/urlbar/tests/unit/xpcshell.toml @@ -39,6 +39,7 @@ support-files = ["data/engine.xml"] ["test_about_urls.js"] ["test_autofill_adaptiveHistory.js"] +requesttimeoutfactor = 3 # Slow on Mac debug ["test_autofill_bookmarked.js"] @@ -47,9 +48,10 @@ support-files = ["data/engine.xml"] ["test_autofill_functional.js"] ["test_autofill_origins.js"] +requesttimeoutfactor = 3 # Slow on Mac debug ["test_autofill_originsAndQueries.js"] -requesttimeoutfactor = 2 # Slow on Linux debug +requesttimeoutfactor = 3 # Slow on Linux debug and Mac debug ["test_autofill_origins_alt_frecency.js"] prefs = [ diff --git a/toolkit/components/places/Database.cpp b/toolkit/components/places/Database.cpp @@ -1353,7 +1353,12 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) { NS_ENSURE_SUCCESS(rv, rv); } - // Firefox 145 uses schema version 83 + if (currentSchemaVersion < 84) { + rv = MigrateV84Up(); + NS_ENSURE_SUCCESS(rv, rv); + } + + // Firefox 147 uses schema version 84 // Schema Upgrades must add migration code here. // >>> IMPORTANT! <<< @@ -2280,6 +2285,17 @@ nsresult Database::MigrateV83Up() { return NS_OK; } +nsresult Database::MigrateV84Up() { + printf("Upgrading database."); + // Recalculate frecency due to changing calculate_frecency. + nsresult rv = mMainConn->ExecuteSimpleSQL( + "UPDATE moz_origins " + "SET recalc_frecency = 1 " + "WHERE frecency > 1"_ns); + NS_ENSURE_SUCCESS(rv, rv); + return NS_OK; +} + int64_t Database::CreateMobileRoot() { MOZ_ASSERT(NS_IsMainThread()); diff --git a/toolkit/components/places/Database.h b/toolkit/components/places/Database.h @@ -332,6 +332,7 @@ class Database final : public nsIObserver, public nsSupportsWeakReference { nsresult MigrateV81Up(); nsresult MigrateV82Up(); nsresult MigrateV83Up(); + nsresult MigrateV84Up(); nsresult UpdateBookmarkRootTitles(); diff --git a/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs b/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs @@ -86,6 +86,8 @@ const DEFAULT_CHUNK_SIZE = 50; // Threshold used to evaluate whether the number of Places events from the last // recalculation is high enough to deserve a recalculation rate increase. const ACCELERATION_EVENTS_THRESHOLD = 250; +// The number of buckets to split moz_origins results +const BUCKETS = 2; /** * Recalculates and decays frecency scores in Places. @@ -323,12 +325,15 @@ export class PlacesFrecencyRecalculator { ` UPDATE moz_origins SET frecency = IFNULL(( - SELECT sum(frecency) - FROM moz_places h - WHERE origin_id = moz_origins.id - AND last_visit_date > - strftime('%s','now','localtime','start of day', - '-${lazy.originsFrecencyCutOffDays} day','utc') * 1000000 + SELECT SUM(f) FROM ( + SELECT CAST(AVG(h.frecency) AS INTEGER) AS f + FROM moz_places h + WHERE origin_id = moz_origins.id + AND last_visit_date > + strftime('%s','now','localtime','start of day', + '-${lazy.originsFrecencyCutOffDays} day','utc') * 1000000 + GROUP BY date(last_visit_date / 1000000, 'unixepoch') + ) ), 1.0), recalc_frecency = 0 WHERE id IN ( SELECT id FROM moz_origins @@ -347,8 +352,23 @@ export class PlacesFrecencyRecalculator { // emptied. // In case of NULL, the default threshold is 2, that is higher than the // default frecency set above. + // Bug 2002569: We should modify the query to use percentiles. let threshold = ( - await db.executeCached(`SELECT avg(frecency) FROM moz_origins`) + await db.executeCached( + ` + WITH ntiled AS ( + SELECT + host, + frecency, + NTILE(:buckets) OVER (ORDER BY frecency ASC) AS ntile + FROM moz_origins + WHERE frecency > 1) + SELECT MAX(frecency) + FROM ntiled + WHERE ntile = 1 + `, + { buckets: BUCKETS } + ) )[0].getResultByIndex(0); await lazy.PlacesUtils.metadata.set( "origin_frecency_threshold", diff --git a/toolkit/components/places/nsINavHistoryService.idl b/toolkit/components/places/nsINavHistoryService.idl @@ -910,7 +910,7 @@ interface nsINavHistoryService : nsISupports // The current database schema version. // To migrate to a new version bump this, add a MigrateVXXUp function to // Database.cpp/h, and a test into tests/migration/ - const unsigned long DATABASE_SCHEMA_VERSION = 83; + const unsigned long DATABASE_SCHEMA_VERSION = 84; /** * System Notifications: diff --git a/toolkit/components/places/tests/migration/places_v84.sqlite b/toolkit/components/places/tests/migration/places_v84.sqlite Binary files differ. diff --git a/toolkit/components/places/tests/migration/test_current_from_v83.js b/toolkit/components/places/tests/migration/test_current_from_v83.js @@ -0,0 +1,131 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function setup() { + let path = await setupPlacesDatabase("places_v83.sqlite"); + let db = await Sqlite.openConnection({ path }); + + const ONE_DAY = 24 * 60 * 60 * 1000; + const NOW = Date.now() * 1000; // Microseconds + const RECENT = NOW - 1 * ONE_DAY * 1000; + const OLD = NOW - 91 * ONE_DAY * 1000; // 91 days ago (outside 90 day window) + + // 1. Setup Moz Origins + // Note: We use distinct IDs to make verification easy. + await db.execute(` + INSERT INTO moz_origins (id, prefix, host, frecency, recalc_frecency) + VALUES + -- Frecency above 1, recent visit. Should recalc. + (100, 'https://', 'high-frecency-recent.com', 2, 0), + + -- Frecency above 1, old visit. Should recalc. + (101, 'https://', 'high-frecency-old.com', 2, 0), + + -- Frecency above 1, no visits. Should recalc. + (102, 'https://', 'high-frecency-empty.com', 2, 0), + + -- Frecency at 1, recent visit. Should skip. + (103, 'https://', 'low-frecency-recent.com', 1, 0), + + -- Frecency at 1, old visit. Should skip. + (104, 'https://', 'low-frecency-old.com', 1, 0), + + -- Frecency at 1, no visits. Should skip. + (105, 'https://', 'low-frecency-empty.com', 1, 0), + + -- Already marked. Should remain 1. + (106, 'https://', 'already-marked.com', -1, 1) + `); + + // 2. Setup Moz Places (Visits) + await db.execute(` + INSERT INTO moz_places + (url, url_hash, frecency, last_visit_date, origin_id, guid) + VALUES + ('https://high-frecency-recent.com/', 'hashA', 2000, ${RECENT}, 100, 'guid0000000A'), + ('https://high-frecency-old.com/', 'hashB', 2000, ${OLD}, 101, 'guid0000000B'), + ('https://high-frecency-empty.com/', 'hashC', 2000, null, 102, 'guid0000000C'), + ('https://low-frecency-recent.com/', 'hashD', 10, ${RECENT}, 103, 'guid0000000D'), + ('https://low-frecency-old.com/', 'hashE', 10, ${OLD}, 104, 'guid0000000E'), + ('https://low-frecency-empty.com/', 'hashF', 10, null, 105, 'guid0000000F'), + ('https://already-marked.com/', 'hashG', 10, null, 106, 'guid0000000G') + `); + + await db.close(); +}); + +add_task(async function database_is_valid() { + // Trigger migration + Assert.equal( + PlacesUtils.history.databaseStatus, + PlacesUtils.history.DATABASE_STATUS_UPGRADED + ); + + const db = await PlacesUtils.promiseDBConnection(); + Assert.equal(await db.getSchemaVersion(), CURRENT_SCHEMA_VERSION); + + // Check results + let rows = await db.execute(` + SELECT id, host, recalc_frecency + FROM moz_origins + WHERE id >= 100 + ORDER BY id ASC + `); + + // Map results by ID for easy lookup + let results = new Map(); + for (let row of rows) { + results.set( + row.getResultByName("id"), + row.getResultByName("recalc_frecency") + ); + } + + const expectedResults = [ + { + id: 100, + shouldRecalc: 1, + desc: "High frecency should be marked for recalc to decay.", + }, + { + id: 101, + shouldRecalc: 1, + desc: "High frecency with old visit (<30 days) should be marked for recalc.", + }, + { + id: 102, + shouldRecalc: 1, + desc: "High frecency with no visits should be marked for recalc.", + }, + { + id: 103, + shouldRecalc: 0, + desc: "Low frecency with recent visit should not be marked for recalc.", + }, + { + id: 104, + shouldRecalc: 0, + desc: "Low frecency with old visit should not be marked for recalc.", + }, + { + id: 105, + shouldRecalc: 0, + desc: "Low frecency with no visits should not be marked for recalc.", + }, + { + id: 106, + shouldRecalc: 1, + desc: "Already marked rows should remain marked.", + }, + ]; + + for (let test of expectedResults) { + Assert.equal( + results.get(test.id), + test.shouldRecalc, + `Origin ${test.id}: ${test.desc}` + ); + } +}); diff --git a/toolkit/components/places/tests/migration/xpcshell.toml b/toolkit/components/places/tests/migration/xpcshell.toml @@ -25,6 +25,7 @@ support-files = [ "places_v81.sqlite", "places_v82.sqlite", "places_v83.sqlite", + "places_v84.sqlite", ] ["test_current_from_downgraded.js"] @@ -56,3 +57,5 @@ support-files = [ ["test_current_from_v81.js"] ["test_current_from_v82.js"] + +["test_current_from_v83.js"] diff --git a/toolkit/components/places/tests/unit/test_origins.js b/toolkit/components/places/tests/unit/test_origins.js @@ -1033,8 +1033,8 @@ add_task(async function test_cutoff() { }); /** - * Returns the expected frecency of the origin of the given URLs, i.e., the sum - * of their frecencies. Each URL is expected to have the same origin. + * Returns the expected frecency of the origin of the given URLs. + * Each URL is expected to have the same origin. * * @param {(string|nsIURL|URL)[]} urls * An array of URL strings. @@ -1042,18 +1042,56 @@ add_task(async function test_cutoff() { * The expected origin frecency. */ async function expectedOriginFrecency(urls) { - let value = 0; + if (!urls.length) { + return 1.0; + } + + // Calculate cutoff: start of today minus N days, in microseconds. + let cutOffDays = Services.prefs.getIntPref( + "places.frecency.originsFrecencyCutOffDays", + 90 + ); + let cutOff = new Date(); + cutOff.setHours(0, 0, 0, 0); + cutOff.setDate(cutOff.getDate() - cutOffDays); + let cutOffMicroseconds = cutOff.getTime() * 1000; + + // Fetch frecency and last_visit_date for each URL, group by day + let dailyFrecencies = new Map(); for (let url of urls) { - let v = Math.max( - (await PlacesTestUtils.getDatabaseValue("moz_places", "frecency", { - url, - last_visit_date: [">", 0], - })) ?? 0, - 0 + let frecency = await PlacesTestUtils.getDatabaseValue( + "moz_places", + "frecency", + { url, last_visit_date: [">", cutOffMicroseconds] } + ); + if (frecency === undefined) { + continue; + } + + let lastVisitDate = await PlacesTestUtils.getDatabaseValue( + "moz_places", + "last_visit_date", + { url, last_visit_date: [">", cutOffMicroseconds] } ); - value += v; + + // Store results by date YYYY-MM-DD which mimics SQLs: + // GROUP BY date(last_visit_date / 1000000, 'unixepoch') + let day = new Date(lastVisitDate / 1000).toISOString().slice(0, 10); + if (!dailyFrecencies.has(day)) { + dailyFrecencies.set(day, []); + } + dailyFrecencies.get(day).push(frecency); } - return value || 1.0; + + // Sum of truncated daily averages which mimics SQL: + // SUM(CAST(AVG(frecency) AS INTEGER)) + let total = 0; + for (const frecencies of dailyFrecencies.values()) { + let sum = frecencies.reduce((a, b) => a + b, 0); + total += Math.trunc(sum / frecencies.length); + } + + return total || 1.0; } /** @@ -1114,12 +1152,25 @@ async function checkThreshold(expectedOriginFrecencies) { DEFAULT_THRESHOLD ); + // The origin frecency threshold is calculated by first filtering results + // above 1. Then we look for the midpoint value of the sorted set. + // If none is found, we default to the default threshold. + // + // This approximates median() which we'll likely use in the future, but + // differs in two ways: + // - We filter values starting with 1 because it likely means we deliberately + // gave it a very low frecency. + // - With even-length arrays we use the lower of the two middle values + // instead of their average. + let filteredResults = expectedOriginFrecencies.filter(value => value > 1); + filteredResults.sort((a, b) => a - b); + let middle = Math.ceil(filteredResults.length / 2); + let maxOfLowerGroup = filteredResults.at(middle - 1); + Assert.equal( threshold, - expectedOriginFrecencies.length - ? expectedOriginFrecencies.reduce((a, b) => a + b, 0) / - expectedOriginFrecencies.length - : DEFAULT_THRESHOLD + maxOfLowerGroup ?? DEFAULT_THRESHOLD, + "Threshold is equal." ); } diff --git a/toolkit/components/places/tests/unit/xpcshell.toml b/toolkit/components/places/tests/unit/xpcshell.toml @@ -201,6 +201,7 @@ prefs = ["places.frecency.pages.alternative.featureGate=true"] ["test_null_interfaces.js"] ["test_origins.js"] +requesttimeoutfactor = 3 # Slow on Mac debug ["test_origins_parsing.js"]