Bug 1654862 - Use a property bag instead of abusing searchParams. r=MattN
authorDrew Willcoxon <adw@mozilla.com>
Wed, 19 Aug 2020 04:41:08 +0000
changeset 545249 1ae21b324796c33236b5d1563739a8a31914ed62
parent 545248 c878b0fb16c7b10587a50d79ae5fd81d94b85e9a
child 545250 a628999fe8c77485dc806844debadda6fc185720
push id37711
push usernbeleuzu@mozilla.com
push dateWed, 19 Aug 2020 10:01:16 +0000
treeherdermozilla-central@157db696462d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1654862
milestone81.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1654862 - Use a property bag instead of abusing searchParams. r=MattN Depends on D86386 Differential Revision: https://phabricator.services.mozilla.com/D86479
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/components/autocomplete/nsIAutoCompleteSearch.idl
toolkit/components/satchel/FormAutoComplete.jsm
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/nsIFormAutoComplete.idl
toolkit/components/search/SearchSuggestionController.jsm
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -965,17 +965,18 @@ nsresult nsAutoCompleteController::Start
     rv = input->GetUserContextId(&userContextId);
     if (NS_SUCCEEDED(rv) &&
         userContextId != nsIScriptSecurityManager::DEFAULT_USER_CONTEXT_ID) {
       searchParam.AppendLiteral(" user-context-id:");
       searchParam.AppendInt(userContextId, 10);
     }
 
     rv = search->StartSearch(mSearchString, searchParam, result,
-                             static_cast<nsIAutoCompleteObserver*>(this));
+                             static_cast<nsIAutoCompleteObserver*>(this),
+                             nullptr);
     if (NS_FAILED(rv)) {
       ++mSearchesFailed;
       MOZ_ASSERT(mSearchesOngoing > 0);
       --mSearchesOngoing;
     }
     // Because of the joy of nested event loops (which can easily happen when
     // some code uses a generator for an asynchronous AutoComplete search),
     // nsIAutoCompleteSearch::StartSearch might cause us to be detached from our
--- a/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl
+++ b/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl
@@ -1,33 +1,37 @@
 /* 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/. */
 
 #include "nsISupports.idl"
 
 interface nsIAutoCompleteResult;
 interface nsIAutoCompleteObserver;
+interface nsIPropertyBag2;
 
 [scriptable, uuid(DE8DB85F-C1DE-4d87-94BA-7844890F91FE)]
 interface nsIAutoCompleteSearch : nsISupports
 {
   /*
    * Search for a given string and notify a listener (either synchronously
    * or asynchronously) of the result
    *
    * @param searchString - The string to search for
    * @param searchParam - An extra parameter
    * @param previousResult - A previous result to use for faster searching
    * @param listener - A listener to notify when the search is complete
+   * @param options An optional set of additional search parameters that may be
+   *        passed to the underlying implementation.
    */
   void startSearch(in AString searchString,
                    in AString searchParam,
                    in nsIAutoCompleteResult previousResult,
-                   in nsIAutoCompleteObserver listener);
+                   in nsIAutoCompleteObserver listener,
+                   [optional] in nsIPropertyBag2 options);
 
   /*
    * Stop all searches that are in progress
    */
   void stopSearch();
 };
 
 [scriptable, uuid(8bd1dbbc-dcce-4007-9afa-b551eb687b61)]
