Bug 1832082 - Introduce alternative frecency for pages. r=daisuke

Differential Revision: https://phabricator.services.mozilla.com/D178700

modules/libpref/init/StaticPrefList.yaml

@@ -12818,6 +12818,42 @@
   value: true
   mirror: always
 
+#---------------------------------------------------------------------------
+# Prefs starting with "places."
+#---------------------------------------------------------------------------
+
+# Whether pages alternative frecency is enabled. This and the following related
+# prefs only apply at restart.
+- name: places.frecency.pages.alternative.featureGate
+  type: bool
+  value: false
+  mirror: once
+
+- name: places.frecency.pages.alternative.highWeight
+  type: uint32_t
+  value: 100
+  mirror: once
+
+- name: places.frecency.pages.alternative.mediumWeight
+  type: uint32_t
+  value: 50
+  mirror: once
+
+- name: places.frecency.pages.alternative.lowWeight
+  type: uint32_t
+  value: 20
+  mirror: once
+
+- name: places.frecency.pages.alternative.halfLifeDays
+  type: uint32_t
+  value: 30
+  mirror: once
+
+- name: places.frecency.pages.alternative.numSampledVisits
+  type: uint32_t
+  value: 10
+  mirror: once
+
 #---------------------------------------------------------------------------
 # Prefs starting with "plain_text."
 #---------------------------------------------------------------------------

modules/libpref/moz.build

