Bug 1942222 - Hook up the AMP-suggestion-matching strategy and add a Nimbus variable for it. r=daisuke

Depends on D233983, D234787

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

Author: 0c0w3

browser/app/profile/firefox.js

@@ -583,6 +583,13 @@ pref("browser.urlbar.quicksuggest.impressionCaps.sponsoredEnabled", false);
 // characters than this threshold.
 pref("browser.urlbar.quicksuggest.ampTopPickCharThreshold", 0);
 
+// The matching strategy for AMP suggestions. Zero is the usual default
+// exact-keyword strategy. Other values are the integers defined on
+// `AmpMatchingStrategy` in `RustSuggest.sys.mjs` (corresponding to the
+// `AmpMatchingStrategy` enum in the Rust component coerced to a 1-based integer
+// value).
+pref("browser.urlbar.quicksuggest.ampMatchingStrategy", 0);
+
 // Comma-separated list of Suggest exposure suggestion types to enable.
 pref("browser.urlbar.quicksuggest.exposureSuggestionTypes", "");
 

browser/components/urlbar/private/AmpSuggestions.sys.mjs

@@ -7,6 +7,7 @@ import { SuggestProvider } from "resource:///modules/urlbar/private/SuggestFeatu
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
+  AmpMatchingStrategy: "resource://gre/modules/RustSuggest.sys.mjs",
   CONTEXTUAL_SERVICES_PING_TYPES:
     "resource:///modules/PartnerLinkAttribution.sys.mjs",
   QuickSuggest: "resource:///modules/QuickSuggest.sys.mjs",
@@ -46,6 +47,25 @@ export class AmpSuggestions extends SuggestProvider {
     return "Amp";
   }
 
+  get rustProviderConstraints() {
+    let intValue = lazy.UrlbarPrefs.get("ampMatchingStrategy");
+    if (!intValue) {
+      // If the value is zero or otherwise falsey, use the usual default
+      // exact-keyword strategy by returning null here.
+      return null;
+    }
+    if (!Object.values(lazy.AmpMatchingStrategy).includes(intValue)) {
+      this.logger.error(
+        "Unknown AmpMatchingStrategy value, using default strategy",
+        { intValue }
+      );
+      return null;
+    }
+    return {
+      ampAlternativeMatching: intValue,
+    };
+  }
+
   isSuggestionSponsored() {
     return true;
   }

browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js

@@ -7,6 +7,11 @@
 
 "use strict";
 
+ChromeUtils.defineESModuleGetters(this, {
+  AmpMatchingStrategy: "resource://gre/modules/RustSuggest.sys.mjs",
+  SuggestionProvider: "resource://gre/modules/RustSuggest.sys.mjs",
+});
+
 const SPONSORED_SEARCH_STRING = "amp";
 const NONSPONSORED_SEARCH_STRING = "wikipedia";
 const SPONSORED_AND_NONSPONSORED_SEARCH_STRING = "sponsored and non-sponsored";
@@ -1868,3 +1873,124 @@ add_task(async function ampTopPickCharThreshold_zero() {
 
   UrlbarPrefs.clear("quicksuggest.ampTopPickCharThreshold");
 });
