Bug 1510281 - Use a private and isolated context for search suggestions. r=mkaply
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 27 Nov 2018 17:18:28 +0000
changeset 507611 093960021b8b20b3c1963c9d1b1fc6809f60fd6b
parent 507610 d476f054342f039445afddff474f4c8b4d9edea2
child 507612 9ef08b4eeea6096d4687a5f957da9f35909d461d
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkaply
bugs1510281
milestone65.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 1510281 - Use a private and isolated context for search suggestions. r=mkaply Differential Revision: https://phabricator.services.mozilla.com/D13082
browser/modules/ContentSearch.jsm
toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
toolkit/components/search/SearchSuggestionController.jsm
toolkit/components/search/nsSearchSuggestions.js
toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
toolkit/components/search/tests/xpcshell/test_searchSuggest.js
toolkit/components/search/tests/xpcshell/test_searchSuggest_cookies.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/browser/modules/ContentSearch.jsm
+++ b/browser/modules/ContentSearch.jsm
@@ -268,21 +268,20 @@ var ContentSearch = {
       throw new Error("Unknown engine name: " + engineName);
     }
 
     let browserData = this._suggestionDataForBrowser(browser, true);
     let { controller } = browserData;
     let ok = SearchSuggestionController.engineOffersSuggestions(engine);
     controller.maxLocalResults = ok ? MAX_LOCAL_SUGGESTIONS : MAX_SUGGESTIONS;
     controller.maxRemoteResults = ok ? MAX_SUGGESTIONS : 0;
-    let priv = PrivateBrowsingUtils.isBrowserPrivate(browser);
     // fetch() rejects its promise if there's a pending request, but since we
     // process our event queue serially, there's never a pending request.
     this._currentSuggestion = { controller, target: browser };
