Bug 1469651 - For autofill, collapse all prefixes per host. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Tue, 03 Jul 2018 14:28:35 -0700
changeset 425222 cb9a1ff4a249e72d3460cb998c9dd18bfe0c4f48
parent 425203 84315fc436a17cec66b42ac53b2c45dee0c717d2
child 425223 c5dd2fdeda523018c87dfb4645f17b733b7a9111
push id34241
push userebalazs@mozilla.com
push dateFri, 06 Jul 2018 09:45:16 +0000
treeherdermozilla-central@80d8f267abd8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1469651
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
+});