+
+// Tests `ampMatchingStrategy`.
+add_task(async function ampMatchingStrategy() {
+  UrlbarPrefs.set("suggest.quicksuggest.nonsponsored", false);
+  UrlbarPrefs.set("suggest.quicksuggest.sponsored", true);
+  await QuickSuggestTestUtils.forceSync();
+
+  // Test each strategy in `AmpMatchingStrategy`. There are only a few.
+  for (let [key, value] of Object.entries(AmpMatchingStrategy)) {
+    await doAmpMatchingStrategyTest({ key, value });
+
+    // Reset back to the default strategy just to make sure that works.
+    await doAmpMatchingStrategyTest({
+      key: "(default)",
+      value: 0,
+    });
+  }
+
+  // Test an invalid strategy integer value. The default strategy should
+  // actually be used. First we need to set a valid non-default strategy.
+  await doAmpMatchingStrategyTest({
+    key: "FTS_AGAINST_TITLE",
+    value: AmpMatchingStrategy.FTS_AGAINST_TITLE,
+  });
+  await doAmpMatchingStrategyTest({
+    key: "(invalid)",
+    value: 99,
+    expectedStrategy: 0,
+  });
+
+  Services.prefs.clearUserPref(
+    "browser.urlbar.quicksuggest.ampMatchingStrategy"
+  );
+  await QuickSuggestTestUtils.forceSync();
+});
+
+async function doAmpMatchingStrategyTest({
+  key,
+  value,
+  expectedStrategy = value,
+}) {
+  info("Doing ampMatchingStrategy test: " + JSON.stringify({ key, value }));
+
+  let sandbox = sinon.createSandbox();
+  let ingestSpy = sandbox.spy(QuickSuggest.rustBackend._test_store, "ingest");
+
+  // Set the strategy. It should trigger ingest. (Assuming it's different from
+  // the current strategy. If it's not, ingest won't happen.)
+  Services.prefs.setIntPref(
+    "browser.urlbar.quicksuggest.ampMatchingStrategy",
+    value
+  );
+
+  let ingestCall = await TestUtils.waitForCondition(() => {
+    return ingestSpy.getCalls().find(call => {
+      let ingestConstraints = call.args[0];
+      return ingestConstraints?.providers[0] == SuggestionProvider.AMP;
+    });
+  }, "Waiting for ingest() to be called with Amp provider");
+
+  // Check the provider constraints in the ingest constraints.
+  let { providerConstraints } = ingestCall.args[0];
+  if (!expectedStrategy) {
+    Assert.ok(
+      !providerConstraints,
+      "ingest() should not have been called with provider constraints"
+    );
+  } else {
+    Assert.ok(
+      providerConstraints,
+      "ingest() should have been called with provider constraints"
+    );
+    Assert.strictEqual(
+      providerConstraints.ampAlternativeMatching,
+      expectedStrategy,
+      "ampAlternativeMatching should have been set"
+    );
+  }
+
+  // Now do a query to make sure it also uses the correct provider constraints.
+  // No need to use `check_results()`. We only need to trigger a query, and
+  // checking the right results unnecessarily complicates things.
+  let querySpy = sandbox.spy(
+    QuickSuggest.rustBackend._test_store,
+    "queryWithMetrics"
+  );
+
+  let controller = UrlbarTestUtils.newMockController();
+  await controller.startQuery(
+    createContext(SPONSORED_SEARCH_STRING, {
+      providers: [UrlbarProviderQuickSuggest.name],
+      isPrivate: false,
+    })
+  );
+
+  let queryCalls = querySpy.getCalls();
+  Assert.equal(queryCalls.length, 1, "query() should have been called once");
+
+  let query = queryCalls[0].args[0];
+  Assert.ok(query, "query() should have been called with a query object");
+  Assert.ok(
+    query.providerConstraints,
+    "query() should have been called with provider constraints"
+  );
+
+  if (!expectedStrategy) {
+    Assert.strictEqual(
+      query.providerConstraints.ampAlternativeMatching,
+      null,
+      "ampAlternativeMatching should not have been set on query provider constraints"
+    );
+  } else {
+    Assert.strictEqual(
+      query.providerConstraints.ampAlternativeMatching,
+      expectedStrategy,
+      "ampAlternativeMatching should have been set on query provider constraints"
+    );
+  }
+
+  sandbox.restore();
+}

toolkit/components/nimbus/FeatureManifest.yaml

@@ -232,6 +232,15 @@ urlbar:
         will be able to click the "Show less frequently" command for addon
         suggestions. If undefined or zero, the user will be able to click the
         command without any limit.
+    ampMatchingStrategy:
+      type: int
+      fallbackPref: browser.urlbar.quicksuggest.ampMatchingStrategy
+      description: >-
+        The matching strategy for AMP suggestions. Leave undefined or set to
+        zero to use the usual default exact-keyword strategy. Otherwise set to
+        an integer value defined on `AmpMatchingStrategy` in
+        `RustSuggest.sys.mjs` (corresponding to the `AmpMatchingStrategy` enum
+        in the Rust component coerced to a 1-based integer value).
     autoFillAdaptiveHistoryEnabled:
       type: boolean
       fallbackPref: browser.urlbar.autoFill.adaptiveHistory.enabled