Bug 1942435 - Fix dismissals for Merino suggestions that aren't managed by a Suggest feature. r=daisuke

I don't think we have any tests specifically for Merino suggestions that don't
correspond to Suggest features, i.e., suggestions with a Merino provider that is
technically unrecognized by Firefox. Dynamic Wikipedia suggestions and
navigational suggestions (a.k.a. top sites) are two examples. So I added tasks
for both sponsored and nonsponsored suggestions to make sure they're disabled
when the relevant pref is disabled, in addition to testing this bug itself.

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

Author: 0c0w3

browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs

@@ -266,13 +266,27 @@ class ProviderQuickSuggest extends UrlbarProvider {
   }
 
   onEngagement(queryContext, controller, details) {
-    let feature = this.#getFeatureByResult(details.result);
-    feature?.onEngagement(
-      queryContext,
-      controller,
-      details,
-      this._trimmedSearchString
-    );
+    let { result } = details;
+
+    // Delegate to the result's feature if there is one.
+    let feature = this.#getFeatureByResult(result);
+    if (feature) {
+      feature.onEngagement(
+        queryContext,
+        controller,
+        details,
+        this._trimmedSearchString
+      );
+      return;
+    }
+
+    // Otherwise, handle commands. The dismiss, manage, and help commands are
+    // supported for results without features. Dismissal is the only one we need
+    // to handle here since urlbar handles the others.
+    if (details.selType == "dismiss" && result.payload.isBlockable) {
+      lazy.QuickSuggest.blockedSuggestions.add(result.payload.url);
+      controller.removeResult(result);
+    }
   }
 
   onSearchSessionEnd(queryContext, controller, details) {

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

@@ -544,22 +544,61 @@ async function doRustProvidersTests({ searchString, tests }) {
  *   The result that the command will act on.
  * @param {string} options.searchString
  *   The search string to pass to `onEngagement()`.
+ * @param {object} options.expectedCountsByCall
+ *   If non-null, this should map controller and view method names to the number
+ *   of times they should be called in response to the command.
+ * @returns {Map}
+ *   A map from names of methods on the controller and view to the number of
+ *   times they were called.
  */
-function triggerCommand({ feature, command, result, searchString = "" }) {
+function triggerCommand({
+  feature,
+  command,
+  result,
+  searchString = "",
+  expectedCountsByCall = null,
+}) {
   info(`Calling ${feature.name}.onEngagement() to trigger command: ${command}`);
+
+  let countsByCall = new Map();
+  let addCall = name => {
+    if (!countsByCall.has(name)) {
+      countsByCall.set(name, 0);
+    }
+    countsByCall.set(name, countsByCall.get(name) + 1);
+  };
+
   feature.onEngagement(
     // query context
     {},
     // controller
     {
-      removeResult() {},
+      removeResult() {
+        addCall("removeResult");
+      },
       view: {
-        acknowledgeFeedback() {},
-        invalidateResultMenuCommands() {},
+        acknowledgeFeedback() {
+          addCall("acknowledgeFeedback");
+        },
+        invalidateResultMenuCommands() {
+          addCall("invalidateResultMenuCommands");
+        },
       },
     },
     // details
     { result, selType: command },
     searchString
   );
+
+  if (expectedCountsByCall) {
+    for (let [name, expectedCount] of Object.entries(expectedCountsByCall)) {
+      Assert.equal(
+        countsByCall.get(name) ?? 0,
+        expectedCount,
+        "Function should have been called the expected number of times: " + name
+      );
+    }
+  }
+
+  return countsByCall;
 }

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

@@ -308,6 +308,9 @@ add_task(async function notRelevant() {
     result,
     command: "not_relevant",
     feature: QuickSuggest.getFeature("FakespotSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
   await QuickSuggest.blockedSuggestions._test_readyPromise;
 
@@ -359,6 +362,9 @@ add_task(async function notInterested() {
     result,
     command: "not_interested",
     feature: QuickSuggest.getFeature("FakespotSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
 
   Assert.ok(

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

@@ -563,6 +563,121 @@ add_task(async function bestMatch() {
   merinoClient().resetSession();
 });
 
+// Tests a sponsored suggestion that isn't managed by a feature.
+add_task(async function unmanaged_sponsored() {
+  await doUnmanagedTest({
+    pref: "suggest.quicksuggest.sponsored",
+    suggestion: {
+      title: "Sponsored without feature",
+      url: "https://example.com/sponsored-without-feature",
+      provider: "sponsored-unrecognized-provider",
+      is_sponsored: true,
+    },
+  });
+});
+
+// Tests a nonsponsored suggestion that isn't managed by a feature.
+add_task(async function unmanaged_nonsponsored() {
+  await doUnmanagedTest({
+    pref: "suggest.quicksuggest.nonsponsored",
+    suggestion: {
+      title: "Nonsponsored without feature",
+      url: "https://example.com/nonsponsored-without-feature",
+      provider: "nonsponsored-unrecognized-provider",
+      // no is_sponsored
+    },
+  });
+});
+
+async function doUnmanagedTest({ pref, suggestion }) {
+  UrlbarPrefs.set("suggest.quicksuggest.nonsponsored", true);
+  UrlbarPrefs.set("suggest.quicksuggest.sponsored", true);
+  await QuickSuggestTestUtils.forceSync();
+
+  UrlbarPrefs.set(PREF_DATA_COLLECTION_ENABLED, true);
+  MerinoTestUtils.server.response.body.suggestions = [suggestion];
+
+  let expectedResult = {
+    type: UrlbarUtils.RESULT_TYPE.URL,
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+    heuristic: false,
+    payload: {
+      title: suggestion.title,
+      url: suggestion.url,
+      displayUrl: suggestion.url.substring("https://".length),
+      provider: suggestion.provider,
+      telemetryType: suggestion.provider,
+      isSponsored: !!suggestion.is_sponsored,
+      source: "merino",
+      isBlockable: true,
+      blockL10n: {
+        id: "urlbar-result-menu-dismiss-firefox-suggest",
+      },
+      isManageable: true,
+      shouldShowUrl: true,
+    },
+  };
+
+  // Do an initial search. Sponsored and nonsponsored suggestions are both
+  // enabled, so the suggestion should be matched.
+  info("Doing search 1");
+  await check_results({
+    context: createContext("test", {
+      providers: [UrlbarProviderQuickSuggest.name],
+      isPrivate: false,
+    }),
+    matches: [expectedResult],
+  });
+
+  // Set the pref to false and do another search. The suggestion shouldn't be
+  // matched.
+  UrlbarPrefs.set(pref, false);
+  await QuickSuggestTestUtils.forceSync();
+
+  info("Doing search 2");
+  await check_results({
+    context: createContext("test", {
+      providers: [UrlbarProviderQuickSuggest.name],
+      isPrivate: false,
+    }),
+    matches: [],
+  });
+
+  // Flip the pref back to true and do a third search.
+  UrlbarPrefs.set(pref, true);
+  await QuickSuggestTestUtils.forceSync();
+
+  info("Doing search 3");
+  let context = createContext("test", {
+    providers: [UrlbarProviderQuickSuggest.name],
+    isPrivate: false,
+  });
+  await check_results({
+    context,
+    matches: [expectedResult],
+  });
+
+  // Trigger the dismiss command on the result.
+  triggerCommand({
+    feature: UrlbarProviderQuickSuggest,
+    command: "dismiss",
+    result: context.results[0],
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
+  });
+  await QuickSuggest.blockedSuggestions._test_readyPromise;
+
+  Assert.ok(
+    await QuickSuggest.blockedSuggestions.has(suggestion.url),
+    "The suggestion URL should be blocked"
+  );
+
+  await QuickSuggest.blockedSuggestions.clear();
+  MerinoTestUtils.server.reset();
+  merinoClient().resetSession();
+}
+
 function merinoClient() {
   return QuickSuggest.getFeature("SuggestBackendMerino")?.client;
 }

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

@@ -334,6 +334,9 @@ add_task(async function notRelevant() {
     result,
     command: "not_relevant",
     feature: QuickSuggest.getFeature("PocketSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
   await QuickSuggest.blockedSuggestions._test_readyPromise;
 
@@ -396,6 +399,9 @@ add_task(async function notInterested() {
     result,
     command: "not_interested",
     feature: QuickSuggest.getFeature("PocketSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
 
   Assert.ok(

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

@@ -542,8 +542,10 @@ add_task(async function notRelevant() {
     result,
     command: "not_relevant",
     feature: QuickSuggest.getFeature("YelpSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
-
   await QuickSuggest.blockedSuggestions._test_readyPromise;
 
   Assert.ok(
@@ -618,6 +620,9 @@ add_task(async function notInterested() {
     result,
     command: "not_interested",
     feature: QuickSuggest.getFeature("YelpSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
 
   Assert.ok(
@@ -747,6 +752,10 @@ add_task(async function showLessFrequently() {
       feature,
       command: "show_less_frequently",
       searchString: input,
+      expectedCountsByCall: {
+        acknowledgeFeedback: 1,
+        invalidateResultMenuCommands: after.canShowLessFrequently ? 0 : 1,
+      },
     });
 
     Assert.equal(

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

@@ -482,6 +482,9 @@ add_task(async function notRelevant() {
     result,
     command: "not_relevant",
     feature: QuickSuggest.getFeature("YelpSuggestions"),
+    expectedCountsByCall: {
+      removeResult: 1,
+    },
   });
   await QuickSuggest.blockedSuggestions._test_readyPromise;
 

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

@@ -610,6 +610,10 @@ async function doShowLessFrequentlyTest({
       result: expectedResult,
       command: "show_less_frequently",
       searchString: input,
+      expectedCountsByCall: {
+        acknowledgeFeedback: 1,
+        invalidateResultMenuCommands: after.canShowLessFrequently ? 0 : 1,
+      },
     });
 
     Assert.equal(