Bug 1469651 - For autofill, collapse all prefixes per host. r?mak
MozReview-Commit-ID: Dy7IoOV8n8f
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -283,36 +283,43 @@ const SQL_AUTOFILL_WITH = `
const SQL_AUTOFILL_FRECENCY_THRESHOLD = `(
SELECT value FROM autofill_frecency_threshold
)`;
function originQuery(conditions = "", bookmarkedFragment = "NULL") {
return `${SQL_AUTOFILL_WITH}
SELECT :query_type,
- host || '/',
- prefix || host || '/',
+ fixed_up_host || '/',
+ prefix || moz_origins.host || '/',
frecency,
- ${bookmarkedFragment} AS bookmarked,
+ bookmarked,
id
- FROM moz_origins
- WHERE host BETWEEN :searchString AND :searchString || X'FFFF'
- AND frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
- ${conditions}
- UNION ALL
- SELECT :query_type,
- fixup_url(host) || '/',
- prefix || host || '/',
- frecency,
- ${bookmarkedFragment} AS bookmarked,
- id
- FROM moz_origins
- WHERE host BETWEEN 'www.' || :searchString AND 'www.' || :searchString || X'FFFF'
- AND frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
- ${conditions}
+ FROM (
+ SELECT host AS host,
+ host AS fixed_up_host,
+ TOTAL(frecency) AS host_frecency,
+ ${bookmarkedFragment} AS bookmarked
+ FROM moz_origins
+ WHERE host BETWEEN :searchString AND :searchString || X'FFFF'
+ ${conditions}
+ GROUP BY host
+ HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+ UNION ALL
+ SELECT host AS host,
+ fixup_url(host) AS fixed_up_host,
+ TOTAL(frecency) AS host_frecency,
+ ${bookmarkedFragment} AS bookmarked
+ FROM moz_origins
+ WHERE host BETWEEN 'www.' || :searchString AND 'www.' || :searchString || X'FFFF'
+ ${conditions}
+ GROUP BY host
+ HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+ ) AS grouped_hosts
+ JOIN moz_origins ON moz_origins.host = grouped_hosts.host
ORDER BY frecency DESC, id DESC
LIMIT 1 `;
}
const SQL_ORIGIN_QUERY = originQuery();
const SQL_ORIGIN_PREFIX_QUERY = originQuery(
`AND prefix BETWEEN :prefix AND :prefix || X'FFFF'`
--- a/toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
@@ -128,8 +128,80 @@ add_task(async function multidotted() {
matches: [{
value: "www.example.co.jp:8888/",
comment: "www.example.co.jp:8888",
style: ["autofill", "heuristic"],
}],
});
await cleanup();
});
+
+// When determining which origins should be autofilled, all the origins sharing
+// a host should be added together to get their combined frecency -- i.e.,
+// prefixes should be collapsed. And then from that list, the origin with the
+// highest frecency should be chosen.
+add_task(async function groupByHost() {
+ // Add some visits to the same host, example.com. Add one http and two https
+ // so that https has a higher frecency and is therefore the origin that should
+ // be autofilled. Also add another origin that has a higher frecency than
+ // 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: "https://example.com/" },
+ { uri: "https://example.com/" },
+
+ { uri: "https://mozilla.org/" },
+ { uri: "https://mozilla.org/" },
+ { uri: "https://mozilla.org/" },
+ ]);
+
+ let httpFrec = frecencyForUrl("http://example.com/");
+ let httpsFrec = frecencyForUrl("https://example.com/");
+ let otherFrec = frecencyForUrl("https://mozilla.org/");
+ Assert.ok(httpFrec < httpsFrec, "Sanity check");
+ Assert.ok(httpsFrec < otherFrec, "Sanity check");
+
+ // Compute the autofill threshold.
+ let db = await PlacesUtils.promiseDBConnection();
+ let rows = await db.execute(`
+ SELECT
+ IFNULL((SELECT value FROM moz_meta WHERE key = "origin_frecency_count"), 0),
+ IFNULL((SELECT value FROM moz_meta WHERE key = "origin_frecency_sum"), 0),
+ IFNULL((SELECT value FROM moz_meta WHERE key = "origin_frecency_sum_of_squares"), 0)
+ `);
+ let count = rows[0].getResultByIndex(0);
+ let sum = rows[0].getResultByIndex(1);
+ let squares = rows[0].getResultByIndex(2);
+ let threshold =
+ (sum / count) + Math.sqrt((squares - ((sum * sum) / count)) / count);
+
+ // Make sure the frecencies of the three origins are as expected in relation
+ // to the threshold.
+ Assert.ok(httpFrec < threshold, "http origin should be < threshold");
+ Assert.ok(httpsFrec < threshold, "https origin should be < threshold");
+ Assert.ok(threshold <= otherFrec, "Other origin should cross threshold");
+
+ Assert.ok(threshold <= httpFrec + httpsFrec,
+ "http and https origin added together should cross threshold");
+
+ // The https origin should be autofilled.
+ await check_autocomplete({
+ search: "ex",
+ autofilled: "example.com/",
+ completed: "https://example.com/",
+ matches: [
+ {
+ value: "example.com/",
+ comment: "https://example.com",
+ style: ["autofill", "heuristic"],
+ },
+ {
+ value: "http://example.com/",
+ comment: "test visit for http://example.com/",
+ style: ["favicon"],
+ },
+ ],
+ });
+
+ await cleanup();
+});