Bug 1492475 - Part 3: The search service init() method should simply return a Promise. r=florian
authorMike de Boer <mdeboer@mozilla.com>
Tue, 16 Oct 2018 15:00:14 +0200
changeset 452865 a947d3cdf078032614edaa491ec3db1d046b55f4
parent 452864 79b2eb2367aab104669bbc75c3b42290f7de1570
child 452866 c1e172dfacad4b14ebdb352bee2fd946716acd59
push id2
push usermdeboer@mozilla.com
push dateTue, 08 Jan 2019 17:01:21 +0000
reviewersflorian
bugs1492475
milestone66.0a1
Bug 1492475 - Part 3: The search service init() method should simply return a Promise. r=florian Differential Revision: https://phabricator.services.mozilla.com/D9277
browser/components/enterprisepolicies/Policies.jsm
browser/components/newtab/lib/TopSitesFeed.jsm
browser/components/search/content/searchbar.js
browser/modules/ContentSearch.jsm
toolkit/components/normandy/lib/NormandyDriver.jsm
toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
toolkit/components/search/nsISearchService.idl
toolkit/components/search/nsSearchService.js
toolkit/components/search/nsSearchSuggestions.js
toolkit/components/utils/ClientEnvironment.jsm
--- a/browser/components/enterprisepolicies/Policies.jsm
+++ b/browser/components/enterprisepolicies/Policies.jsm
@@ -795,17 +795,17 @@ var Policies = {
 
   "SearchEngines": {
     onBeforeUIStartup(manager, param) {
       if (param.PreventInstalls) {
         manager.disallowFeature("installSearchEngine", true);
       }
     },
     onAllWindowsRestored(manager, param) {
-      Services.search.init(async () => {
+      Services.search.init().then(async () => {
         if (param.Remove) {
           // Only rerun if the list of engine names has changed.
           await runOncePerModification("removeSearchEngines",
                                        JSON.stringify(param.Remove),
                                        async function() {
             for (let engineName of param.Remove) {
               let engine = Services.search.getEngineByName(engineName);
               if (engine) {
--- a/browser/components/newtab/lib/TopSitesFeed.jsm
+++ b/browser/components/newtab/lib/TopSitesFeed.jsm
@@ -211,17 +211,17 @@ this.TopSitesFeed = class TopSitesFeed {
     return false;
   }
 
   async getLinksWithDefaults() {
     const numItems = this.store.getState().Prefs.values[ROWS_PREF] * TOP_SITES_MAX_SITES_PER_ROW;
     const searchShortcutsExperiment = this.store.getState().Prefs.values[SEARCH_SHORTCUTS_EXPERIMENT];
     // We must wait for search services to initialize in order to access default
     // search engine properties without triggering a synchronous initialization
-    await new Promise(resolve => Services.search.init(resolve));
+    await Services.search.init();
 
     // Get all frecent sites from history.
     let frecent = [];
     const cache = await this.frecentCache.request({
       // We need to overquery due to the top 5 alexa search + default search possibly being removed
       numItems: numItems + SEARCH_FILTERS.length + 1,
       topsiteFrecency: FRECENCY_THRESHOLD,
     });
--- a/browser/components/search/content/searchbar.js
+++ b/browser/components/search/content/searchbar.js
@@ -116,29 +116,25 @@ class MozSearchbar extends MozXULElement
 
     Services.obs.addObserver(this.observer, "browser-search-engine-modified");
     Services.obs.addObserver(this.observer, "browser-search-service");
 
     this._initialized = true;
 
     (window.delayedStartupPromise || Promise.resolve()).then(() => {
       window.requestIdleCallback(() => {
-        Services.search.init(aStatus => {
+        Services.search.init().then(aStatus => {
           // Bail out if the binding's been destroyed
           if (!this._initialized)
             return;
 
-          if (Components.isSuccessCode(aStatus)) {
-            // Refresh the display (updating icon, etc)
-            this.updateDisplay();
-            BrowserSearch.updateOpenSearchBadge();
-          } else {
-            Cu.reportError("Cannot initialize search service, bailing out: " + aStatus);
-          }
-        });
+          // Refresh the display (updating icon, etc)
+          this.updateDisplay();
+          BrowserSearch.updateOpenSearchBadge();
+        }).catch(status => Cu.reportError("Cannot initialize search service, bailing out: " + status));
       });
     });
 
     // Wait until the popupshowing event to avoid forcing immediate
     // attachment of the search-one-offs binding.
     this.textbox.popup.addEventListener("popupshowing", () => {
       let oneOffButtons = this.textbox.popup.oneOffButtons;
       // Some accessibility tests create their own <searchbar> that doesn't
--- a/browser/modules/ContentSearch.jsm
+++ b/browser/modules/ContentSearch.jsm
@@ -550,14 +550,13 @@ var ContentSearch = {
       if (!(prop in data)) {
         throw new Error("Message data missing required property: " + prop);
       }
     }
   },
 
   _initService() {
     if (!this._initServicePromise) {
-      this._initServicePromise =
-        new Promise(resolve => Services.search.init(resolve));
+      this._initServicePromise = Services.search.init();
     }
     return this._initServicePromise;
   },
 };
--- a/toolkit/components/normandy/lib/NormandyDriver.jsm
+++ b/toolkit/components/normandy/lib/NormandyDriver.jsm
@@ -67,22 +67,19 @@ var NormandyDriver = function(sandboxMan
         syncTotalDevices: null,
         plugins: {},
         doNotTrack: Preferences.get("privacy.donottrackheader.enabled", false),
         distribution: Preferences.get("distribution.id", "default"),
       };
       appinfo.syncTotalDevices = appinfo.syncDesktopDevices + appinfo.syncMobileDevices;
 
       const searchEnginePromise = new Promise(resolve => {
-        Services.search.init(rv => {
-          if (Components.isSuccessCode(rv)) {
-            appinfo.searchEngine = Services.search.defaultEngine.identifier;
-          }
-          resolve();
-        });
+        Services.search.init().then(() => {
+          appinfo.searchEngine = Services.search.defaultEngine.identifier;
+        }).finally(resolve);
       });
 
       const pluginsPromise = (async () => {
         let plugins = await AddonManager.getAddonsByTypes(["plugin"]);
         plugins.forEach(plugin => appinfo.plugins[plugin.name] = {
           name: plugin.name,
           description: plugin.description,
           version: plugin.version,
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
@@ -33,36 +33,29 @@ const SearchAutocompleteProviderInternal
   enginesByAlias: new Map(),
 
   /**
    * {array<{ {nsISearchEngine} engine, {array<string>} tokenAliases }>} Array
    * of engines that have "@" aliases.
    */
   tokenAliasEngines: [],
 
-  initialize() {
-    return new Promise((resolve, reject) => {
-      Services.search.init(async status => {
-        if (!Components.isSuccessCode(status)) {
-          reject(new Error("Unable to initialize search service."));
-        }
+  async initialize() {
+    try {
+      await Services.search.init();
+    } catch (errorCode) {
+      throw new Error("Unable to initialize search service.");
+    }
 
-        try {
-          // The initial loading of the search engines must succeed.
-          await this._refresh();
-
-          Services.obs.addObserver(this, SEARCH_ENGINE_TOPIC, true);
+    // The initial loading of the search engines must succeed.
+    await this._refresh();
 
-          this.initialized = true;
-          resolve();
-        } catch (ex) {
-          reject(ex);
-        }
-      });
-    });
+    Services.obs.addObserver(this, SEARCH_ENGINE_TOPIC, true);
+
+    this.initialized = true;
   },
 
   initialized: false,
 
   observe(subject, topic, data) {
     switch (data) {
       case "engine-added":
       case "engine-changed":
--- a/toolkit/components/search/nsISearchService.idl
+++ b/toolkit/components/search/nsISearchService.idl
@@ -237,23 +237,21 @@ interface nsIBrowserSearchInitObserver :
 };
 
 [scriptable, uuid(0301834b-2630-440e-8b98-db8dc55f34b9)]
 interface nsISearchService : nsISupports
 {
   /**
    * Start asynchronous initialization.
    *
-   * The callback is triggered once initialization is complete, which may be
+   * The promise is resolved once initialization is complete, which may be
    * immediately, if initialization has already been completed by some previous
-   * call to this method. The callback is always invoked asynchronously.
-   *
-   * @param aObserver An optional object observing the end of initialization.
+   * call to this method.
    */
-  void init([optional] in nsIBrowserSearchInitObserver aObserver);
+  Promise init();
 
   /**
    * Determine whether initialization has been completed.
    *
    * Clients of the service can use this attribute to quickly determine whether
    * initialization is complete, and decide to trigger some immediate treatment,
    * to launch asynchronous initialization or to bailout.
    *
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -2707,28 +2707,16 @@ SearchService.prototype = {
     // LOG(warning);
 
     // this._syncInit();
     // if (!Components.isSuccessCode(this._initRV)) {
     //   throw this._initRV;
     // }
   },
 
-  _promiseInitialized() {
-    return new Promise((resolve, reject) => {
-      this.init(retVal => {
-        if (!Components.isSuccessCode(retVal)) {
-          reject(retVal);
-        } else {
-          resolve();
-        }
-      });
-    });
-  },
-
   // Synchronous implementation of the initializer.
   // Used by |_ensureInitialized| as a fallback if initialization is not
   // complete.
   _syncInit: function SRCH_SVC__syncInit() {
     LOG("_syncInit start");
     this._initStarted = true;
 
     let cache = this._readCacheFile();
@@ -2741,17 +2729,21 @@ SearchService.prototype = {
       this._initRV = Cr.NS_ERROR_FAILURE;
       LOG("_syncInit: failure loading engines: " + ex);
     }
     this._addObservers();
 
     gInitialized = true;
     this._cacheFileJSON = null;
 
-    this._initObservers.resolve(this._initRV);
+    if (Components.isSuccessCode(this._initRV)) {
+      this._initObservers.resolve(this._initRV);
+    } else {
+      this._initObservers.reject(this._initRV);
+    }
 
     Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "init-complete");
     Services.telemetry.getHistogramById("SEARCH_SERVICE_INIT_SYNC").add(true);
 
     LOG("_syncInit end");
   },
 
   /**
@@ -2784,17 +2776,21 @@ SearchService.prototype = {
         throw ex;
       }
       this._initRV = Cr.NS_ERROR_FAILURE;
       LOG("_asyncInit: failure loading engines: " + ex);
     }
     this._addObservers();
     gInitialized = true;
     this._cacheFileJSON = null;
-    this._initObservers.resolve(this._initRV);
+    if (Components.isSuccessCode(this._initRV)) {
+      this._initObservers.resolve(this._initRV);
+    } else {
+      this._initObservers.reject(this._initRV);
+    }
     Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "init-complete");
     Services.telemetry.getHistogramById("SEARCH_SERVICE_INIT_SYNC").add(false);
 
     LOG("_asyncInit: Completed _asyncInit");
   },
 
   _metaData: { },
   setGlobalAttr(name, val) {
@@ -3803,77 +3799,65 @@ SearchService.prototype = {
       return this._sortedEngines;
 
     return this._sortedEngines.filter(function(engine) {
                                         return !engine.hidden;
                                       });
   },
 
   // nsISearchService
-  init: function SRCH_SVC_init(observer) {
+  async init() {
     LOG("SearchService.init");
-    let self = this;
-    if (!this._initStarted) {
-      TelemetryStopwatch.start("SEARCH_SERVICE_INIT_MS");
-      this._initStarted = true;
-      (async function task() {
-        try {
-          // Complete initialization by calling asynchronous initializer.
-          await self._asyncInit();
-          TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS");
-        } catch (ex) {
-          if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-            // No need to pursue asynchronous because synchronous fallback was
-            // called and has finished.
-            TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS");
-          } else {
-            self._initObservers.reject(ex);
-            TelemetryStopwatch.cancel("SEARCH_SERVICE_INIT_MS");
-          }
-        }
-      })();
+    if (this._initStarted) {
+      return this._initObservers.promise;
     }
-    if (observer) {
-      const callback = observer.onInitComplete || observer;
-      this._initObservers.promise.then(
-        function onSuccess() {
-          try {
-            callback(self._initRV);
-          } catch (e) {
-            Cu.reportError(e);
-          }
-        },
-        function onError(aReason) {
-          Cu.reportError("Internal error while initializing SearchService: " + aReason);
-          callback(Cr.NS_ERROR_UNEXPECTED);
-        }
-      );
+
+    TelemetryStopwatch.start("SEARCH_SERVICE_INIT_MS");
+    this._initStarted = true;
+    try {
+      // Complete initialization by calling asynchronous initializer.
+      await this._asyncInit();
+      TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS");
+    } catch (ex) {
+      if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
+        // No need to pursue asynchronous because synchronous fallback was
+        // called and has finished.
+        TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS");
+      } else {
+        this._initObservers.reject(ex.result);
+        TelemetryStopwatch.cancel("SEARCH_SERVICE_INIT_MS");
+      }
     }
+
+    if (!Components.isSuccessCode(this._initRV)) {
+      throw this._initRV;
+    }
+    return this._initRV;
   },
 
   get isInitialized() {
     return gInitialized;
   },
 
   async getEngines() {
-    await this._promiseInitialized();
+    await this.init();
     LOG("getEngines: getting all engines");
     var engines = this._getSortedEngines(true);
     return engines;
   },
 
   async getVisibleEngines() {
-    await this._promiseInitialized();
+    await this.init();
     LOG("getVisibleEngines: getting all visible engines");
     var engines = this._getSortedEngines(false);
     return engines;
   },
 
   async getDefaultEngines() {
-    await this._promiseInitialized();
+    await this.init();
     function isDefault(engine) {
       return engine._isDefault;
     }
     var engines = this._sortedEngines.filter(isDefault);
     var engineOrder = {};
     var engineName;
     var i = 1;
 
@@ -3928,17 +3912,17 @@ SearchService.prototype = {
 
       return a.name.localeCompare(b.name);
     }
     engines.sort(compareEngines);
     return engines;
   },
 
   async getEnginesByExtensionID(extensionID) {
-    await this._promiseInitialized();
+    await this.init();
     LOG("getEngines: getting all engines for " + extensionID);
     var engines = this._getSortedEngines(true).filter(function(engine) {
       return engine._extensionID == extensionID;
     });
     return engines;
   },
 
   getEngineByName: function SRCH_SVC_getEngineByName(aEngineName) {
@@ -3969,17 +3953,17 @@ SearchService.prototype = {
         alias,
         description,
         method,
         template,
         extensionID,
       };
     }
 
-    await this._promiseInitialized();
+    await this.init();
     if (!name)
       FAIL("Invalid name passed to addEngineWithDetails!");
     if (!params.template)
       FAIL("Invalid template passed to addEngineWithDetails!");
     let existingEngine = this._engines[name];
     if (existingEngine) {
       if (params.extensionID &&
           existingEngine._loadPath.startsWith(`jar:[profile]/extensions/${params.extensionID}`)) {
@@ -4032,17 +4016,17 @@ SearchService.prototype = {
       queryCharset: searchProvider.encoding || "UTF-8",
       mozParams: searchProvider.params,
     };
     return this.addEngineWithDetails(searchProvider.name.trim(), params);
   },
 
   async addEngine(engineURL, iconURL, confirm, extensionID) {
     LOG("addEngine: Adding \"" + engineURL + "\".");
-    await this._promiseInitialized();
+    await this.init();
     let errCode;
     try {
       var uri = makeURI(engineURL);
       var engine = new Engine(uri, false);
       engine._setIcon(iconURL, false);
       engine._confirm = confirm;
       if (extensionID) {
         engine._extensionID = extensionID;
@@ -4063,17 +4047,17 @@ SearchService.prototype = {
       if (engine)
         engine._installCallback = null;
       FAIL("addEngine: Error adding engine:\n" + ex, errCode || Cr.NS_ERROR_FAILURE);
     }
     return engine;
   },
 
   async removeEngine(engine) {
-    await this._promiseInitialized();
+    await this.init();
     if (!engine)
       FAIL("no engine passed to removeEngine!");
 
     var engineToRemove = null;
     for (var e in this._engines) {
       if (engine.wrappedJSObject == this._engines[e])
         engineToRemove = this._engines[e];
     }
@@ -4112,17 +4096,17 @@ SearchService.prototype = {
 
       // Since we removed an engine, we need to update the preferences.
       this._saveSortedEngineList();
     }
     notifyAction(engineToRemove, SEARCH_ENGINE_REMOVED);
   },
 
   async moveEngine(engine, newIndex) {
-    await this._promiseInitialized();
+    await this.init();
     if ((newIndex > this._sortedEngines.length) || (newIndex < 0))
       FAIL("SRCH_SVC_moveEngine: Index out of bounds!");
     if (!(engine instanceof Ci.nsISearchEngine) && !(engine instanceof Engine))
       FAIL("SRCH_SVC_moveEngine: Invalid engine passed to moveEngine!");
     if (engine.hidden)
       FAIL("moveEngine: Can't move a hidden engine!", Cr.NS_ERROR_FAILURE);
 
     engine = engine.wrappedJSObject;
@@ -4263,22 +4247,22 @@ SearchService.prototype = {
     this.setGlobalAttr("current", newName);
     this.setGlobalAttr("hash", getVerificationHash(newName));
 
     notifyAction(this._currentEngine, SEARCH_ENGINE_DEFAULT);
     notifyAction(this._currentEngine, SEARCH_ENGINE_CURRENT);
   },
 
   async getDefault() {
-    await this._promiseInitialized();
+    await this.init();
     return this.defaultEngine;
   },
 
   async setDefault(engine) {
-    await this._promiseInitialized();
+    await this.init();
     return this.defaultEngine = engine;
   },
 
   async getDefaultEngineInfo() {
     let result = {};
 
     let engine;
     try {
--- a/toolkit/components/search/nsSearchSuggestions.js
+++ b/toolkit/components/search/nsSearchSuggestions.js
@@ -130,23 +130,19 @@ SuggestAutoComplete.prototype = {
 
     // Start search immediately if possible, otherwise once the search
     // service is initialized
     if (Services.search.isInitialized) {
       this._triggerSearch(searchString, formHistorySearchParam, listener, privacyMode);
       return;
     }
 
-    Services.search.init(aResult => {
-      if (!Components.isSuccessCode(aResult)) {
-        Cu.reportError("Could not initialize search service, bailing out: " + aResult);
-        return;
-      }
+    Services.search.init().then(() => {
       this._triggerSearch(searchString, formHistorySearchParam, listener, privacyMode);
-    });
+    }).catch(result => Cu.reportError("Could not initialize search service, bailing out: " + result));
   },
 
   /**
    * Actual implementation of search.
    */
   _triggerSearch(searchString, searchParam, listener, privacyMode) {
     this._listener = listener;
     this._suggestionController.fetch(searchString,
--- a/toolkit/components/utils/ClientEnvironment.jsm
+++ b/toolkit/components/utils/ClientEnvironment.jsm
@@ -65,17 +65,17 @@ class ClientEnvironmentBase {
   }
 
   static get isDefaultBrowser() {
     return ShellService.isDefaultBrowser();
   }
 
   static get searchEngine() {
     return (async () => {
-      const searchInitialized = await new Promise(resolve => Services.search.init(resolve));
+      const searchInitialized = await Services.search.init();
       if (Components.isSuccessCode(searchInitialized)) {
         return Services.search.defaultEngine.identifier;
       }
       return null;
     })();
   }
 
   static get syncSetup() {