Bug 1488879 - Autofill non-hidden origins and URLs with frecencies <= 0. r=mak, a=jcristau
authorDrew Willcoxon <adw@mozilla.com>
Wed, 19 Sep 2018 14:58:25 +0200
changeset 481138 63be402638c8
parent 481137 169867270ec5
child 481139 e2b254faaebb
push id1786
push userjcristau@mozilla.com
push dateWed, 19 Sep 2018 13:14:52 +0000
treeherdermozilla-release@0a1434f27587 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, jcristau
bugs1488879
milestone62.0.2
Bug 1488879 - Autofill non-hidden origins and URLs with frecencies <= 0. r=mak, a=jcristau (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]