Bug 1697392 - Simplify SearchSuggestionController to return the form history results, rather than an FormAutoCompleteResult. r=mak

This moves creating FormAutoCompleteResult out of the controller and into SearchSuggestions.
SearchSuggestions is only used for the autocomplete interface and so this simplifies the work for content search and address bar.

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

browser/actors/ContentSearchParent.sys.mjs

@@ -87,7 +87,7 @@ export let ContentSearch = {
   _currentEventPromise: null,
 
   // This is used to handle search suggestions.  It maps xul:browsers to objects
-  // { controller, previousFormHistoryResult }.  See _onMessageGetSuggestions.
+  // { controller, previousFormHistoryResults }.  See _onMessageGetSuggestions.
   _suggestionMap: new WeakMap(),
 
   // Resolved when we finish shutting down.
@@ -193,14 +193,18 @@ export let ContentSearch = {
 
   removeFormHistoryEntry(browser, entry) {
     let browserData = this._suggestionDataForBrowser(browser);
-    if (browserData && browserData.previousFormHistoryResult) {
-      let { previousFormHistoryResult } = browserData;
-      for (let i = 0; i < previousFormHistoryResult.matchCount; i++) {
-        if (previousFormHistoryResult.getValueAt(i) === entry) {
-          previousFormHistoryResult.removeValueAt(i);
-          break;
-        }
-      }
+    if (browserData?.previousFormHistoryResults) {
+      let result = browserData.previousFormHistoryResults.find(
+        e => e.text == entry
+      );
+      lazy.FormHistory.update({
+        op: "remove",
+        fieldname: browserData.controller.formHistoryParam,
+        value: entry,
+        guid: result.guid,
+      }).catch(err =>
+        console.error("Error removing form history entry: ", err)
+      );
     }
   },
 
@@ -297,12 +301,12 @@ export let ContentSearch = {
       return result;
     }
 
-    // Keep the form history result so RemoveFormHistoryEntry can remove entries
+    // Keep the form history results so RemoveFormHistoryEntry can remove entries
     // from it.  Keeping only one result isn't foolproof because the client may
     // try to remove an entry from one set of suggestions after it has requested
     // more but before it's received them.  In that case, the entry may not
     // appear in the new suggestions.  But that should happen rarely.
-    browserData.previousFormHistoryResult = suggestions.formHistoryResult;
+    browserData.previousFormHistoryResults = suggestions.formHistoryResults;
     result = {
       engineName,
       term: suggestions.term,

browser/components/urlbar/UrlbarProviderSearchSuggestions.sys.mjs

@@ -322,8 +322,9 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       return null;
     }
 
-    this._suggestionsController = new lazy.SearchSuggestionController();
-    this._suggestionsController.formHistoryParam = queryContext.formHistoryName;
+    this._suggestionsController = new lazy.SearchSuggestionController(
+      queryContext.formHistoryName
+    );
 
     // If there's a form history entry that equals the search string, the search
     // suggestions controller will include it, and we'll make a result for it.

toolkit/components/search/SearchSuggestionController.sys.mjs

@@ -16,16 +16,6 @@ ChromeUtils.defineModuleGetter(
   "FormHistory",
   "resource://gre/modules/FormHistory.jsm"
 );
-ChromeUtils.defineModuleGetter(
-  lazy,
-  "FormHistoryClient",
-  "resource://gre/modules/FormAutoComplete.jsm"
-);
-ChromeUtils.defineModuleGetter(
-  lazy,
-  "FormAutoCompleteResult",
-  "resource://gre/modules/FormAutoComplete.jsm"
-);
 
 const DEFAULT_FORM_HISTORY_PARAM = "searchbar-history";
 const HTTP_OK = 200;
@@ -156,6 +146,16 @@ var gFirstPartyDomains = new Map();
  *
  */
 export class SearchSuggestionController {
+  /**
+   * Constructor
+   *
+   * @param {string} [formHistoryParam]
+   *   The form history type to use with this controller.
+   */
+  constructor(formHistoryParam = DEFAULT_FORM_HISTORY_PARAM) {
+    this.formHistoryParam = formHistoryParam;
+  }
+
   /**
    * The maximum length of a value to be stored in search history.
    *
@@ -354,24 +354,13 @@ export class SearchSuggestionController {
     // We don't cache these results as we assume that the in-memory SQL cache is
     // good enough in performance.
     let params = {
-      fieldname: this.formHistoryParam || DEFAULT_FORM_HISTORY_PARAM,
+      fieldname: this.formHistoryParam,
     };
 
     if (context.restrictToEngine) {
       params.source = context.engine.name;
     }
 
-    // Needed to keep the legacy autocomplete happy, including removing items.
-    let client = new lazy.FormHistoryClient({
-      formField: null,
-      inputName: params.fieldname,
-    });
-    let formHistoryResult = new lazy.FormAutoCompleteResult(
-      client,
-      [],
-      params.fieldname,
-      context.searchString
-    );
     let results = await lazy.FormHistory.getAutoCompleteResults(
       context.searchString,
       params
@@ -379,11 +368,7 @@ export class SearchSuggestionController {
 
     context.awaitingLocalResults = false;
 
-    formHistoryResult.entries = results;
-    return {
-      formHistoryResult,
-      result: results.map(r => r.text),
-    };
+    return { localResults: results };
   }
 
   /**
@@ -641,7 +626,6 @@ export class SearchSuggestionController {
       term: context.searchString,
       remote: [],
       local: [],
-      formHistoryResult: null,
     };
 
     for (let resultData of suggestResults) {
@@ -651,14 +635,11 @@ export class SearchSuggestionController {
           "SearchSuggestionController found an unexpected string value: " +
             resultData
         );
-      } else if (resultData.formHistoryResult) {
-        // Local results have a formHistoryResult property.
-        results.formHistoryResult = resultData.formHistoryResult;
-        if (resultData.result) {
-          results.local = resultData.result.map(
-            s => new SearchSuggestionEntry(s)
-          );
-        }
+      } else if (resultData.localResults) {
+        results.formHistoryResults = resultData.localResults;
+        results.local = resultData.localResults.map(
+          s => new SearchSuggestionEntry(s.text)
+        );
       } else if (resultData.result) {
         // Remote result
         let richSuggestionData = this.#getRichSuggestionData(resultData.result);

toolkit/components/search/SearchSuggestions.sys.mjs

@@ -9,6 +9,16 @@ ChromeUtils.defineESModuleGetters(lazy, {
   SearchSuggestionController:
     "resource://gre/modules/SearchSuggestionController.sys.mjs",
 });
+ChromeUtils.defineModuleGetter(
+  lazy,
+  "FormHistoryClient",
+  "resource://gre/modules/FormAutoComplete.jsm"
+);
+ChromeUtils.defineModuleGetter(
+  lazy,
+  "FormAutoCompleteResult",
+  "resource://gre/modules/FormAutoComplete.jsm"
+);
 
 /**
  * SuggestAutoComplete is a base class that implements nsIAutoCompleteSearch
@@ -80,11 +90,6 @@ class SuggestAutoComplete {
    *   results are ready.
    */
   startSearch(searchString, searchParam, previousResult, listener) {
-    // Don't reuse a previous form history result when it no longer applies.
-    if (!previousResult) {
-      this.#formHistoryResult = null;
-    }
-
     var formHistorySearchParam = searchParam.split("|")[0];
 
     // Receive the information about the privacy mode of the window to which
@@ -134,7 +139,6 @@ class SuggestAutoComplete {
   }
 
   #suggestionController;
-  #formHistoryResult;
 
   /**
    * Maximum number of history items displayed. This is capped at 7
@@ -188,8 +192,22 @@ class SuggestAutoComplete {
       finalResults = finalResults.concat(nonTailEntries.map(e => e.value));
     }
 
+    // Bug 1822297: This re-uses the wrappers from Satchel, to avoid re-writing
+    // our own nsIAutoCompleteSimpleResult implementation for now. However,
+    // we should do that at some stage to remove the dependency on satchel.
+    let client = new lazy.FormHistoryClient({
+      formField: null,
+      inputName: this.#suggestionController.formHistoryParam,
+    });
+    let formHistoryResult = new lazy.FormAutoCompleteResult(
+      client,
+      results.formHistoryResults,
+      this.#suggestionController.formHistoryParam,
+      searchString
+    );
+
     // Notify the FE of our new results
-    this.onResultsReady(results.term, finalResults, results.formHistoryResult);
+    this.onResultsReady(results.term, finalResults, formHistoryResult);
   }
 
   QueryInterface = ChromeUtils.generateQI([