Bug 1048198 - Searchbar Search Suggestions is no longer able to disable. r=mattn, a=sledru
authorJared Wein <jwein@mozilla.com>
Tue, 12 Aug 2014 11:30:16 -0400
changeset 217661 02feec56dfae4902089820146683d60d267a3c68
parent 217660 62505420ae112a5df004f2ec8fb8df5e1fce659f
child 217662 e03d0042d2e3e6887b3bde5cbc19dccde0191b6d
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattn, sledru
bugs1048198
milestone33.0a2
Bug 1048198 - Searchbar Search Suggestions is no longer able to disable. r=mattn, a=sledru
toolkit/components/search/SearchSuggestionController.jsm
toolkit/components/search/nsSearchSuggestions.js
toolkit/components/search/tests/xpcshell/test_searchSuggest.js
--- a/toolkit/components/search/SearchSuggestionController.jsm
+++ b/toolkit/components/search/SearchSuggestionController.jsm
@@ -12,16 +12,26 @@ Cu.import("resource://gre/modules/XPCOMU
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NS_ASSERT", "resource://gre/modules/debug.js");
 
 const SEARCH_RESPONSE_SUGGESTION_JSON = "application/x-suggestions+json";
 const DEFAULT_FORM_HISTORY_PARAM      = "searchbar-history";
 const HTTP_OK            = 200;
 const REMOTE_TIMEOUT     = 500; // maximum time (ms) to wait before giving up on a remote suggestions
+const BROWSER_SUGGEST_PREF = "browser.search.suggest.enabled";
+
+/**
+ * Remote search suggestions will be shown if gRemoteSuggestionsEnabled
+ * is true. Global because only one pref observer is needed for all instances.
+ */
+let gRemoteSuggestionsEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
+Services.prefs.addObserver(BROWSER_SUGGEST_PREF, function(aSubject, aTopic, aData) {
+  gRemoteSuggestionsEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
+}, false);
 
 /**
  * SearchSuggestionController.jsm exists as a helper module to allow multiple consumers to request and display
  * search suggestions from a given engine, regardless of the base implementation. Much of this
  * code was originally in nsSearchSuggestions.js until it was refactored to separate it from the
  * nsIAutoCompleteSearch dependency.
  * One instance of SearchSuggestionController should be used per field since form history results are cached.
  */
@@ -119,17 +129,17 @@ this.SearchSuggestionController.prototyp
       throw new Error("Number of requested results must be positive");
     }
 
     // Array of promises to resolve before returning results.
     let promises = [];
     this._searchString = searchTerm;
 
     // Remote results
-    if (searchTerm && this.maxRemoteResults &&
+    if (searchTerm && gRemoteSuggestionsEnabled && this.maxRemoteResults &&
         engine.supportsResponseType(SEARCH_RESPONSE_SUGGESTION_JSON)) {
       this._deferredRemoteResult = this._fetchRemote(searchTerm, engine, privateMode);
       promises.push(this._deferredRemoteResult.promise);
     }
 
     // Local results from form history
     if (this.maxLocalResults) {
       let deferredHistoryResult = this._fetchFormHistory(searchTerm);
--- a/toolkit/components/search/nsSearchSuggestions.js
+++ b/toolkit/components/search/nsSearchSuggestions.js
@@ -1,16 +1,12 @@
 /* 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/. */
 
-const BROWSER_SUGGEST_PREF = "browser.search.suggest.enabled";
-const XPCOM_SHUTDOWN_TOPIC              = "xpcom-shutdown";
-const NS_PREFBRANCH_PREFCHANGE_TOPIC_ID = "nsPref:changed";
-
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/nsFormAutoCompleteResult.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SearchSuggestionController",
                                   "resource://gre/modules/SearchSuggestionController.jsm");
 
@@ -23,33 +19,26 @@ XPCOMUtils.defineLazyModuleGetter(this, 
  * @constructor
  */
 function SuggestAutoComplete() {
   this._init();
 }
 SuggestAutoComplete.prototype = {
 
   _init: function() {
-    this._addObservers();
-    this._suggestEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
     this._suggestionController = new SearchSuggestionController(obj => this.onResultsReturned(obj));
   },
 
   get _suggestionLabel() {
     delete this._suggestionLabel;
     let bundle = Services.strings.createBundle("chrome://global/locale/search/search.properties");
     return this._suggestionLabel = bundle.GetStringFromName("suggestion_label");
   },
 
   /**
-   * Search suggestions will be shown if this._suggestEnabled is true.
-   */
-  _suggestEnabled: null,
-
-  /**
    * The object implementing nsIAutoCompleteObserver that we notify when
    * we have found results
    * @private
    */
   _listener: null,
 
   /**
    * Maximum number of history items displayed. This is capped at 7
@@ -173,42 +162,16 @@ SuggestAutoComplete.prototype = {
   /**
    * Ends the search result gathering process. Part of nsIAutoCompleteSearch
    * implementation.
    */
   stopSearch: function() {
     this._suggestionController.stop();
   },
 
-  /**
-   * nsIObserver
-   */
-  observe: function SAC_observe(aSubject, aTopic, aData) {
-    switch (aTopic) {
-      case NS_PREFBRANCH_PREFCHANGE_TOPIC_ID:
-        this._suggestEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
-        break;
-      case XPCOM_SHUTDOWN_TOPIC:
-        this._removeObservers();
-        break;
-    }
-  },
-
-  _addObservers: function SAC_addObservers() {
-    Services.prefs.addObserver(BROWSER_SUGGEST_PREF, this, false);
-
-    Services.obs.addObserver(this, XPCOM_SHUTDOWN_TOPIC, false);
-  },
-
-  _removeObservers: function SAC_removeObservers() {
-    Services.prefs.removeObserver(BROWSER_SUGGEST_PREF, this);
-
-    Services.obs.removeObserver(this, XPCOM_SHUTDOWN_TOPIC);
-  },
-
   // nsISupports
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIAutoCompleteSearch,
                                          Ci.nsIAutoCompleteObserver])
 };
 
 /**
  * SearchSuggestAutoComplete is a service implementation that handles suggest
  * results specific to web searches.
--- a/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
+++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
@@ -273,16 +273,59 @@ add_task(function* one_of_each() {
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
   do_check_eq(result.local.length, 1);
   do_check_eq(result.local[0], "letter A");
   do_check_eq(result.remote.length, 1);
   do_check_eq(result.remote[0], "letter B");
 });
 
+add_task(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 = yield controller.fetch("letter ", false, getEngine);
+  do_check_eq(result.term, "letter ");
+  do_check_eq(result.local.length, 1);
+  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.remote.length, 0);
+  Services.prefs.clearUserPref("browser.search.suggest.enabled");
+});
+
+add_task(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 = yield controller.fetch("letter ", false, getEngine);
+  do_check_eq(result.term, "letter ");
+  do_check_eq(result.local.length, 1);
+  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.remote.length, 0);
+});
+
+add_task(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 = 1;
+  Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
+  let result = yield controller.fetch("letter ", false, getEngine);
+  do_check_eq(result.term, "letter ");
+  do_check_eq(result.local.length, 1);
+  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.remote.length, 1);
+  do_check_eq(result.remote[0], "letter B");
+});
+
+add_task(function* clear_suggestions_pref() {
+  Services.prefs.clearUserPref("browser.search.suggest.enabled");
+});
+
 add_task(function* one_local_zero_remote() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 0;
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
   do_check_eq(result.local.length, 1);
   do_check_eq(result.local[0], "letter A");