@@ -70,6 +70,7 @@ pref_groups = [
     "page_load",
     "pdfjs",
     "permissions",
+    "places",
     "plain_text",
     "plugin",
     "plugins",

toolkit/components/places/Database.cpp

@@ -8,6 +8,7 @@
 #include "mozilla/ScopeExit.h"
 #include "mozilla/SpinEventLoopUntil.h"
 #include "mozilla/JSONStringWriteFuncs.h"
+#include "mozilla/StaticPrefs_places.h"
 
 #include "Database.h"
 
@@ -1640,6 +1641,11 @@ nsresult Database::InitFunctions() {
   rv = SetShouldStartFrecencyRecalculationFunction::create(mMainConn);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  if (StaticPrefs::places_frecency_pages_alternative_featureGate_AtStartup()) {
+    rv = CalculateAltFrecencyFunction::create(mMainConn);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   return NS_OK;
 }
 

toolkit/components/places/History.cpp

@@ -43,6 +43,7 @@
 #include "js/Array.h"  // JS::GetArrayLength, JS::IsArrayObject, JS::NewArrayObject
 #include "js/PropertyAndElement.h"  // JS_DefineElement, JS_GetElement, JS_GetProperty
 #include "mozilla/StaticPrefs_layout.h"
+#include "mozilla/StaticPrefs_places.h"
 #include "mozilla/dom/ContentProcessMessageManager.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/PlacesObservers.h"
@@ -1206,6 +1207,24 @@ class InsertVisitedURIs final : public Runnable {
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
+    if (StaticPrefs::
+            places_frecency_pages_alternative_featureGate_AtStartup()) {
+      nsCOMPtr<mozIStorageStatement> stmt = mHistory->GetStatement(
+          "UPDATE moz_places "
+          "SET alt_frecency = CALCULATE_ALT_FRECENCY(id, :redirect), "
+          "recalc_alt_frecency = 0 "
+          "WHERE id = :page_id");
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
+      rv = stmt->BindInt64ByName("page_id"_ns, aPlace.placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv =
+          stmt->BindInt32ByName("redirect"_ns, aPlace.useFrecencyRedirectBonus);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
     return NS_OK;
   }
 

toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs

@@ -64,16 +64,6 @@ const DEFERRED_TASK_MAX_IDLE_WAIT_MS = 5 * 60000;
 // Number of entries to update at once.
 const DEFAULT_CHUNK_SIZE = 50;
 
-// Pref controlling altenrative origins frecency.
-const ORIGINS_ALT_FRECENCY_PREF =
-  "places.frecency.origins.alternative.featureGate";
-// Current version of alternative origins frecency.
-//  ! IMPORTANT: Always bump this up when making changes to the algorithm.
-const ORIGINS_ALT_FRECENCY_VERSION = 2;
-// Key used to store a descriptor object for the alternative frecency in the
-// moz_meta table.
-const ORIGINS_ALT_FRECENCY_META_KEY = "origin_alternative_frecency";
-
 export class PlacesFrecencyRecalculator {
   classID = Components.ID("1141fd31-4c1a-48eb-8f1a-2f05fad94085");
 
@@ -88,11 +78,12 @@ export class PlacesFrecencyRecalculator {
    */
   #alternativeFrecencyHelper = null;
 
-  originsAlternativeFrecencyInfo = {
+  /**
-    pref: ORIGINS_ALT_FRECENCY_PREF,
+   * This is useful for testing.
-    key: ORIGINS_ALT_FRECENCY_META_KEY,
+   */
-    version: ORIGINS_ALT_FRECENCY_VERSION,
+  get alternativeFrecencyInfo() {
-  };
+    return this.#alternativeFrecencyHelper?.sets;
+  }
 
   constructor() {
     lazy.logger.trace("Initializing Frecency Recalculator");
@@ -205,7 +196,7 @@ export class PlacesFrecencyRecalculator {
 
     // If alternative frecency is enabled, also recalculate a chunk of it.
     affectedCount +=
-      await this.#alternativeFrecencyHelper.recalculateSomeOriginsAlternativeFrecencies(
+      await this.#alternativeFrecencyHelper.recalculateSomeAlternativeFrecencies(
         { chunkSize }
       );
 
@@ -331,112 +322,180 @@ export class PlacesFrecencyRecalculator {
 class AlternativeFrecencyHelper {
   initializedDeferred = lazy.PromiseUtils.defer();
   #recalculator = null;
-  #useOriginsAlternativeFrecency = false;
-  /**
-   * Store an object containing variables influencing the calculation.
-   * Any change to this object will cause a full recalculation on restart.
-   */
-  #variables = null;
 
-  constructor(recalculator) {
+  sets = {
-    this.#recalculator = recalculator;
+    pages: {
-    this.#variables = {
+      // This pref is only read once and used to kick-off recalculations.
-      version: ORIGINS_ALT_FRECENCY_VERSION,
+      enabled: Services.prefs.getBoolPref(
+        "places.frecency.pages.alternative.featureGate",
+        false
+      ),
+      // Key used to store variables in the moz_meta table.
+      metadataKey: "page_alternative_frecency",
+      // The table containing frecency.
+      table: "moz_places",
+      // Object containing variables influencing the calculation.
+      // Any change to this object will cause a full recalculation on restart.
+      variables: {
+        // Current version of origins alternative frecency.
+        //  ! IMPORTANT: Always bump up when making changes to the algorithm.
+        version: 1,
+        highWeight: Services.prefs.getIntPref(
+          "places.frecency.pages.alternative.highWeight",
+          100
+        ),
+        mediumWeight: Services.prefs.getIntPref(
+          "places.frecency.pages.alternative.mediumWeight",
+          50
+        ),
+        lowWeight: Services.prefs.getIntPref(
+          "places.frecency.pages.alternative.lowWeight",
+          20
+        ),
+        halfLifeDays: Services.prefs.getIntPref(
+          "places.frecency.pages.alternative.halfLifeDays",
+          30
+        ),
+        numSampledVisits: Services.prefs.getIntPref(
+          "places.frecency.pages.alternative.numSampledVisits",
+          10
+        ),
+      },
+      method: this.#recalculateSomePagesAlternativeFrecencies,
+    },
+
+    origins: {
+      // This pref is only read once and used to kick-off recalculations.
+      enabled: Services.prefs.getBoolPref(
+        "places.frecency.origins.alternative.featureGate",
+        false
+      ),
+      // Key used to store variables in the moz_meta table.
+      metadataKey: "origin_alternative_frecency",
+      // The table containing frecency.
+      table: "moz_origins",
+      // Object containing variables influencing the calculation.
+      // Any change to this object will cause a full recalculation on restart.
+      variables: {
+        // Current version of origins alternative frecency.
+        //  ! IMPORTANT: Always bump up when making changes to the algorithm.
+        version: 2,
         // Frecencies of pages are ignored after these many days.
         daysCutOff: Services.prefs.getIntPref(
           "places.frecency.origins.alternative.daysCutOff",
           90
         ),
+      },
+      method: this.#recalculateSomeOriginsAlternativeFrecencies,
+    },
   };
 
-    this.#kickOffOriginsAlternativeFrecency()
+  constructor(recalculator) {
+    this.#recalculator = recalculator;
+    this.#kickOffAlternativeFrecencies()
       .catch(console.error)
       .finally(() => this.initializedDeferred.resolve());
   }
 
-  async #kickOffOriginsAlternativeFrecency() {
+  async #kickOffAlternativeFrecencies() {
-    // First check the status of the pref, we only read it once on startup
+    let recalculateFirstChunk = false;
-    // and don't mind if it changes later, since it's pretty much used on
+    for (let [type, set] of Object.entries(this.sets)) {
-    // startup to kick-off recalculations and create some triggers.
+      // Now check the variables cached in the moz_meta table. If not found we
-    this.#useOriginsAlternativeFrecency = Services.prefs.getBoolPref(
+      // assume alternative frecency was disabled in the previous session.
-      ORIGINS_ALT_FRECENCY_PREF,
-      false
-    );
-
-    // Now check the state cached in the moz_meta table. If there's no state we
-    // assume alternative frecency is disabled.
       let storedVariables = await lazy.PlacesUtils.metadata.get(
-      ORIGINS_ALT_FRECENCY_META_KEY,
+        set.metadataKey,
         Object.create(null)
       );
 
       // Check whether this is the first-run, that happens when the alternative
-    // ranking is enabled and it was not at the previous session, or its version
+      // ranking is enabled and it was not at the previous session, or variables
-    // was bumped up. We should recalc all origins alternative frecency.
+      // were changed. We should recalculate all the alternative frecency values.
       if (
-      this.#useOriginsAlternativeFrecency &&
+        set.enabled &&
-      !lazy.ObjectUtils.deepEqual(this.#variables, storedVariables)
+        !lazy.ObjectUtils.deepEqual(set.variables, storedVariables)
       ) {
-      lazy.logger.trace("Origins alternative frecency must be recalculated");
+        lazy.logger.trace(
+          `Alternative frecency of ${type} must be recalculated`
+        );
         await lazy.PlacesUtils.withConnectionWrapper(
-        "PlacesFrecencyRecalculator :: Alternative Origins Frecency Set Recalc",
+          `PlacesFrecencyRecalculator :: ${type} alternative frecency set recalc`,
           async db => {
-          await db.execute(`UPDATE moz_origins SET recalc_alt_frecency = 1`);
+            await db.execute(`UPDATE ${set.table} SET recalc_alt_frecency = 1`);
           }
         );
-      await lazy.PlacesUtils.metadata.set(
+        await lazy.PlacesUtils.metadata.set(set.metadataKey, set.variables);
-        ORIGINS_ALT_FRECENCY_META_KEY,
+        recalculateFirstChunk = true;
-        this.#variables
+        continue;
+      }
+
+      if (!set.enabled && storedVariables) {
+        lazy.logger.trace(`Clean up alternative frecency of ${type}`);
+        // Clear alternative frecency to save on space.
+        await lazy.PlacesUtils.withConnectionWrapper(
+          `PlacesFrecencyRecalculator :: ${type} alternative frecency set NULL`,
+          async db => {
+            await db.execute(`UPDATE ${set.table} SET alt_frecency = NULL`);
+          }
         );
+        await lazy.PlacesUtils.metadata.delete(set.metadataKey);
+      }
+    }
 
-      // Unblock recalculateSomeOriginsAlternativeFrecencies().
+    if (recalculateFirstChunk) {
-      this.initializedDeferred.resolve();
-
       // Do a first recalculation immediately, so we don't leave the user
       // with unranked entries for too long.
-      await this.recalculateSomeOriginsAlternativeFrecencies();
+      await this.recalculateSomeAlternativeFrecencies();
-      if (lazy.PlacesUtils.isInAutomation) {
+
-        Services.obs.notifyObservers(
-          null,
-          "test-origins-alternative-frecency-first-recalc"
-        );
-      }
       // Ensure the recalculation task is armed for a second run.
       lazy.PlacesUtils.history.shouldStartFrecencyRecalculation = true;
       this.#recalculator.maybeStartFrecencyRecalculation();
-      return;
-    }
-
-    if (!this.#useOriginsAlternativeFrecency && storedVariables) {
-      lazy.logger.trace("Origins alternative frecency clean up");
-      // Clear alternative frecency to save on space.
-      await lazy.PlacesUtils.withConnectionWrapper(
-        "PlacesFrecencyRecalculator :: Alternative Origins Frecency Set Null",
-        async db => {
-          await db.execute(`UPDATE moz_origins SET alt_frecency = NULL`);
-        }
-      );
-      await lazy.PlacesUtils.metadata.delete(ORIGINS_ALT_FRECENCY_META_KEY);
     }
   }
 
   /**
-   * Updates a chunk of outdated origins frecency values.
+   * Updates a chunk of outdated frecency values.
    * @param {Number} chunkSize maximum number of entries to update at a time,
    *   set to -1 to update any entry.
    * @resolves {Number} Number of affected pages.
    */
-  async recalculateSomeOriginsAlternativeFrecencies({
+  async recalculateSomeAlternativeFrecencies({
     chunkSize = DEFAULT_CHUNK_SIZE,
   } = {}) {
-    // Check initialization.
+    let affected = 0;
-    await this.initializedDeferred.promise;
+    for (let set of Object.values(this.sets)) {
-
+      if (!set.enabled) {
-    // Check whether we should do anything at all.
+        continue;
-    if (!this.#useOriginsAlternativeFrecency) {
+      }
-      return 0;
+      try {
+        affected += await set.method({ chunkSize, variables: set.variables });
+      } catch (ex) {
+        console.error(ex);
+      }
+    }
+    return affected;
   }
 
+  async #recalculateSomePagesAlternativeFrecencies({ chunkSize, variables }) {
+    lazy.logger.trace(
+      `Recalculate ${chunkSize} alternative pages frecency values`
+    );
+    let db = await lazy.PlacesUtils.promiseUnsafeWritableDBConnection();
+    let affected = await db.executeCached(
+      `UPDATE moz_places
+       SET alt_frecency = CALCULATE_ALT_FRECENCY(moz_places.id),
+           recalc_alt_frecency = 0
+       WHERE id IN (
+        SELECT id FROM moz_places
+          WHERE recalc_alt_frecency = 1
+          ORDER BY frecency DESC
+          LIMIT ${chunkSize}
+      )
+      RETURNING id`
+    );
+    return affected;
+  }
+
+  async #recalculateSomeOriginsAlternativeFrecencies({ chunkSize, variables }) {
     lazy.logger.trace(
       `Recalculate ${chunkSize} alternative origins frecency values`
     );
@@ -451,9 +510,8 @@ class AlternativeFrecencyHelper {
           FROM moz_places h
           WHERE origin_id = moz_origins.id
           AND last_visit_date >
-            strftime('%s','now','localtime','start of day','-${
+            strftime('%s','now','localtime','start of day',
-              this.#variables.daysCutOff
+                     '-${variables.daysCutOff} day','utc') * 1000000
-            } day','utc') * 1000000
         ), recalc_alt_frecency = 0
         WHERE id IN (
           SELECT id FROM moz_origins

toolkit/components/places/SQLFunctions.cpp

@@ -4,6 +4,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/storage.h"
+#include "mozilla/StaticPrefs_places.h"
 #include "nsString.h"
 #include "nsFaviconService.h"
 #include "nsNavBookmarks.h"
@@ -765,6 +766,171 @@ CalculateFrecencyFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
   return NS_OK;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+//// Frecency Calculation Function
+
+/* static */
+nsresult CalculateAltFrecencyFunction::create(mozIStorageConnection* aDBConn) {
+  RefPtr<CalculateAltFrecencyFunction> function =
+      new CalculateAltFrecencyFunction();
+
+  nsresult rv =
+      aDBConn->CreateFunction("calculate_alt_frecency"_ns, -1, function);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
+NS_IMPL_ISUPPORTS(CalculateAltFrecencyFunction, mozIStorageFunction)
+
+NS_IMETHODIMP
+CalculateAltFrecencyFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
+                                             nsIVariant** _result) {
+  // Fetch arguments.  Use default values if they were omitted.
+  uint32_t numEntries;
+  nsresult rv = aArguments->GetNumEntries(&numEntries);
+  NS_ENSURE_SUCCESS(rv, rv);
+  MOZ_ASSERT(numEntries <= 2, "unexpected number of arguments");
+
+  int64_t pageId = aArguments->AsInt64(0);
+  MOZ_ASSERT(pageId > 0, "Should always pass a valid page id");
+  if (pageId <= 0) {
+    *_result = MakeAndAddRef<IntegerVariant>(0).take();
+    return NS_OK;
+  }
+
+  int32_t isRedirect = 0;
+  if (numEntries > 1) {
+    isRedirect = aArguments->AsInt32(1);
+  }
+  // This is a const version of the history object for thread-safety.
+  const nsNavHistory* history = nsNavHistory::GetConstHistoryService();
+  NS_ENSURE_STATE(history);
+  RefPtr<Database> DB = Database::GetDatabase();
+  NS_ENSURE_STATE(DB);
+
+  /*
+    Exponentially decay each visit with an half-life of halfLifeDays.
+    Score per each visit is a weight exponentially decayed depending on how
+    far away is from a reference date, that is the most recent visit date.
+    The weight for each visit is assigned depending on the visit type and other
+    information (bookmarked, a redirect, a typed entry).
+    If a page has no visits, consider a single visit with an high weight and
+    decay its score using the bookmark date as reference time.
+    Frecency is the sum of all the scores / number of samples.
+    The final score is further decayed using the same half-life.
+    To avoid having to decay the score manually, the stored value is the number
+    of days after which the score would become 1.
+
+    TODO: Add reference link to source docs here.
+  */
+  nsCOMPtr<mozIStorageStatement> stmt = DB->GetStatement(
+      "WITH "
+      "lambda (lambda) AS ( "
+      "  SELECT ln(2) / :halfLifeDays "
+      "), "
+      "visits (days, weight) AS ( "
+      "  SELECT "
+      "    v.visit_date / 86400000000, "
+      "    (SELECT CASE "
+      "      WHEN IFNULL(s.visit_type, v.visit_type) = 3 "     // is a bookmark
+      "        OR  ( v.source <> 3 "                           // is a search
+      "          AND IFNULL(s.visit_type, v.visit_type) = 2 "  // is typed
+      "          AND t.id IS NULL AND NOT :isRedirect "        // not a redirect
+      "        ) "
+      "      THEN :highWeight "
+      "      WHEN t.id IS NULL AND NOT :isRedirect "  // not a redirect
+      "       AND IFNULL(s.visit_type, v.visit_type) NOT IN (4, 8, 9) "
+      "      THEN :mediumWeight "
+      "      ELSE :lowWeight "
+      "     END) "
+      "  FROM moz_historyvisits v "
+      // If it's a redirect target, use the visit_type of the source.
+      "  LEFT JOIN moz_historyvisits s ON s.id = v.from_visit "
+      "                               AND v.visit_type IN (5,6) "
+      // If it's a redirect, use a low weight.
+      "  LEFT JOIN moz_historyvisits t ON t.from_visit = v.id "
+      "                               AND t.visit_type IN (5,6) "
+      "  WHERE v.place_id = :pageId "
+      "  ORDER BY v.visit_date DESC "
+      "  LIMIT :numSampledVisits "
+      "), "
+      "bookmark (days, weight) AS ( "
+      "  SELECT dateAdded / 86400000000, 100 "
+      "  FROM moz_bookmarks "
+      "  WHERE fk = :pageId "
+      "  ORDER BY dateAdded DESC "
+      "  LIMIT 1 "
+      "), "
+      "samples (days, weight) AS ( "
+      "  SELECT * FROM bookmark WHERE (SELECT count(*) FROM visits) = 0 "
+      "  UNION ALL "
+      "  SELECT * FROM visits "
+      "), "
+      "reference (days, samples_count) AS ( "
+      "  SELECT max(samples.days), count(*) FROM samples "
+      "), "
+      "scores (score) AS ( "
+      "  SELECT (weight * exp(-lambda * (samples.days - reference.days))) "
+      "  FROM samples, reference, lambda "
+      ") "
+      "SELECT CASE "
+      "WHEN (substr(url, 0, 7) = 'place:') THEN 0 "
+      "ELSE "
+      "  reference.days + CAST (( "
+      "    ln( "
+      "      (sum(score) / samples_count * MAX(visit_count, samples_count)) * "
+      "        exp(-lambda) "
+      "    ) / lambda "
+      "  ) AS INTEGER) "
+      "END "
+      "FROM moz_places h, reference, lambda, scores "
+      "WHERE h.id = :pageId");
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper infoScoper(stmt);
+
+  rv = stmt->BindInt64ByName("pageId"_ns, pageId);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName("isRedirect"_ns, isRedirect);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName(
+      "halfLifeDays"_ns,
+      StaticPrefs::places_frecency_pages_alternative_halfLifeDays_AtStartup());
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName(
+      "numSampledVisits"_ns,
+      StaticPrefs::
+          places_frecency_pages_alternative_numSampledVisits_AtStartup());
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName(
+      "lowWeight"_ns,
+      StaticPrefs::places_frecency_pages_alternative_lowWeight_AtStartup());
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName(
+      "mediumWeight"_ns,
+      StaticPrefs::places_frecency_pages_alternative_mediumWeight_AtStartup());
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindInt64ByName(
+      "highWeight"_ns,
+      StaticPrefs::places_frecency_pages_alternative_highWeight_AtStartup());
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  bool hasResult = false;
+  rv = stmt->ExecuteStep(&hasResult);
+  NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && hasResult, NS_ERROR_UNEXPECTED);
+
+  bool isNull;
+  if (NS_SUCCEEDED(stmt->GetIsNull(0, &isNull)) && isNull) {
+    *_result = MakeAndAddRef<NullVariant>().take();
+  } else {
+    int32_t score;
+    rv = stmt->GetInt32(0, &score);
+    NS_ENSURE_SUCCESS(rv, rv);
+    *_result = MakeAndAddRef<IntegerVariant>(score).take();
+  }
+  return NS_OK;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// GUID Creation Function
 

toolkit/components/places/SQLFunctions.h

@@ -220,6 +220,40 @@ class CalculateFrecencyFunction final : public mozIStorageFunction {
   ~CalculateFrecencyFunction() = default;
 };
 
+////////////////////////////////////////////////////////////////////////////////
+//// Alternative Frecency Calculation Function
+
+/**
+ * This function is used to calculate alternative frecency for a page.
+ *
+ * In SQL, you'd use it in when setting frecency like:
+ * SET alt_frecency = CALCULATE_ALT_FRECENCY(place_id).
+ * Optional parameters must be passed in if the page is not yet in the database,
+ * otherwise they will be fetched from it automatically.
+ *
+ * @param {int64_t} pageId
+ *        The id of the page.  Pass -1 if the page is being added right now.
+ * @param {int32_t} [useRedirectBonus]
+ *        Whether we should use the lower redirect bonus for the most recent
+ *        page visit.  If not passed in, it will use a database guess.
+ */
+class CalculateAltFrecencyFunction final : public mozIStorageFunction {
+ public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_MOZISTORAGEFUNCTION
+
+  /**
+   * Registers the function with the specified database connection.
+   *
+   * @param aDBConn
+   *        The database connection to register with.
+   */
+  static nsresult create(mozIStorageConnection* aDBConn);
+
+ private:
+  ~CalculateAltFrecencyFunction() = default;
+};
+
 /**
  * SQL function to generate a GUID for a place or bookmark item.  This is just
  * a wrapper around GenerateGUID in Helpers.h.

toolkit/components/places/tests/PlacesTestUtils.sys.mjs

@@ -562,21 +562,22 @@ export var PlacesTestUtils = Object.freeze({
   },
 
   /**
-   * Updates a specified field in a database table, based on the given
+   * Updates specified fields in a database table, based on the given
    * conditions.
    * @param {string} table - The name of the database table to add to.
-   * @param {string} field - The name of the field to update the value for.
+   * @param {string} fields - an object with field, value pairs
-   * @param {string} value - The value to set.
+   * @param {Object} [conditions] - An object containing the conditions to filter
-   * @param {Object} conditions - An object containing the conditions to filter
    * the query results. The keys represent the names of the columns to filter
    * by, and the values represent the filter values.
    * @return {Promise} A Promise that resolves to the number of affected rows.
    * @throws If no rows were affected.
    */
-  async updateDatabaseValue(table, field, value, conditions) {
+  async updateDatabaseValues(table, fields, conditions = {}) {
     let { fragment: where, params } = this._buildWhereClause(table, conditions);
-    let query = `UPDATE ${table} SET ${field} = :val ${where} RETURNING rowid`;
+    let query = `UPDATE ${table} SET ${Object.keys(fields)
-    params.val = value;
+      .map(f => f + " = :" + f)
+      .join()} ${where} RETURNING rowid`;
+    params = Object.assign(fields, params);
     return lazy.PlacesUtils.withConnectionWrapper(
       "setDatabaseValue",
       async conn => {

toolkit/components/places/tests/unit/test_frecency_origins_alternative.js

@@ -5,7 +5,7 @@
 // Note: the order of the tests here matters, since we are emulating subsquent
 // starts of the recalculator component with different initial conditions.
 
-let altFrecency = PlacesFrecencyRecalculator.originsAlternativeFrecencyInfo;
+const FEATURE_PREF = "places.frecency.origins.alternative.featureGate";
 
 async function restartRecalculator() {
   let subject = {};
@@ -42,12 +42,17 @@ add_setup(async function () {
 add_task(async function test_normal_init() {
   // Ensure moz_meta doesn't report anything.
   Assert.ok(
-    !Services.prefs.getBoolPref(altFrecency.pref),
+    !PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.enabled,
     "Check the pref is disabled by default"
   );
+  // Avoid hitting the cache, we want to check the actual database value.
+  PlacesUtils.metadata.cache.clear();
   Assert.ok(
     ObjectUtils.isEmpty(
-      await PlacesUtils.metadata.get(altFrecency.key, Object.create(null))
+      await PlacesUtils.metadata.get(
+        PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+        Object.create(null)
+      )
     ),
     "Check there's no variables stored"
   );
@@ -55,46 +60,111 @@ add_task(async function test_normal_init() {
 
 add_task(
   {
-    pref_set: [[altFrecency.pref, true]],
+    pref_set: [[FEATURE_PREF, true]],
   },
   async function test_enable_init() {
-    // Set recalc_alt_frecency = 0 for the entries in moz_origins to verify
+    // Set alt_frecency to NULL and recalc_alt_frecency = 0 for the entries in
-    // they are flipped back to 1.
+    // moz_origins to verify they are recalculated.
-    await PlacesUtils.withConnectionWrapper(
+    await PlacesTestUtils.updateDatabaseValues("moz_origins", {
-      "Set recalc_alt_frecency to 0",
+      alt_frecency: null,
-      async db => {
+      recalc_alt_frecency: 0,
-        await db.execute(`UPDATE moz_origins SET recalc_alt_frecency = 0`);
+    });
+
+    await restartRecalculator();
+
+    // Ensure moz_meta doesn't report anything.
+    Assert.ok(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.enabled,
+      "Check the pref is enabled"
+    );
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.equal(
+      (
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins
+            .metadataKey,
+          Object.create(null)
+        )
+      ).version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.variables
+        .version,
+      "Check the algorithm version has been stored"
+    );
+
+    // Check all alternative frecencies have been calculated, since we just have
+    // a few.
+    let origins = await getAllOrigins();
+    Assert.ok(
+      origins.every(o => o.recalc_alt_frecency == 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      origins.every(o => o.alt_frecency > 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      PlacesFrecencyRecalculator.isRecalculationPending,
+      "Recalculation should be pending"
+    );
   }
 );
 
-    let promiseInitialRecalc = TestUtils.topicObserved(
+add_task(
-      "test-origins-alternative-frecency-first-recalc"
+  {
+    pref_set: [[FEATURE_PREF, true]],
+  },
+  async function test_different_version() {
+    let origins = await getAllOrigins();
+    Assert.ok(
+      origins.every(o => o.recalc_alt_frecency == 0),
+      "All the entries should not need recalculation"
     );
+
+    // Set alt_frecency to NULL for the entries in moz_origins to verify they
+    // are recalculated.
+    await PlacesTestUtils.updateDatabaseValues("moz_origins", {
+      alt_frecency: null,
+    });
+
+    // It doesn't matter that the version is, it just have to be different.
+    let variables = await PlacesUtils.metadata.get(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+      Object.create(null)
+    );
+    variables.version = 999;
+    await PlacesUtils.metadata.set(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+      variables
+    );
+
     await restartRecalculator();
 
     // Check alternative frecency has been marked for recalculation.
     // Note just after init we reculate a chunk, and this test code is expected
     // to run before that... though we can't be sure, so if this starts failing
     // intermittently we'll have to add more synchronization test code.
-    let origins = await getAllOrigins();
+    origins = await getAllOrigins();
+    // Ensure moz_meta has been updated.
     Assert.ok(
-      origins.every(o => o.recalc_alt_frecency == 1),
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.enabled,
-      "All the entries have been marked for recalc"
-    );
-
-    // Ensure moz_meta doesn't report anything.
-    Assert.ok(
-      Services.prefs.getBoolPref(altFrecency.pref),
       "Check the pref is enabled"
     );
-    Assert.equal(
+    // Avoid hitting the cache, we want to check the actual database value.
-      (await PlacesUtils.metadata.get(altFrecency.key, Object.create(null)))
+    PlacesUtils.metadata.cache.clear();
+    Assert.deepEqual(
+      (
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins
+            .metadataKey,
+          Object.create(null)
+        )
+      ).version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.variables
         .version,
-      altFrecency.version,
       "Check the algorithm version has been stored"
     );
 
-    await promiseInitialRecalc;
     // Check all alternative frecencies have been calculated, since we just have
     // a few.
     origins = await getAllOrigins();
@@ -115,57 +185,7 @@ add_task(
 
 add_task(
   {
-    pref_set: [[altFrecency.pref, true]],
+    pref_set: [[FEATURE_PREF, true]],
-  },
-  async function test_different_version() {
-    let origins = await getAllOrigins();
-    Assert.ok(
-      origins.every(o => o.recalc_alt_frecency == 0),
-      "All the entries should not need recalculation"
-    );
-
-    // It doesn't matter that the version is, it just have to be different.
-    let variables = await PlacesUtils.metadata.get(
-      altFrecency.key,
-      Object.create(null)
-    );
-    variables.version = 999;
-    await PlacesUtils.metadata.set(altFrecency.key, variables);
-
-    let promiseInitialRecalc = TestUtils.topicObserved(
-      "test-origins-alternative-frecency-first-recalc"
-    );
-    await restartRecalculator();
-
-    // Check alternative frecency has been marked for recalculation.
-    // Note just after init we reculate a chunk, and this test code is expected
-    // to run before that... though we can't be sure, so if this starts failing
-    // intermittently we'll have to add more synchronization test code.
-    origins = await getAllOrigins();
-    Assert.ok(
-      origins.every(o => o.recalc_alt_frecency == 1),
-      "All the entries have been marked for recalc"
-    );
-
-    // Ensure moz_meta has been updated.
-    Assert.ok(
-      Services.prefs.getBoolPref(altFrecency.pref),
-      "Check the pref is enabled"
-    );
-    Assert.deepEqual(
-      (await PlacesUtils.metadata.get(altFrecency.key, Object.create(null)))
-        .version,
-      altFrecency.version,
-      "Check the algorithm version has been stored"
-    );
-
-    await promiseInitialRecalc;
-  }
-);
-
-add_task(
-  {
-    pref_set: [[altFrecency.pref, true]],
   },
   async function test_different_variables() {
     let origins = await getAllOrigins();
@@ -174,45 +194,62 @@ add_task(
       "All the entries should not need recalculation"
     );
 
+    // Set alt_frecency to NULL for the entries in moz_origins to verify they
+    // are recalculated.
+    await PlacesTestUtils.updateDatabaseValues("moz_origins", {
+      alt_frecency: null,
+    });
+
     // Change variables.
     let variables = await PlacesUtils.metadata.get(
-      altFrecency.key,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
       Object.create(null)
     );
     Assert.greater(Object.keys(variables).length, 1);
     Assert.ok("version" in variables, "At least the version is always present");
-    await PlacesUtils.metadata.set(altFrecency.key, {
+    await PlacesUtils.metadata.set(
-      version: altFrecency.version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+      {
+        version:
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.variables
+            .version,
         someVar: 1,
-    });
+      }
-
-    let promiseInitialRecalc = TestUtils.topicObserved(
-      "test-origins-alternative-frecency-first-recalc"
     );
+
     await restartRecalculator();
 
-    // Check alternative frecency has been marked for recalculation.
-    // Note just after init we reculate a chunk, and this test code is expected
-    // to run before that... though we can't be sure, so if this starts failing
-    // intermittently we'll have to add more synchronization test code.
-    origins = await getAllOrigins();
-    Assert.ok(
-      origins.every(o => o.recalc_alt_frecency == 1),
-      "All the entries have been marked for recalc"
-    );
-
     // Ensure moz_meta has been updated.
     Assert.ok(
-      Services.prefs.getBoolPref(altFrecency.pref),
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.enabled,
       "Check the pref is enabled"
     );
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
     Assert.deepEqual(
-      await PlacesUtils.metadata.get(altFrecency.key, Object.create(null)),
+      await PlacesUtils.metadata.get(
+        PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+        Object.create(null)
+      ),
       variables,
       "Check the algorithm variables have been stored"
     );
 
-    await promiseInitialRecalc;
+    // Check all alternative frecencies have been calculated, since we just have
+    // a few.
+    origins = await getAllOrigins();
+    Assert.ok(
+      origins.every(o => o.recalc_alt_frecency == 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      origins.every(o => o.alt_frecency > 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      PlacesFrecencyRecalculator.isRecalculationPending,
+      "Recalculation should be pending"
+    );
   }
 );
 
@@ -222,11 +259,17 @@ add_task(async function test_disable() {
     origins.every(o => o.recalc_alt_frecency == 0),
     "All the entries should not need recalculation"
   );
-
+  // Avoid hitting the cache, we want to check the actual database value.
+  PlacesUtils.metadata.cache.clear();
   Assert.equal(
-    (await PlacesUtils.metadata.get(altFrecency.key, Object.create(null)))
+    (
+      await PlacesUtils.metadata.get(
+        PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+        Object.create(null)
+      )
+    ).version,
+    PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.variables
       .version,
-    altFrecency.version,
     "Check the algorithm version has been stored"
   );
 
@@ -244,9 +287,14 @@ add_task(async function test_disable() {
   );
 
   // Ensure moz_meta has been updated.
+  // Avoid hitting the cache, we want to check the actual database value.
+  PlacesUtils.metadata.cache.clear();
   Assert.ok(
     ObjectUtils.isEmpty(
-      await PlacesUtils.metadata.get(altFrecency.key, Object.create(null))
+      await PlacesUtils.metadata.get(
+        PlacesFrecencyRecalculator.alternativeFrecencyInfo.origins.metadataKey,
+        Object.create(null)
+      )
     ),
     "Check the algorithm variables has been removed"
   );

toolkit/components/places/tests/unit/test_frecency_pages_alternative.js

@@ -0,0 +1,352 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests alternative pages frecency.
+// Note: the order of the tests here matters, since we are emulating subsquent
+// starts of the recalculator component with different initial conditions.
+
+const FEATURE_PREF = "places.frecency.pages.alternative.featureGate";
+
+async function restartRecalculator() {
+  let subject = {};
+  PlacesFrecencyRecalculator.observe(
+    subject,
+    "test-alternative-frecency-init",
+    ""
+  );
+  await subject.promise;
+}
+
+async function getAllPages() {
+  let db = await PlacesUtils.promiseDBConnection();
+  let rows = await db.execute(`SELECT * FROM moz_places`);
+  Assert.greater(rows.length, 0);
+  return rows.map(r => ({
+    url: r.getResultByName("url"),
+    frecency: r.getResultByName("frecency"),
+    recalc_frecency: r.getResultByName("recalc_frecency"),
+    alt_frecency: r.getResultByName("alt_frecency"),
+    recalc_alt_frecency: r.getResultByName("recalc_alt_frecency"),
+  }));
+}
+
+add_setup(async function () {
+  await PlacesTestUtils.addVisits([
+    "https://testdomain1.moz.org",
+    "https://testdomain2.moz.org",
+    "https://testdomain3.moz.org",
+  ]);
+  registerCleanupFunction(PlacesUtils.history.clear);
+});
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, false]],
+  },
+  async function test_normal_init() {
+    // The test starts with the pref enabled, otherwise we'd not have the SQL
+    // function defined. So here we disable it, then enable again later.
+    await restartRecalculator();
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.ok(
+      ObjectUtils.isEmpty(
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+          Object.create(null)
+        )
+      ),
+      "Check there's no variables stored"
+    );
+  }
+);
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, true]],
+  },
+  async function test_enable_init() {
+    // Set alt_frecency to NULL and recalc_alt_frecency = 0 for the entries in
+    // moz_places to verify they are recalculated.
+    await PlacesTestUtils.updateDatabaseValues("moz_places", {
+      alt_frecency: null,
+      recalc_alt_frecency: 0,
+    });
+
+    await restartRecalculator();
+
+    // Ensure moz_meta doesn't report anything.
+    Assert.ok(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.enabled,
+      "Check the pref is enabled"
+    );
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.equal(
+      (
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+          Object.create(null)
+        )
+      ).version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.variables
+        .version,
+      "Check the algorithm version has been stored"
+    );
+
+    // Check all alternative frecencies have been calculated, since we just have
+    // a few.
+    let pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      pages.every(p => p.alt_frecency > 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      PlacesFrecencyRecalculator.isRecalculationPending,
+      "Recalculation should be pending"
+    );
+  }
+);
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, true]],
+  },
+  async function test_different_version() {
+    let pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries should not need recalculation"
+    );
+
+    // Set alt_frecency to NULL to verify all the entries are recalculated.
+    await PlacesTestUtils.updateDatabaseValues("moz_places", {
+      alt_frecency: null,
+    });
+
+    // It doesn't matter that the version is, it just have to be different.
+    let variables = await PlacesUtils.metadata.get(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+      Object.create(null)
+    );
+    variables.version = 999;
+    await PlacesUtils.metadata.set(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+      variables
+    );
+
+    await restartRecalculator();
+
+    // Check alternative frecency has been marked for recalculation.
+    // Note just after init we reculate a chunk, and this test code is expected
+    // to run before that... though we can't be sure, so if this starts failing
+    // intermittently we'll have to add more synchronization test code.
+    pages = await getAllPages();
+    // Ensure moz_meta has been updated.
+    Assert.ok(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.enabled,
+      "Check the pref is enabled"
+    );
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.deepEqual(
+      (
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+          Object.create(null)
+        )
+      ).version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.variables
+        .version,
+      "Check the algorithm version has been stored"
+    );
+
+    // Check all alternative frecencies have been calculated, since we just have
+    // a few.
+    pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      pages.every(p => p.alt_frecency > 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      PlacesFrecencyRecalculator.isRecalculationPending,
+      "Recalculation should be pending"
+    );
+  }
+);
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, true]],
+  },
+  async function test_different_variables() {
+    let pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries should not need recalculation"
+    );
+
+    // Set alt_frecency to NULL to verify all the entries are recalculated.
+    await PlacesTestUtils.updateDatabaseValues("moz_places", {
+      alt_frecency: null,
+    });
+
+    // Change variables.
+    let variables = await PlacesUtils.metadata.get(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+      Object.create(null)
+    );
+    Assert.greater(Object.keys(variables).length, 1);
+    Assert.ok("version" in variables, "At least the version is always present");
+    await PlacesUtils.metadata.set(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+      {
+        version:
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.variables
+            .version,
+        someVar: 1,
+      }
+    );
+
+    await restartRecalculator();
+
+    // Ensure moz_meta has been updated.
+    Assert.ok(
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.enabled,
+      "Check the pref is enabled"
+    );
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.deepEqual(
+      await PlacesUtils.metadata.get(
+        PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+        Object.create(null)
+      ),
+      variables,
+      "Check the algorithm variables have been stored"
+    );
+
+    // Check all alternative frecencies have been calculated, since we just have
+    // a few.
+    pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      pages.every(p => p.alt_frecency > 0),
+      "All the entries have been recalculated"
+    );
+    Assert.ok(
+      PlacesFrecencyRecalculator.isRecalculationPending,
+      "Recalculation should be pending"
+    );
+  }
+);
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, false]],
+  },
+  async function test_disable() {
+    let pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "All the entries should not need recalculation"
+    );
+
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.equal(
+      (
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+          Object.create(null)
+        )
+      ).version,
+      PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.variables
+        .version,
+      "Check the algorithm version has been stored"
+    );
+
+    await restartRecalculator();
+
+    // Check alternative frecency has not been marked for recalculation.
+    pages = await getAllPages();
+    Assert.ok(
+      pages.every(p => p.recalc_alt_frecency == 0),
+      "The entries not have been marked for recalc"
+    );
+    Assert.ok(
+      pages.every(p => p.alt_frecency === null),
+      "All the alt_frecency values should have been nullified"
+    );
+
+    // Ensure moz_meta has been updated.
+    // Avoid hitting the cache, we want to check the actual database value.
+    PlacesUtils.metadata.cache.clear();
+    Assert.ok(
+      ObjectUtils.isEmpty(
+        await PlacesUtils.metadata.get(
+          PlacesFrecencyRecalculator.alternativeFrecencyInfo.pages.metadataKey,
+          Object.create(null)
+        )
+      ),
+      "Check the algorithm variables has been removed"
+    );
+  }
+);
+
+add_task(
+  {
+    pref_set: [[FEATURE_PREF, true]],
+  },
+  async function test_score() {
+    await restartRecalculator();
+
+    // This is not intended to cover the algorithm as a whole, but just as a
+    // sanity check for scores.
+
+    await PlacesTestUtils.addVisits([
+      {
+        url: "https://low.moz.org",
+        transition: PlacesUtils.history.TRANSITIONS.FRAMED_LINK,
+      },
+      {
+        url: "https://old.moz.org",
+        visitDate: (Date.now() - 2 * 86400000) * 1000,
+      },
+      { url: "https://base.moz.org" },
+      { url: "https://manyvisits.moz.org" },
+      { url: "https://manyvisits.moz.org" },
+      { url: "https://manyvisits.moz.org" },
+      { url: "https://manyvisits.moz.org" },
+    ]);
+    await PlacesUtils.bookmarks.insert({
+      url: "https://unvisitedbookmark.moz.org",
+      parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    });
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
+    let getFrecency = url =>
+      PlacesTestUtils.getDatabaseValue("moz_places", "alt_frecency", {
+        url,
+      });
+    let low = await getFrecency("https://low.moz.org/");
+    let old = await getFrecency("https://old.moz.org/");
+    Assert.greater(old, low);
+    let base = await getFrecency("https://base.moz.org/");
+    Assert.greater(base, old);
+    let unvisitedBm = await getFrecency("https://unvisitedbookmark.moz.org/");
+    Assert.greater(unvisitedBm, base);
+    let manyVisits = await getFrecency("https://manyvisits.moz.org/");
+    Assert.greater(manyVisits, unvisitedBm);
+  }
+);

toolkit/components/places/tests/unit/test_frecency_pages_recalc_alt.js

@@ -7,17 +7,20 @@ add_task(async function test() {
   info("test recalc_alt_frecency is set to 1 when a visit is added");
   const now = new Date();
   const URL = "https://mozilla.org/test/";
-  let getValue = url =>
+  let getRecalc = url =>
     PlacesTestUtils.getDatabaseValue("moz_places", "recalc_alt_frecency", {
       url,
     });
-  let setValue = (url, val) =>
+  let setRecalc = (url, val) =>
-    PlacesTestUtils.updateDatabaseValue(
+    PlacesTestUtils.updateDatabaseValues(
       "moz_places",
-      "recalc_alt_frecency",
+      { recalc_alt_frecency: val },
-      val,
       { url }
     );
+  let getFrecency = url =>
+    PlacesTestUtils.getDatabaseValue("moz_places", "alt_frecency", {
+      url,
+    });
   await PlacesTestUtils.addVisits([
     {
       url: URL,
@@ -28,46 +31,46 @@ add_task(async function test() {
       visitDate: new Date(new Date().setDate(now.getDate() - 30)),
     },
   ]);
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 1);
-  await setValue(URL, 0);
+  Assert.greater(await getFrecency(URL), 0);
 
   info("Remove just one visit (otherwise the page would be orphaned).");
   await PlacesUtils.history.removeVisitsByFilter({
     beginDate: new Date(now.valueOf() - 10000),
     endDate: new Date(now.valueOf() + 10000),
   });
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 1);
-  await setValue(URL, 0);
+  await setRecalc(URL, 0);
 
   info("Add a bookmark to the page");
   let bm = await PlacesUtils.bookmarks.insert({
     url: URL,
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
   });
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 1);
-  await setValue(URL, 0);
+  await setRecalc(URL, 0);
 
   info("Clear history");
   await PlacesUtils.history.clear();
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 1);
-  await setValue(URL, 0);
+  await setRecalc(URL, 0);
 
   // Add back a visit so the page is not an orphan once we remove the bookmark.
   await PlacesTestUtils.addVisits(URL);
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 0);
-  await setValue(URL, 0);
+  Assert.greater(await getFrecency(URL), 0);
 
   info("change the bookmark URL");
   const URL2 = "https://editedbookmark.org/";
   bm.url = URL2;
   await PlacesUtils.bookmarks.update(bm);
-  Assert.equal(await getValue(URL), 1);
+  Assert.equal(await getRecalc(URL), 1);
-  Assert.equal(await getValue(URL2), 1);
+  Assert.equal(await getRecalc(URL2), 1);
-  await setValue(URL, 0);
+  await setRecalc(URL, 0);
-  await setValue(URL2, 0);
+  await setRecalc(URL2, 0);
 
   info("Remove the bookmark from the page");
   await PlacesUtils.bookmarks.remove(bm);
-  Assert.equal(await getValue(URL2), 1);
+  Assert.equal(await getRecalc(URL2), 1);
-  await setValue(URL2, 0);
+  await setRecalc(URL2, 0);
 });

toolkit/components/places/tests/unit/xpcshell.ini

@@ -1,6 +1,7 @@
 [DEFAULT]
 head = head_bookmarks.js
 firefox-appdir = browser
+prefs = places.loglevel="All"
 support-files =
   bookmarks.corrupt.html
   bookmarks.json
@@ -59,7 +60,10 @@ skip-if = os == "linux" # Bug 821781
 [test_frecency_decay.js]
 [test_frecency_origins_alternative.js]
 [test_frecency_origins_recalc.js]
+[test_frecency_pages_alternative.js]
+prefs = places.frecency.pages.alternative.featureGate=true
 [test_frecency_pages_recalc_alt.js]
+prefs = places.frecency.pages.alternative.featureGate=true
 [test_frecency_recalc_triggers.js]
 [test_frecency_recalculator.js]
 [test_frecency_unvisited_bookmark.js]