Bug 1469651 - For autofill, collapse all prefixes per host. r?mak draft
authorDrew Willcoxon <adw@mozilla.com>
Tue, 03 Jul 2018 14:28:35 -0700
changeset 813784 9f685536b0205f556eae8dd10a9c3e35511f8992
parent 813240 a8dc3a8a9a0d0794d40f55283b66716eff70f5eb
push id115005
push userbmo:adw@mozilla.com
push dateTue, 03 Jul 2018 21:29:02 +0000
reviewersmak
bugs1469651
milestone63.0a1
Bug 1469651 - For autofill, collapse all prefixes per host. r?mak MozReview-Commit-ID: Dy7IoOV8n8f
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
--- 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();
+});