-    let suggestions = await controller.fetch(searchString, priv, engine);
+    let suggestions = await controller.fetch(searchString, engine);
     this._currentSuggestion = null;
 
     // suggestions will be null if the request was cancelled
     let result = {};
     if (!suggestions) {
       return result;
     }
 
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
@@ -132,17 +132,17 @@ class SuggestionsFetch {
               maxRemoteResults,
               userContextId) {
     this._controller = new SearchSuggestionController();
     this._controller.maxLocalResults = maxLocalResults;
     this._controller.maxRemoteResults = maxRemoteResults;
     this._engine = engine;
     this._suggestions = [];
     this._success = false;
-    this._promise = this._controller.fetch(searchString, inPrivateContext, engine, userContextId).then(results => {
+    this._promise = this._controller.fetch(searchString, engine, userContextId).then(results => {
       this._success = true;
       if (results) {
         this._suggestions.push(
           ...results.local.map(r => ({ suggestion: r, historical: true })),
           ...results.remote.map(r => ({ suggestion: r, historical: false }))
         );
       }
     }).catch(err => {
--- a/toolkit/components/search/SearchSuggestionController.jsm
+++ b/toolkit/components/search/SearchSuggestionController.jsm
@@ -41,16 +41,18 @@ Services.prefs.addObserver(BROWSER_SUGGE
  *                                returned by the search method instead if you prefer.
  * @constructor
  */
 function SearchSuggestionController(callback = null) {
   this._callback = callback;
 }
 
 this.SearchSuggestionController.prototype = {
+  FIRST_PARTY_DOMAIN: "search.suggestions.8c845959-a33d-4787-953c-5d55a0afd56e.mozilla",
+
   /**
    * The maximum number of local form history results to return. This limit is
    * only enforced if remote results are also returned.
    */
   maxLocalResults: 5,
 
   /**
    * The maximum number of remote search engine results to return.
@@ -94,35 +96,34 @@ this.SearchSuggestionController.prototyp
 
   // Public methods
 
   /**
    * Fetch search suggestions from all of the providers. Fetches in progress will be stopped and
    * results from them will not be provided.
    *
    * @param {string} searchTerm - the term to provide suggestions for
-   * @param {bool} privateMode - whether the request is being made in the context of private browsing
    * @param {nsISearchEngine} engine - search engine for the suggestions.
    * @param {int} userContextId - the userContextId of the selected tab.
+   * @param {bool} privateMode - whether the request should be made private.
+   *        Note that usually we want a private context, even in a non-private
+   *        window, because we don't want to store cookies and offline data.
    *
    * @return {Promise} resolving to an object containing results or null.
    */
-  fetch(searchTerm, privateMode, engine, userContextId) {
+  fetch(searchTerm, engine, userContextId = 0, privateMode = true) {
     // There is no smart filtering from previous results here (as there is when looking through
     // history/form data) because the result set returned by the server is different for every typed
     // value - e.g. "ocean breathes" does not return a subset of the results returned for "ocean".
 
     this.stop();
 
     if (!Services.search.isInitialized) {
       throw new Error("Search not initialized yet (how did you get here?)");
     }
-    if (typeof privateMode === "undefined") {
-      throw new Error("The privateMode argument is required to avoid unintentional privacy leaks");
-    }
     if (!(engine instanceof Ci.nsISearchEngine)) {
       throw new Error("Invalid search engine");
     }
     if (!this.maxLocalResults && !this.maxRemoteResults) {
       throw new Error("Zero results expected, what are you trying to do?");
     }
     if (this.maxLocalResults < 0 || this.maxRemoteResults < 0) {
       throw new Error("Number of requested results must be positive");
@@ -226,18 +227,22 @@ this.SearchSuggestionController.prototyp
   _fetchRemote(searchTerm, engine, privateMode, userContextId) {
     let deferredResponse = PromiseUtils.defer();
     this._request = new XMLHttpRequest();
     let submission = engine.getSubmission(searchTerm,
                                           SEARCH_RESPONSE_SUGGESTION_JSON);
     let method = (submission.postData ? "POST" : "GET");
     this._request.open(method, submission.uri.spec, true);
 
-    this._request.setOriginAttributes({userContextId,
-                                       privateBrowsingId: privateMode ? 1 : 0});
+    this._request.setOriginAttributes({
+      userContextId,
+      privateBrowsingId: privateMode ? 1 : 0,
+      // Use a unique first-party domain to isolate the suggestions cookies.
+      firstPartyDomain: this.FIRST_PARTY_DOMAIN,
+    });
 
     this._request.mozBackgroundRequest = true; // suppress dialogs and fail silently
 
     this._request.addEventListener("load", this._onRemoteLoaded.bind(this, deferredResponse));
     this._request.addEventListener("error", (evt) => deferredResponse.resolve("HTTP error"));
     // Reject for an abort assuming it's always from .stop() in which case we shouldn't return local
     // or remote results for existing searches.
     this._request.addEventListener("abort", (evt) => deferredResponse.reject("HTTP request aborted"));
--- a/toolkit/components/search/nsSearchSuggestions.js
+++ b/toolkit/components/search/nsSearchSuggestions.js
@@ -114,48 +114,38 @@ SuggestAutoComplete.prototype = {
    */
   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
-    // this search box belongs.  The front-end's search.xml bindings passes this
-    // information in the searchParam parameter.  The alternative would have
-    // been to modify nsIAutoCompleteSearch to add an argument to startSearch
-    // and patch all of autocomplete to be aware of this, but the searchParam
-    // argument is already an opaque argument, so this solution is hopefully
-    // less hackish (although still gross.)
-    var privacyMode = (searchParam.split("|")[1] == "private");
-
     // Start search immediately if possible, otherwise once the search
     // service is initialized
     if (Services.search.isInitialized) {
-      this._triggerSearch(searchString, formHistorySearchParam, listener, privacyMode);
+      this._triggerSearch(searchString, formHistorySearchParam, listener);
       return;
     }
 
     Services.search.init(aResult => {
       if (!Components.isSuccessCode(aResult)) {
         Cu.reportError("Could not initialize search service, bailing out: " + aResult);
         return;
       }
-      this._triggerSearch(searchString, formHistorySearchParam, listener, privacyMode);
+      this._triggerSearch(searchString, formHistorySearchParam, listener);
     });
   },
 
   /**
    * Actual implementation of search.
    */
-  _triggerSearch(searchString, searchParam, listener, privacyMode) {
+  _triggerSearch(searchString, searchParam, listener) {
     this._listener = listener;
     this._suggestionController.fetch(searchString,
-                                     privacyMode,
                                      Services.search.defaultEngine);
   },
 
   /**
    * Ends the search result gathering process. Part of nsIAutoCompleteSearch
    * implementation.
    */
   stopSearch() {
--- a/toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
+++ b/toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
@@ -16,17 +16,20 @@ function handleRequest(request, response
     let result = [query, completions];
     response.write(JSON.stringify(result));
     return result;
   }
 
   response.setStatusLine(request.httpVersion, 200, "OK");
 
   let q = request.method == "GET" ? query.q : undefined;
-  if (q == "no remote" || q == "no results") {
+  if (q == "cookie") {
+    response.setHeader("Set-Cookie", "cookie=1");
+    writeSuggestions(q);
+  } else if (q == "no remote" || q == "no results") {
     writeSuggestions(q);
   } else if (q == "Query Mismatch") {
     writeSuggestions("This is an incorrect query string", ["some result"]);
   } else if (q == "Query Case Mismatch") {
     writeSuggestions(q.toUpperCase(), [q]);
   } else if (q == "") {
     writeSuggestions("", ["The server should never be sent an empty query"]);
   } else if (q && q.startsWith("mo")) {
--- a/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
+++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
@@ -91,162 +91,162 @@ add_task(async function simple_no_result
   await new Promise(resolve => {
     let controller = new SearchSuggestionController((result) => {
       Assert.equal(result.term, "no remote");
       Assert.equal(result.local.length, 0);
       Assert.equal(result.remote.length, 0);
       resolve();
     });
 
-    controller.fetch("no remote", false, getEngine);
+    controller.fetch("no remote", getEngine);
   });
 });
 
 add_task(async function simple_no_result_callback_and_promise() {
   // Make sure both the callback and promise get results
   let deferred = PromiseUtils.defer();
   let controller = new SearchSuggestionController((result) => {
     Assert.equal(result.term, "no results");
     Assert.equal(result.local.length, 0);
     Assert.equal(result.remote.length, 0);
     deferred.resolve();
   });
 
-  let result = await controller.fetch("no results", false, getEngine);
+  let result = await controller.fetch("no results", getEngine);
   Assert.equal(result.term, "no results");
   Assert.equal(result.local.length, 0);
   Assert.equal(result.remote.length, 0);
 
   await deferred.promise;
 });
 
 add_task(async function simple_no_result_promise() {
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("no remote", false, getEngine);
+  let result = await controller.fetch("no remote", getEngine);
   Assert.equal(result.term, "no remote");
   Assert.equal(result.local.length, 0);
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function simple_remote_no_local_result() {
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, getEngine);
+  let result = await controller.fetch("mo", getEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 0);
   Assert.equal(result.remote.length, 3);
   Assert.equal(result.remote[0], "Mozilla");
   Assert.equal(result.remote[1], "modern");
   Assert.equal(result.remote[2], "mom");
 });
 
 add_task(async function simple_remote_no_local_result_alternative_type() {
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, alternateJSONEngine);
+  let result = await controller.fetch("mo", alternateJSONEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 0);
   Assert.equal(result.remote.length, 3);
   Assert.equal(result.remote[0], "Mozilla");
   Assert.equal(result.remote[1], "modern");
   Assert.equal(result.remote[2], "mom");
 });
 
 add_task(async function remote_term_case_mismatch() {
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("Query Case Mismatch", false, getEngine);
+  let result = await controller.fetch("Query Case Mismatch", getEngine);
   Assert.equal(result.term, "Query Case Mismatch");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "Query Case Mismatch");
 });
 
 add_task(async function simple_local_no_remote_result() {
   await updateSearchHistory("bump", "no remote entries");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("no remote", false, getEngine);
+  let result = await controller.fetch("no remote", getEngine);
   Assert.equal(result.term, "no remote");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "no remote entries");
   Assert.equal(result.remote.length, 0);
 
   await updateSearchHistory("remove", "no remote entries");
 });
 
 add_task(async function simple_non_ascii() {
   await updateSearchHistory("bump", "I ❤️ XUL");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("I ❤️", false, getEngine);
+  let result = await controller.fetch("I ❤️", getEngine);
   Assert.equal(result.term, "I ❤️");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "I ❤️ XUL");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "I ❤️ Mozilla");
 });
 
 add_task(async function both_local_remote_result_dedupe() {
   await updateSearchHistory("bump", "Mozilla");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, getEngine);
+  let result = await controller.fetch("mo", getEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "Mozilla");
   Assert.equal(result.remote.length, 2);
   Assert.equal(result.remote[0], "modern");
   Assert.equal(result.remote[1], "mom");
 });
 
 add_task(async function POST_both_local_remote_result_dedupe() {
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, postEngine);
+  let result = await controller.fetch("mo", postEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "Mozilla");
   Assert.equal(result.remote.length, 2);
   Assert.equal(result.remote[0], "modern");
   Assert.equal(result.remote[1], "mom");
 });
 
 add_task(async function both_local_remote_result_dedupe2() {
   await updateSearchHistory("bump", "mom");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, getEngine);
+  let result = await controller.fetch("mo", getEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 2);
   Assert.equal(result.local[0], "mom");
   Assert.equal(result.local[1], "Mozilla");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "modern");
 });
 
 add_task(async function both_local_remote_result_dedupe3() {
   // All of the server entries also exist locally
   await updateSearchHistory("bump", "modern");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("mo", false, getEngine);
+  let result = await controller.fetch("mo", getEngine);
   Assert.equal(result.term, "mo");
   Assert.equal(result.local.length, 3);
   Assert.equal(result.local[0], "modern");
   Assert.equal(result.local[1], "mom");
   Assert.equal(result.local[2], "Mozilla");
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function fetch_twice_in_a_row() {
   // Two entries since the first will match the first fetch but not the second.
   await updateSearchHistory("bump", "delay local");
   await updateSearchHistory("bump", "delayed local");
 
   let controller = new SearchSuggestionController();
-  let resultPromise1 = controller.fetch("delay", false, getEngine);
+  let resultPromise1 = controller.fetch("delay", getEngine);
 
   // A second fetch while the server is still waiting to return results leads to an abort.
-  let resultPromise2 = controller.fetch("delayed ", false, getEngine);
+  let resultPromise2 = controller.fetch("delayed ", getEngine);
   await resultPromise1.then((results) => Assert.equal(null, results));
 
   let result = await resultPromise2;
   Assert.equal(result.term, "delayed ");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "delayed local");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "delayed ");
@@ -254,186 +254,186 @@ add_task(async function fetch_twice_in_a
 
 add_task(async function fetch_twice_subset_reuse_formHistoryResult() {
   // This tests if we mess up re-using the cached form history result.
   // Two entries since the first will match the first fetch but not the second.
   await updateSearchHistory("bump", "delay local");
   await updateSearchHistory("bump", "delayed local");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("delay", false, getEngine);
+  let result = await controller.fetch("delay", getEngine);
   Assert.equal(result.term, "delay");
   Assert.equal(result.local.length, 2);
   Assert.equal(result.local[0], "delay local");
   Assert.equal(result.local[1], "delayed local");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "delay");
 
   // Remove the entry from the DB but it should remain in the cached formHistoryResult.
   await updateSearchHistory("remove", "delayed local");
 
-  let result2 = await controller.fetch("delayed ", false, getEngine);
+  let result2 = await controller.fetch("delayed ", getEngine);
   Assert.equal(result2.term, "delayed ");
   Assert.equal(result2.local.length, 1);
   Assert.equal(result2.local[0], "delayed local");
   Assert.equal(result2.remote.length, 1);
   Assert.equal(result2.remote[0], "delayed ");
 });
 
 add_task(async function both_identical_with_more_than_max_results() {
   // Add letters A through Z to form history which will match the server
   for (let charCode = "A".charCodeAt(); charCode <= "Z".charCodeAt(); charCode++) {
     await updateSearchHistory("bump", "letter " + String.fromCharCode(charCode));
   }
 
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 7;
   controller.maxRemoteResults = 10;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 7);
   for (let i = 0; i < controller.maxLocalResults; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.local.length + result.remote.length, 10);
   for (let i = 0; i < result.remote.length; i++) {
     Assert.equal(result.remote[i],
                  "letter " + String.fromCharCode("A".charCodeAt() + controller.maxLocalResults + i));
   }
 });
 
 add_task(async function noremote_maxLocal() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 2; // (should be ignored because no remote results)
   controller.maxRemoteResults = 0;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 26);
   for (let i = 0; i < result.local.length; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function someremote_maxLocal() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 2;
   controller.maxRemoteResults = 4;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 2);
   for (let i = 0; i < result.local.length; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.remote.length, 2);
   // "A" and "B" will have been de-duped, start at C for remote results
   for (let i = 0; i < result.remote.length; i++) {
     Assert.equal(result.remote[i], "letter " + String.fromCharCode("C".charCodeAt() + i));
   }
 });
 
 add_task(async function one_of_each() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 2;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "letter A");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "letter B");
 });
 
 add_task(async function local_result_returned_remote_result_disabled() {
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 1;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 26);
   for (let i = 0; i < 26; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.remote.length, 0);
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
 });
 
 add_task(async function local_result_returned_remote_result_disabled_after_creation_of_controller() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 1;
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 26);
   for (let i = 0; i < 26; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.remote.length, 0);
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
 });
 
 add_task(async function one_of_each_disabled_before_creation_enabled_after_creation_of_controller() {
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 2;
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "letter A");
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "letter B");
 });
 
 add_task(async function reset_suggestions_pref() {
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
 });
 
 add_task(async function one_local_zero_remote() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 0;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 26);
   for (let i = 0; i < 26; i++) {
     Assert.equal(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
   }
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function zero_local_one_remote() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 0;
   controller.maxRemoteResults = 1;
-  let result = await controller.fetch("letter ", false, getEngine);
+  let result = await controller.fetch("letter ", getEngine);
   Assert.equal(result.term, "letter ");
   Assert.equal(result.local.length, 0);
   Assert.equal(result.remote.length, 1);
   Assert.equal(result.remote[0], "letter A");
 });
 
 add_task(async function stop_search() {
   let controller = new SearchSuggestionController((result) => {
     do_throw("The callback shouldn't be called after stop()");
   });
-  let resultPromise = controller.fetch("mo", false, getEngine);
+  let resultPromise = controller.fetch("mo", getEngine);
   controller.stop();
   await resultPromise.then((result) => {
     Assert.equal(null, result);
   });
 });
 
 add_task(async function empty_searchTerm() {
   // Empty searches don't go to the server but still get form history.
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("", false, getEngine);
+  let result = await controller.fetch("", getEngine);
   Assert.equal(result.term, "");
   Assert.ok(result.local.length > 0);
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function slow_timeout() {
   let d = PromiseUtils.defer();
   function check_result(result) {
@@ -445,25 +445,25 @@ add_task(async function slow_timeout() {
   await updateSearchHistory("bump", "slow local result");
 
   let controller = new SearchSuggestionController();
   setTimeout(function check_timeout() {
     // The HTTP response takes 10 seconds so check that we already have results after 2 seconds.
     check_result(result);
     d.resolve();
   }, 2000);
-  let result = await controller.fetch("slow ", false, getEngine);
+  let result = await controller.fetch("slow ", getEngine);
   check_result(result);
   await d.promise;
 });
 
 add_task(async function slow_stop() {
   let d = PromiseUtils.defer();
   let controller = new SearchSuggestionController();
-  let resultPromise = controller.fetch("slow ", false, getEngine);
+  let resultPromise = controller.fetch("slow ", getEngine);
   setTimeout(function check_timeout() {
     // The HTTP response takes 10 seconds but we timeout in less than a second so just use 0.
     controller.stop();
     d.resolve();
   }, 0);
   await resultPromise.then((result) => {
     Assert.equal(null, result);
   });
@@ -473,105 +473,98 @@ add_task(async function slow_stop() {
 
 
 // Error handling
 
 add_task(async function remote_term_mismatch() {
   await updateSearchHistory("bump", "Query Mismatch Entry");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("Query Mismatch", false, getEngine);
+  let result = await controller.fetch("Query Mismatch", getEngine);
   Assert.equal(result.term, "Query Mismatch");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "Query Mismatch Entry");
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function http_404() {
   await updateSearchHistory("bump", "HTTP 404 Entry");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("HTTP 404", false, getEngine);
+  let result = await controller.fetch("HTTP 404", getEngine);
   Assert.equal(result.term, "HTTP 404");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "HTTP 404 Entry");
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function http_500() {
   await updateSearchHistory("bump", "HTTP 500 Entry");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("HTTP 500", false, getEngine);
+  let result = await controller.fetch("HTTP 500", getEngine);
   Assert.equal(result.term, "HTTP 500");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "HTTP 500 Entry");
   Assert.equal(result.remote.length, 0);
 });
 
 add_task(async function unresolvable_server() {
   await updateSearchHistory("bump", "Unresolvable Server Entry");
 
   let controller = new SearchSuggestionController();
-  let result = await controller.fetch("Unresolvable Server", false, unresolvableEngine);
+  let result = await controller.fetch("Unresolvable Server", unresolvableEngine);
   Assert.equal(result.term, "Unresolvable Server");
   Assert.equal(result.local.length, 1);
   Assert.equal(result.local[0], "Unresolvable Server Entry");
   Assert.equal(result.remote.length, 0);
 });
 
 
 // Exception handling
 
-add_task(async function missing_pb() {
-  Assert.throws(() => {
-    let controller = new SearchSuggestionController();
-    controller.fetch("No privacy");
-  }, /priva/i);
-});
-
 add_task(async function missing_engine() {
   Assert.throws(() => {
     let controller = new SearchSuggestionController();
-    controller.fetch("No engine", false);
+    controller.fetch("No engine");
   }, /engine/i);
 });
 
 add_task(async function invalid_engine() {
   Assert.throws(() => {
     let controller = new SearchSuggestionController();
-    controller.fetch("invalid engine", false, {});
+    controller.fetch("invalid engine", {});
   }, /engine/i);
 });
 
 add_task(async function no_results_requested() {
   Assert.throws(() => {
     let controller = new SearchSuggestionController();
     controller.maxLocalResults = 0;
     controller.maxRemoteResults = 0;
-    controller.fetch("No results requested", false, getEngine);
+    controller.fetch("No results requested", getEngine);
   }, /result/i);
 });
 
 add_task(async function minus_one_results_requested() {
   Assert.throws(() => {
     let controller = new SearchSuggestionController();
     controller.maxLocalResults = -1;
-    controller.fetch("-1 results requested", false, getEngine);
+    controller.fetch("-1 results requested", getEngine);
   }, /result/i);
 });
 
 add_task(async function test_userContextId() {
   let controller = new SearchSuggestionController();
-  controller._fetchRemote = function(searchTerm, engine, privateMode, userContextId) {
+  controller._fetchRemote = function(searchTerm, engine, userContextId, privateMode) {
     Assert.equal(userContextId, 1);
     return PromiseUtils.defer();
   };
 
-  controller.fetch("test", false, getEngine, 1);
+  controller.fetch("test", getEngine, 1);
 });
 
 // Helpers
 
 function updateSearchHistory(operation, value) {
   return new Promise((resolve, reject) => {
     FormHistory.update({
                          op: operation,
new file mode 100644
--- /dev/null
+++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest_cookies.js
@@ -0,0 +1,91 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable mozilla/no-arbitrary-setTimeout */
+
+/**
+ * Testing search suggestions from SearchSuggestionController.jsm.
+ */
+
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/FormHistory.jsm");
+ChromeUtils.import("resource://gre/modules/SearchSuggestionController.jsm");
+ChromeUtils.import("resource://gre/modules/Timer.jsm");
+ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm");
+
+// We must make sure the FormHistoryStartup component is
+// initialized in order for it to respond to FormHistory
+// requests from nsFormAutoComplete.js.
+var formHistoryStartup = Cc["@mozilla.org/satchel/form-history-startup;1"].
+                         getService(Ci.nsIObserver);
+formHistoryStartup.observe(null, "profile-after-change", null);
+
+var httpServer = new HttpServer();
+var getEngine;
+
+add_task(async function setup() {
+  Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
+
+  let server = useHttpServer();
+  server.registerContentType("sjs", "sjs");
+
+  registerCleanupFunction(async function cleanup() {
+    Services.prefs.clearUserPref("browser.search.suggest.enabled");
+  });
+
+  [getEngine] = await addTestEngines([
+    {
+      name: "GET suggestion engine",
+      xmlFileName: "engineMaker.sjs?" + JSON.stringify({
+        baseURL: gDataUrl,
+        name: "GET suggestion engine",
+        method: "GET",
+      }),
+    },
+  ]);
+});
+
+add_task(async function test_private() {
+  await new Promise(resolve => {
+    let controller = new SearchSuggestionController((result) => {
+      Assert.equal(result.term, "cookie");
+      Assert.equal(result.local.length, 0);
+      Assert.equal(result.remote.length, 0);
+      resolve();
+    });
+    controller.fetch("cookie", getEngine);
+  });
+  info("Enumerating cookies");
+  let enumerator = Services.cookies.enumerator;
+  let cookies = [];
+  for (let cookie of enumerator) {
+    info("Cookie:" + cookie.rawHost + " " + JSON.stringify(cookie.originAttributes));
+    cookies.push(cookie);
+    break;
+  }
+  Assert.equal(cookies.length, 0, "Should not find any cookie");
+});
+
+add_task(async function test_nonprivate() {
+  let controller;
+  await new Promise(resolve => {
+     controller = new SearchSuggestionController((result) => {
+      Assert.equal(result.term, "cookie");
+      Assert.equal(result.local.length, 0);
+      Assert.equal(result.remote.length, 0);
+      resolve();
+    });
+    controller.fetch("cookie", getEngine, 0, false);
+  });
+  info("Enumerating cookies");
+  let enumerator = Services.cookies.enumerator;
+  let cookies = [];
+  for (let cookie of enumerator) {
+    info("Cookie:" + cookie.rawHost + " " + JSON.stringify(cookie.originAttributes));
+    cookies.push(cookie);
+    break;
+  }
+  Assert.equal(cookies.length, 1, "Should find one cookie");
+  Assert.equal(cookies[0].originAttributes.firstPartyDomain,
+               controller.FIRST_PARTY_DOMAIN, "Check firstPartyDomain");
+});
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -61,16 +61,17 @@ support-files = data/search_ignorelist.j
 [test_defaultEngine.js]
 [test_notifications.js]
 [test_parseSubmissionURL.js]
 [test_SearchStaticData.js]
 [test_addEngine_callback.js]
 [test_multipleIcons.js]
 [test_resultDomain.js]
 [test_searchSuggest.js]
+[test_searchSuggest_cookies.js]
 [test_async.js]
 [test_async_addon.js]
 tags = addons
 [test_async_addon_no_override.js]
 tags = addons
 [test_async_distribution.js]
 [test_async_disthidden.js]
 [test_async_profile_engine.js]