Bug 1488879 - Autofill non-hidden origins and URLs with frecencies <= 0. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Mon, 10 Sep 2018 15:33:42 +0000
changeset 435537 f4f0af49af15
parent 435536 07ce49ea7662
child 435538 8c55a3d07f0c
push id34614
push userapavel@mozilla.com
push dateMon, 10 Sep 2018 21:59:18 +0000
treeherdermozilla-central@fea371cafd2c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1488879
milestone64.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 1488879 - Autofill non-hidden origins and URLs with frecencies <= 0. r=mak (1) Modify the threshold calculation so the minimum threshold is 0, not 1. That means that no moz_origins will be excluded from consideration because their frecencies are 0. moz_origins always have frecencies >= 0. (2) Modify the urlQuery to include moz_places with -1 frecencies but that aren't hidden. Differential Revision: https://phabricator.services.mozilla.com/D5308
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/autofill_tasks.js
toolkit/components/places/tests/unifiedcomplete/test_zero_frecency.js
toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -258,34 +258,33 @@ const SQL_ADAPTIVE_QUERY =
 // Result row indexes for originQuery()
 const QUERYINDEX_ORIGIN_AUTOFILLED_VALUE = 1;
 const QUERYINDEX_ORIGIN_URL = 2;
 const QUERYINDEX_ORIGIN_FRECENCY = 3;
 
 // `WITH` clause for the autofill queries.  autofill_frecency_threshold.value is
 // the mean of all moz_origins.frecency values + stddevMultiplier * one standard
 // deviation.  This is inlined directly in the SQL (as opposed to being a custom
-// Sqlite function for example) in order to be as efficient as possible.  The
-// MAX() is to make sure that places with <= 0 frecency are never autofilled.
+// Sqlite function for example) in order to be as efficient as possible.
 const SQL_AUTOFILL_WITH = `
   WITH
   frecency_stats(count, sum, squares) AS (
     SELECT
       CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_count") AS REAL),
       CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_sum") AS REAL),
       CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_sum_of_squares") AS REAL)
   ),
   autofill_frecency_threshold(value) AS (
-    SELECT MAX(1,
+    SELECT
       CASE count
       WHEN 0 THEN 0.0
       WHEN 1 THEN sum
       ELSE (sum / count) + (:stddevMultiplier * sqrt((squares - ((sum * sum) / count)) / count))
       END
-    ) FROM frecency_stats
+    FROM frecency_stats
   )
 `;
 
 const SQL_AUTOFILL_FRECENCY_THRESHOLD = `(
   SELECT value FROM autofill_frecency_threshold
 )`;
 
 function originQuery(conditions = "", bookmarkedFragment = "NULL") {
@@ -352,28 +351,30 @@ function urlQuery(conditions1, condition
           SELECT :query_type,
                  url,
                  :strippedURL,
                  frecency,
                  foreign_count > 0 AS bookmarked,
                  id
           FROM moz_places
           WHERE rev_host = :revHost
-                AND frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+                AND MAX(frecency, 0) >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+                AND hidden = 0
                 ${conditions1}
           UNION ALL
           SELECT :query_type,
                  url,
                  :strippedURL,
                  frecency,
                  foreign_count > 0 AS bookmarked,
                  id
           FROM moz_places
           WHERE rev_host = :revHost || 'www.'
-                AND frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+                AND MAX(frecency, 0) >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
+                AND hidden = 0
                 ${conditions2}
           ORDER BY frecency DESC, id DESC
           LIMIT 1 `;
 }
 
 const SQL_URL_QUERY = urlQuery(
   `AND strip_prefix_and_userinfo(url) BETWEEN :strippedURL AND :strippedURL || X'FFFF'`,
   `AND strip_prefix_and_userinfo(url) BETWEEN 'www.' || :strippedURL AND 'www.' || :strippedURL || X'FFFF'`
--- a/toolkit/components/places/tests/unifiedcomplete/autofill_tasks.js
+++ b/toolkit/components/places/tests/unifiedcomplete/autofill_tasks.js
@@ -666,9 +666,67 @@ function addAutofillTasks(origins) {
           comment,
           style: ["autofill", "heuristic"],
         },
       ],
     });
 
     await cleanup();
   });
+
+  // Bookmark a page and then clear history.  The bookmarked origin/URL should
+  // be autofilled even though its frecency is <= 0 since the autofill threshold
+  // is 0.
+  add_task(async function zeroThreshold() {
+    await addBookmark({
+      uri: "http://" + url,
+    });
+
+    await PlacesUtils.history.clear();
+
+    // Make sure the place's frecency is <= 0.  (It will be reset to -1 on the
+    // history.clear() above, and then on idle it will be reset to 0.  xpcshell
+    // tests disable the idle service, so in practice it should always be -1,
+    // but in order to avoid possible intermittent failures in the future, don't
+    // assume that.)
+    let placeFrecency =
+      await PlacesTestUtils.fieldInDB("http://" + url, "frecency");
+    Assert.ok(placeFrecency <= 0);
+
+    // Make sure the origin's frecency is 0.
+    let db = await PlacesUtils.promiseDBConnection();
+    let rows = await db.execute(`
+      SELECT frecency FROM moz_origins WHERE host = '${host}';
+    `);
+    Assert.equal(rows.length, 1);
+    let originFrecency = rows[0].getResultByIndex(0);
+    Assert.equal(originFrecency, 0);
+
+    // Make sure the autofill threshold is 0.
+    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);
+    Assert.equal(count, 0);
+    Assert.equal(sum, 0);
+    Assert.equal(squares, 0);
+
+    await check_autocomplete({
+      search,
+      autofilled: url,
+      completed: "http://" + url,
+      matches: [
+        {
+          value: url,
+          comment,
+          style: ["autofill", "heuristic"],
+        },
+      ],
+    });
+
+    await cleanup();
+  });
 }
deleted file mode 100644
--- a/toolkit/components/places/tests/unifiedcomplete/test_zero_frecency.js
+++ /dev/null
@@ -1,33 +0,0 @@
-/* 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/. */
-
-// Ensure inline autocomplete doesn't return zero frecency pages.
-
-add_task(async function test_zero_frec_domain() {
-  info("Searching for zero frecency domain should not autoFill it");
-  await PlacesTestUtils.addVisits({
-    uri: NetUtil.newURI("http://mozilla.org/framed_link/"),
-    transition: TRANSITION_FRAMED_LINK,
-  });
-  await check_autocomplete({
-    search: "moz",
-    autofilled: "moz",
-    completed:  "moz",
-  });
-  await cleanup();
-});
-
-add_task(async function test_zero_frec_url() {
-  info("Searching for zero frecency url should not autoFill it");
-  await PlacesTestUtils.addVisits({
-    uri: NetUtil.newURI("http://mozilla.org/framed_link/"),
-    transition: TRANSITION_FRAMED_LINK,
-  });
-  await check_autocomplete({
-    search: "mozilla.org/f",
-    autofilled: "mozilla.org/f",
-    completed:  "mozilla.org/f",
-  });
-  await cleanup();
-});
--- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
+++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
@@ -51,9 +51,8 @@ skip-if = !sync
 [test_search_engine_restyle.js]
 [test_search_suggestions.js]
 [test_special_search.js]
 [test_swap_protocol.js]
 [test_tab_matches.js]
 [test_trimming.js]
 [test_visit_url.js]
 [test_word_boundary_search.js]
-[test_zero_frecency.js]