Bug 1823450 - Allow autofill use origins alternative frecency. r=daisuke

Differential Revision: https://phabricator.services.mozilla.com/D174963
2023-04-14 15:19:32 +00:00 · 2023-04-14 15:19:32 +00:00 · 5ced6a9b31
--- a/browser/components/urlbar/UrlbarProviderAutofill.sys.mjs
+++ b/browser/components/urlbar/UrlbarProviderAutofill.sys.mjs
@ -30,11 +30,30 @@ const QUERYTYPE = {
  AUTOFILL_ADAPTIVE: 3,
 };

+// Constants to support an alternative frecency algorithm.
+const ORIGIN_USE_ALT_FRECENCY = Services.prefs.getBoolPref(
+  "places.frecency.origins.alternative.featureGate",
+  false
+);
+const ORIGIN_FRECENCY_FIELD = ORIGIN_USE_ALT_FRECENCY
+  ? "alt_frecency"
+  : "frecency";
+
 // `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.
-const SQL_AUTOFILL_WITH = `
+const SQL_AUTOFILL_WITH = ORIGIN_USE_ALT_FRECENCY
+  ? `
+    WITH
+    autofill_frecency_threshold(value) AS (
+      SELECT IFNULL(
+        (SELECT value FROM moz_meta WHERE key = 'origin_alt_frecency_threshold'),
+        0.0
+      )
+    )
+    `
+  : `
    WITH
    frecency_stats(count, sum, squares) AS (
      SELECT
@ -82,12 +101,14 @@ function originQuery(where) {
      id,
      prefix,
      first_value(prefix) OVER (
-        PARTITION BY host ORDER BY frecency DESC, prefix = "https://" DESC, id DESC
+        PARTITION BY host ORDER BY ${ORIGIN_FRECENCY_FIELD} DESC, prefix = "https://" DESC, id DESC
      ),
      host,
      fixup_url(host),
-      TOTAL(frecency) OVER (PARTITION BY fixup_url(host)),
-      frecency,
+      IFNULL(${
+        ORIGIN_USE_ALT_FRECENCY ? "avg(alt_frecency)" : "total(frecency)"
+      } OVER (PARTITION BY fixup_url(host)), 0.0),
+      ${ORIGIN_FRECENCY_FIELD},
      MAX(EXISTS(
        SELECT 1 FROM moz_places WHERE origin_id = o.id AND foreign_count > 0
      )) OVER (PARTITION BY fixup_url(host)),
@ -448,7 +469,10 @@ class ProviderAutofill extends UrlbarProvider {
    let db = await lazy.PlacesUtils.promiseLargeCacheDBConnection();
    let conditions = [];
    // Pay attention to the order of params, since they are not named.
-    let params = [lazy.UrlbarPrefs.get("autoFill.stddevMultiplier"), ...hosts];
+    let params = [...hosts];
+    if (!ORIGIN_USE_ALT_FRECENCY) {
+      params.unshift(lazy.UrlbarPrefs.get("autoFill.stddevMultiplier"));
+    }
    let sources = queryContext.sources;
    if (
      sources.includes(UrlbarUtils.RESULT_SOURCE.HISTORY) &&
@ -469,12 +493,14 @@ class ProviderAutofill extends UrlbarProvider {
          id,
          prefix,
          first_value(prefix) OVER (
-            PARTITION BY host ORDER BY frecency DESC, prefix = "https://" DESC, id DESC
+            PARTITION BY host ORDER BY ${ORIGIN_FRECENCY_FIELD} DESC, prefix = "https://" DESC, id DESC
          ),
          host,
          fixup_url(host),
-          TOTAL(frecency) OVER (PARTITION BY fixup_url(host)),
-          frecency,
+          IFNULL(${
+            ORIGIN_USE_ALT_FRECENCY ? "avg(alt_frecency)" : "total(frecency)"
+          } OVER (PARTITION BY fixup_url(host)), 0.0),
+          ${ORIGIN_FRECENCY_FIELD},
          MAX(EXISTS(
            SELECT 1 FROM moz_places WHERE origin_id = o.id AND foreign_count > 0
          )) OVER (PARTITION BY fixup_url(host)),
@ -518,8 +544,10 @@ class ProviderAutofill extends UrlbarProvider {
    let opts = {
      query_type: QUERYTYPE.AUTOFILL_ORIGIN,
      searchString: searchStr.toLowerCase(),
-      stddevMultiplier: lazy.UrlbarPrefs.get("autoFill.stddevMultiplier"),
    };
+    if (!ORIGIN_USE_ALT_FRECENCY) {
+      opts.stddevMultiplier = lazy.UrlbarPrefs.get("autoFill.stddevMultiplier");
+    }
    if (this._strippedPrefix) {
      opts.prefix = this._strippedPrefix;
    }
--- a/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js
+++ b/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js
@ -0,0 +1,178 @@
+/* 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/. */
+
+// This is a basic autofill test to ensure enabling the alternative frecency
+// algorithm doesn't break autofill or tab-to-search. A more comprehensive
+// testing of the algorithm itself is not included since it's something that
+// may change frequently according to experimentation results.
+// Other existing autofill tests will, of course, need to be adapted once an
+// algorithm is promoted to be the default.
+
+XPCOMUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
+  return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
+    Ci.nsIObserver
+  ).wrappedJSObject;
+});
+
+testEngine_setup();
+
+add_task(
+  {
+    pref_set: [["browser.urlbar.suggest.quickactions", false]],
+  },
+  async function test_autofill() {
+    const origin = "example.com";
+    let context = createContext(origin.substring(0, 2), { isPrivate: false });
+    await check_results({
+      context,
+      matches: [
+        makeSearchResult(context, {
+          engineName: "Suggestions",
+          heuristic: true,
+        }),
+      ],
+    });
+    // Add many visits.
+    const url = `https://${origin}/`;
+    await PlacesTestUtils.addVisits(new Array(10).fill(url));
+    Assert.equal(
+      await PlacesUtils.metadata.get("origin_alt_frecency_threshold", 0),
+      0,
+      "Check there's no threshold initially"
+    );
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
+    Assert.greater(
+      await PlacesUtils.metadata.get("origin_alt_frecency_threshold", 0),
+      0,
+      "Check a threshold has been calculated"
+    );
+    await check_results({
+      context,
+      autofilled: `${origin}/`,
+      completed: url,
+      matches: [
+        makeVisitResult(context, {
+          uri: url,
+          title: `test visit for ${url}`,
+          heuristic: true,
+        }),
+      ],
+    });
+    await PlacesUtils.history.clear();
+  }
+);
+
+add_task(
+  {
+    pref_set: [["browser.urlbar.suggest.quickactions", false]],
+  },
+  async function test_autofill_www() {
+    const origin = "example.com";
+    // Add many visits.
+    const url = `https://www.${origin}/`;
+    await PlacesTestUtils.addVisits(new Array(10).fill(url));
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
+
+    let context = createContext(origin.substring(0, 2), { isPrivate: false });
+    await check_results({
+      context,
+      autofilled: `${origin}/`,
+      completed: url,
+      matches: [
+        makeVisitResult(context, {
+          uri: url,
+          title: `test visit for ${url}`,
+          heuristic: true,
+        }),
+      ],
+    });
+    await PlacesUtils.history.clear();
+  }
+);
+
+add_task(
+  {
+    pref_set: [
+      ["browser.urlbar.suggest.quickactions", false],
+      ["browser.urlbar.tabToSearch.onboard.interactionsLeft", 0],
+    ],
+  },
+  async function test_autofill_prefix_priority() {
+    const origin = "localhost";
+    const url = `https://${origin}/`;
+    await PlacesTestUtils.addVisits([url, `http://${origin}/`]);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
+
+    let engine = Services.search.defaultEngine;
+    let context = createContext(origin.substring(0, 2), { isPrivate: false });
+    await check_results({
+      context,
+      autofilled: `${origin}/`,
+      completed: url,
+      matches: [
+        makeVisitResult(context, {
+          uri: url,
+          title: `test visit for ${url}`,
+          heuristic: true,
+        }),
+        makeSearchResult(context, {
+          engineName: engine.name,
+          engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS,
+          uri: UrlbarUtils.stripPublicSuffixFromHost(engine.searchUrlDomain),
+          providesSearchMode: true,
+          query: "",
+          providerName: "TabToSearch",
+        }),
+      ],
+    });
+    await PlacesUtils.history.clear();
+  }
+);
+
+add_task(
+  {
+    pref_set: [["browser.urlbar.suggest.quickactions", false]],
+  },
+  async function test_autofill_threshold() {
+    async function getOriginAltFrecency(origin) {
+      let db = await PlacesUtils.promiseDBConnection();
+      let rows = await db.execute(
+        "SELECT alt_frecency FROM moz_origins WHERE host = :origin",
+        { origin }
+      );
+      return rows?.[0].getResultByName("alt_frecency");
+    }
+
+    await PlacesTestUtils.addVisits(new Array(10).fill("https://example.com/"));
+    await PlacesTestUtils.addVisits("https://somethingelse.org/");
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
+    let threshold = await PlacesUtils.metadata.get(
+      "origin_alt_frecency_threshold",
+      0
+    );
+    Assert.greater(
+      threshold,
+      await getOriginAltFrecency("somethingelse.org"),
+      "Check mozilla.org has a lower frecency than the threshold"
+    );
+
+    let engine = Services.search.defaultEngine;
+    let context = createContext("so", { isPrivate: false });
+    await check_results({
+      context,
+      matches: [
+        makeSearchResult(context, {
+          heuristic: true,
+          query: "so",
+          engineName: engine.name,
+        }),
+        makeVisitResult(context, {
+          uri: "https://somethingelse.org/",
+          title: "test visit for https://somethingelse.org/",
+        }),
+      ],
+    });
+    await PlacesUtils.history.clear();
+  }
+);
--- a/browser/components/urlbar/tests/unit/xpcshell.ini
+++ b/browser/components/urlbar/tests/unit/xpcshell.ini
@ -12,6 +12,8 @@ support-files =
 [test_autofill_do_not_trim.js]
 [test_autofill_functional.js]
 [test_autofill_origins.js]
+[test_autofill_origins_alt_frecency.js]
+prefs = places.frecency.origins.alternative.featureGate=true
 [test_autofill_originsAndQueries.js]
 [test_autofill_prefix_fallback.js]
 [test_autofill_search_engines.js]
--- a/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs
+++ b/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs
@ -445,6 +445,18 @@ class AlternativeFrecencyHelper {
        RETURNING id`
      );
      affectedCount += affected.length;
+
+      // Calculate and store the alternative frecency threshold. Origins above
+      // this threshold will be considered meaningful and autofilled.
+      if (affected.length) {
+        let threshold = (
+          await db.executeCached(`SELECT avg(frecency) FROM moz_origins`)
+        )[0].getResultByIndex(0);
+        await lazy.PlacesUtils.metadata.set(
+          "origin_alt_frecency_threshold",
+          threshold
+        );
+      }
    });

    return affectedCount;
--- a/toolkit/components/places/tests/PlacesTestUtils.sys.mjs
+++ b/toolkit/components/places/tests/PlacesTestUtils.sys.mjs
@ -451,12 +451,20 @@ export var PlacesTestUtils = Object.freeze({
  /**
   * A debugging helper that dumps the contents of an SQLite table.
   *
-   * @param {Sqlite.OpenedConnection} db
-   *        The mirror database connection.
   * @param {String} table
   *        The table name.
+   * @param {Sqlite.OpenedConnection} [db]
+   *        The mirror database connection.
+   * @param {String[]} [columns]
+   *        Clumns to be printed, defaults to all.
   */
-  async dumpTable(db, table, columns = null) {
+  async dumpTable({ table, db, columns }) {
+    if (!table) {
+      throw new Error("Must pass a `table` name");
+    }
+    if (!db) {
+      db = await lazy.PlacesUtils.promiseDBConnection();
+    }
    if (!columns) {
      columns = (await db.execute(`PRAGMA table_info('${table}')`)).map(r =>
        r.getResultByName("name")
--- a/toolkit/components/places/tests/maintenance/test_preventive_maintenance.js
+++ b/toolkit/components/places/tests/maintenance/test_preventive_maintenance.js
@ -1111,11 +1111,11 @@ tests.push({
          rows.forEach(r => {
            aResultArray.push(r.getResultByName("id"));
          });
-          await PlacesTestUtils.dumpTable(db, "moz_bookmarks", [
-            "id",
-            "parent",
-            "position",
-          ]);
+          await PlacesTestUtils.dumpTable({
+            db,
+            table: "moz_bookmarks",
+            columns: ["id", "parent", "position"],
+          });
        });
      });
    }
@ -1155,11 +1155,11 @@ tests.push({
        let position = row.getResultByName("position");
        if (aResultArray.indexOf(id) != position) {
          info("Expected order: " + aResultArray);
-          await PlacesTestUtils.dumpTable(db, "moz_bookmarks", [
-            "id",
-            "parent",
-            "position",
-          ]);
+          await PlacesTestUtils.dumpTable({
+            db,
+            table: "moz_bookmarks",
+            columns: ["id", "parent", "position"],
+          });
          do_throw(`Unexpected bookmarks order for ${aParent}.`);
        }
      }