--- a/toolkit/components/satchel/FormAutoComplete.jsm
+++ b/toolkit/components/satchel/FormAutoComplete.jsm
@@ -288,76 +288,85 @@ FormAutoComplete.prototype = {
     }
     dump("FormAutoComplete: " + message + "\n");
     Services.console.logStringMessage("FormAutoComplete: " + message);
   },
 
   /*
    * autoCompleteSearchAsync
    *
-   * aSearchParam    -- The searchParam, each entry is separated by the "\x1F"
-   *                    char. The first entry is the `name` attribute from the
-   *                    form input being autocompleted, while the next entries
-   *                    are in `key=value` form.
+   * aInputName -- |name| or |id| attribute value from the form input being
+   *               autocompleted
    * aUntrimmedSearchString -- current value of the input
    * aField -- HTMLInputElement being autocompleted (may be null if from chrome)
    * aPreviousResult -- previous search result, if any.
    * aDatalistResult -- results from list=datalist for aField.
    * aListener -- nsIFormAutoCompleteObserver that listens for the nsIAutoCompleteResult
    *              that may be returned asynchronously.
+   *  options -- an optional nsIPropertyBag2 containing additional search
+   *             parameters.
    */
   autoCompleteSearchAsync(
-    aSearchParam,
+    aInputName,
     aUntrimmedSearchString,
     aField,
     aPreviousResult,
     aDatalistResult,
-    aListener
+    aListener,
+    aOptions
   ) {
     // Guard against void DOM strings filtering into this code.
-    if (typeof aSearchParam === "object") {
-      aSearchParam = "";
+    if (typeof aInputName === "object") {
+      aInputName = "";
     }
     if (typeof aUntrimmedSearchString === "object") {
       aUntrimmedSearchString = "";
     }
-
-    let searchParams = aSearchParam.split("\x1F");
-    let inputName = searchParams.shift();
     let params = {};
-    for (let p of searchParams) {
-      let [key, val] = p.split("=");
-      params[key] = val;
+    if (aOptions) {
+      try {
+        aOptions.QueryInterface(Ci.nsIPropertyBag2);
+        for (let { name, value } of aOptions.enumerator) {
+          params[name] = value;
+        }
+      } catch (ex) {
+        Cu.reportError("Invalid options object: " + ex);
+      }
     }
 
     let client = new FormHistoryClient({
       formField: aField,
-      inputName,
+      inputName: aInputName,
     });
 
     function maybeNotifyListener(result) {
       if (aListener) {
         aListener.onSearchCompletion(result);
       }
     }
 
     // If we have datalist results, they become our "empty" result.
     let emptyResult =
       aDatalistResult ||
-      new FormAutoCompleteResult(client, [], inputName, aUntrimmedSearchString);
+      new FormAutoCompleteResult(
+        client,
+        [],
+        aInputName,
+        aUntrimmedSearchString
+      );
     if (!this._enabled) {
       maybeNotifyListener(emptyResult);
       return;
     }
 
     // Don't allow form inputs (aField != null) to get results from
     // search bar history.
-    if (inputName == "searchbar-history" && aField) {
+    if (aInputName == "searchbar-history" && aField) {
       this.log(
-        'autoCompleteSearch for input name "' + inputName + '" is denied'
+        'autoCompleteSearch for input name "' + aInputName + '" is denied'
       );
       maybeNotifyListener(emptyResult);
       return;
     }
 
     if (aField && isAutocompleteDisabled(aField)) {
       this.log("autoCompleteSearch not allowed due to autcomplete=off");
       maybeNotifyListener(emptyResult);
@@ -466,17 +475,17 @@ FormAutoComplete.prototype = {
     } else {
       this.log("Creating new autocomplete search result.");
 
       // Start with an empty list.
       let result = aDatalistResult
         ? new FormAutoCompleteResult(
             client,
             [],
-            inputName,
+            aInputName,
             aUntrimmedSearchString
           )
         : emptyResult;
 
       let processEntry = aEntries => {
         if (aField && aField.maxLength > -1) {
           result.entries = aEntries.filter(
             el => el.text.length <= aField.maxLength
@@ -489,17 +498,17 @@ FormAutoComplete.prototype = {
           result = this.mergeResults(result, aDatalistResult);
         }
 
         maybeNotifyListener(result);
       };
 
       this.getAutoCompleteValues(
         client,
-        inputName,
+        aInputName,
         searchString,
         params,
         processEntry
       );
     }
   },
 
   mergeResults(historyResult, datalistResult) {
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -664,17 +664,18 @@ nsFormFillController::GetUserContextId(u
 
 ////////////////////////////////////////////////////////////////////////
 //// nsIAutoCompleteSearch
 
 NS_IMETHODIMP
 nsFormFillController::StartSearch(const nsAString& aSearchString,
                                   const nsAString& aSearchParam,
                                   nsIAutoCompleteResult* aPreviousResult,
-                                  nsIAutoCompleteObserver* aListener) {
+                                  nsIAutoCompleteObserver* aListener,
+                                  nsIPropertyBag2* aOptions) {
   MOZ_LOG(sLogger, LogLevel::Debug, ("StartSearch for %p", mFocusedInput));
 
   nsresult rv;
 
   // If the login manager has indicated it's responsible for this field, let it
   // handle the autocomplete. Otherwise, handle with form history.
   // This method is sometimes called in unit tests and from XUL without a
   // focused node.
@@ -711,17 +712,17 @@ nsFormFillController::StartSearch(const 
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     auto formAutoComplete = GetFormAutoComplete();
     NS_ENSURE_TRUE(formAutoComplete, NS_ERROR_FAILURE);
 
     formAutoComplete->AutoCompleteSearchAsync(aSearchParam, aSearchString,
                                               mFocusedInput, aPreviousResult,
-                                              datalistResult, this);
+                                              datalistResult, this, aOptions);
     mLastFormAutoComplete = formAutoComplete;
   }
 
   return NS_OK;
 }
 
 nsresult nsFormFillController::PerformInputListAutoComplete(
     const nsAString& aSearch, nsIAutoCompleteResult** aResult) {
--- a/toolkit/components/satchel/nsIFormAutoComplete.idl
+++ b/toolkit/components/satchel/nsIFormAutoComplete.idl
@@ -2,30 +2,32 @@
  * 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/. */
 
 
 #include "nsISupports.idl"
 
 interface nsIAutoCompleteResult;
 interface nsIFormAutoCompleteObserver;
+interface nsIPropertyBag2;
 
 webidl HTMLInputElement;
 
 [scriptable, uuid(bfd9b82b-0ab3-4b6b-9e54-aa961ff4b732)]
 interface nsIFormAutoComplete: nsISupports {
     /**
      * Generate results for a form input autocomplete menu asynchronously.
      */
     void autoCompleteSearchAsync(in AString aInputName,
                                  in AString aSearchString,
                                  in HTMLInputElement aField,
                                  in nsIAutoCompleteResult aPreviousResult,
                                  in nsIAutoCompleteResult aDatalistResult,
-                                 in nsIFormAutoCompleteObserver aListener);
+                                 in nsIFormAutoCompleteObserver aListener,
+                                 [optional] in nsIPropertyBag2 options);
 
     /**
      * If a search is in progress, stop it. Otherwise, do nothing. This is used
      * to cancel an existing search, for example, in preparation for a new search.
      */
     void stopAutoCompleteSearch();
 };
 
--- a/toolkit/components/search/SearchSuggestionController.jsm
+++ b/toolkit/components/search/SearchSuggestionController.jsm
@@ -345,24 +345,29 @@ SearchSuggestionController.prototype = {
           }
         },
       };
 
       let formHistory = Cc[
         "@mozilla.org/autocomplete/search;1?name=form-history"
       ].createInstance(Ci.nsIAutoCompleteSearch);
       let params = this.formHistoryParam || DEFAULT_FORM_HISTORY_PARAM;
+      let options = null;
       if (source) {
-        params += "\x1Fsource=" + source;
+        options = Cc["@mozilla.org/hash-property-bag;1"].createInstance(
+          Ci.nsIWritablePropertyBag2
+        );
+        options.setPropertyAsAUTF8String("source", source);
       }
       formHistory.startSearch(
         searchTerm,
         params,
         this._formHistoryResult,
-        acSearchObserver
+        acSearchObserver,
+        options
       );
     });
   },
 
   /**
    * Report bandwidth used by search activities. It only reports when it matches
    * search provider information.
    *