Bug 1492475 - Part 5: Since async initialization of the search service now is implicit behavior, remove the distinctive verbiage used internally. r=florian
authorMike de Boer <mdeboer@mozilla.com>
Tue, 16 Oct 2018 16:53:50 +0200
changeset 452867 cd41189eac88aa6023af1b0a060c15ddcd407952
parent 452866 c1e172dfacad4b14ebdb352bee2fd946716acd59
child 452868 2ae7189dfaa63cab0e264e7a2796b1610505c40a
push id2
push usermdeboer@mozilla.com
push dateTue, 08 Jan 2019 17:01:21 +0000
reviewersflorian
bugs1492475
milestone66.0a1
Bug 1492475 - Part 5: Since async initialization of the search service now is implicit behavior, remove the distinctive verbiage used internally. r=florian Differential Revision: https://phabricator.services.mozilla.com/D9279
toolkit/components/search/nsSearchService.js
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -1284,17 +1284,17 @@ Engine.prototype = {
    * Retrieves the data from the engine's file asynchronously.
    * The document element is placed in the engine's data field.
    *
    * @param file The file to load the search plugin from.
    *
    * @returns {Promise} A promise, resolved successfully if initializing from
    * data succeeds, rejected if it fails.
    */
-  async _asyncInitFromFile(file) {
+  async _initFromFile(file) {
     if (!file || !(await OS.File.exists(file.path)))
       FAIL("File must exist before calling initFromFile!", Cr.NS_ERROR_UNEXPECTED);
 
     let fileURI = Services.io.newFileURI(file);
     await this._retrieveSearchXMLData(fileURI.spec);
 
     // Now that the data is loaded, initialize the engine object
     this._initFromData();
@@ -1329,18 +1329,18 @@ Engine.prototype = {
   /**
    * Retrieves the engine data from a URI asynchronously and initializes it.
    *
    * @param uri The uri to load the search plugin from.
    *
    * @returns {Promise} A promise, resolved successfully if retrieveing data
    * succeeds.
    */
-  async _asyncInitFromURI(uri) {
-    LOG("_asyncInitFromURI: Loading engine from: \"" + uri.spec + "\".");
+  async _initFromURI(uri) {
+    LOG("_initFromURI: Loading engine from: \"" + uri.spec + "\".");
     await this._retrieveSearchXMLData(uri.spec);
     // Now that the data is loaded, initialize the engine object
     this._initFromData();
   },
 
   /**
    * Retrieves the engine data for a given URI asynchronously.
    *
@@ -2582,40 +2582,16 @@ ParseSubmissionResult.prototype = {
     return this._termsLength;
   },
   QueryInterface: ChromeUtils.generateQI([Ci.nsISearchParseSubmissionResult]),
 };
 
 const gEmptyParseSubmissionResult =
       Object.freeze(new ParseSubmissionResult(null, "", -1, 0));
 
-function executeSoon(func) {
-  Services.tm.dispatchToMainThread(func);
-}
-
-/**
- * Check for sync initialization has completed or not.
- *
- * @param {aPromise} A promise.
- *
- * @returns the value returned by the invoked method.
- * @throws NS_ERROR_ALREADY_INITIALIZED if sync initialization has completed.
- */
-function checkForSyncCompletion(aPromise) {
-  return aPromise.then(function(aValue) {
-    if (gInitialized) {
-      throw Components.Exception("Synchronous fallback was called and has " +
-                                 "finished so no need to pursue asynchronous " +
-                                 "initialization",
-                                 Cr.NS_ERROR_ALREADY_INITIALIZED);
-    }
-    return aValue;
-  });
-}
-
 // nsISearchService
 function SearchService() {
   this._initObservers = PromiseUtils.defer();
 }
 
 SearchService.prototype = {
   classID: Components.ID("{7319788a-fe93-4db3-9f39-818cf08f4256}"),
 
@@ -2653,53 +2629,45 @@ SearchService.prototype = {
   },
 
   /**
    * Asynchronous implementation of the initializer.
    *
    * @returns {Promise} A promise, resolved successfully if the initialization
    * succeeds.
    */
-  async _asyncInit() {
-    LOG("_asyncInit start");
+  async _init() {
+    LOG("_init start");
 
     // See if we have a cache file so we don't have to parse a bunch of XML.
-    // Not using checkForSyncCompletion here because we want to ensure we
-    // fetch the region and geo specific defaults asynchronously even
-    // if a sync init has been forced.
-    let cache = await this._asyncReadCacheFile();
+    let cache = await this._readCacheFile();
 
     try {
-      await checkForSyncCompletion(ensureKnownRegion(this));
+      await ensureKnownRegion(this);
     } catch (ex) {
-      if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-        throw ex;
-      }
-      LOG("_asyncInit: failure determining region: " + ex);
+      LOG("_init: failure determining region: " + ex);
     }
     try {
-      await checkForSyncCompletion(this._asyncLoadEngines(cache));
+      await this._loadEngines(cache);
     } catch (ex) {
-      if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-        throw ex;
-      }
       this._initRV = Cr.NS_ERROR_FAILURE;
-      LOG("_asyncInit: failure loading engines: " + ex);
+      LOG("_init: failure loading engines: " + ex);
     }
     this._addObservers();
     gInitialized = true;
     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");
+    LOG("_init: Completed _init");
+    return this._initRV;
   },
 
   _metaData: { },
   setGlobalAttr(name, val) {
     this._metaData[name] = val;
     this.batchTask.disarm();
     this.batchTask.arm();
   },
@@ -2820,21 +2788,20 @@ SearchService.prototype = {
   },
 
   /**
    * Loads engines asynchronously.
    *
    * @returns {Promise} A promise, resolved successfully if loading data
    * succeeds.
    */
-  async _asyncLoadEngines(cache) {
-    LOG("_asyncLoadEngines: start");
+  async _loadEngines(cache) {
+    LOG("_loadEngines: start");
     Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "find-jar-engines");
-    let chromeURIs =
-      await checkForSyncCompletion(this._asyncFindJAREngines());
+    let chromeURIs = await this._findJAREngines();
 
     // Get the non-empty distribution directories into distDirs...
     let distDirs = [];
     let locations;
     try {
       locations = getDir(NS_APP_DISTRIBUTION_SEARCH_DIR_LIST,
                          Ci.nsISimpleEnumerator);
     } catch (e) {
@@ -2842,17 +2809,17 @@ SearchService.prototype = {
       // so this throws during unit tests (but not xpcshell tests).
       locations = [];
     }
     for (let dir of locations) {
       let iterator = new OS.File.DirectoryIterator(dir.path,
                                                    { winPattern: "*.xml" });
       try {
         // Add dir to distDirs if it contains any files.
-        let {done} = await checkForSyncCompletion(iterator.next());
+        let {done} = await iterator.next();
         if (!done) {
           distDirs.push(dir);
         }
       } finally {
         iterator.close();
       }
     }
 
@@ -2864,46 +2831,44 @@ SearchService.prototype = {
     let rebuildCache = !cache.engines ||
                        cache.version != CACHE_VERSION ||
                        cache.locale != getLocale() ||
                        cache.buildID != buildID ||
                        cache.visibleDefaultEngines.length != this._visibleDefaultEngines.length ||
                        this._visibleDefaultEngines.some(notInCacheVisibleEngines);
 
     if (!rebuildCache) {
-      LOG("_asyncLoadEngines: loading from cache directories");
+      LOG("_loadEngines: loading from cache directories");
       this._loadEnginesFromCache(cache);
       if (Object.keys(this._engines).length) {
-        LOG("_asyncLoadEngines: done using existing cache");
+        LOG("_loadEngines: done using existing cache");
         return;
       }
-      LOG("_asyncLoadEngines: No valid engines found in cache. Loading engines from disk.");
+      LOG("_loadEngines: No valid engines found in cache. Loading engines from disk.");
     }
 
-    LOG("_asyncLoadEngines: Absent or outdated cache. Loading engines from disk.");
+    LOG("_loadEngines: Absent or outdated cache. Loading engines from disk.");
     for (let loadDir of distDirs) {
-      let enginesFromDir =
-        await checkForSyncCompletion(this._asyncLoadEnginesFromDir(loadDir));
+      let enginesFromDir = await this._loadEnginesFromDir(loadDir);
       enginesFromDir.forEach(this._addEngineToStore, this);
     }
-    let enginesFromURLs =
-      await checkForSyncCompletion(this._asyncLoadFromChromeURLs(chromeURIs));
+    let enginesFromURLs = await this._loadFromChromeURLs(chromeURIs);
     enginesFromURLs.forEach(this._addEngineToStore, this);
 
-    LOG("_asyncLoadEngines: loading user-installed engines from the obsolete cache");
+    LOG("_loadEngines: loading user-installed engines from the obsolete cache");
     this._loadEnginesFromCache(cache, true);
 
     this._loadEnginesMetadataFromCache(cache);
     this._buildCache();
 
-    LOG("_asyncLoadEngines: done using rebuilt cache");
+    LOG("_loadEngines: done using rebuilt cache");
   },
 
-  _asyncReInit() {
-    LOG("_asyncReInit");
+  _reInit() {
+    LOG("_reInit");
     // Start by clearing the initialized state, so we don't abort early.
     gInitialized = false;
 
     (async () => {
       try {
         if (this._batchTask) {
           LOG("finalizing batch task");
           let task = this._batchTask;
@@ -2920,20 +2885,20 @@ SearchService.prototype = {
         this._searchOrder = [];
         this._metaData = {};
 
         // Tests that want to force a synchronous re-initialization need to
         // be notified when we are done uninitializing.
         Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC,
                                      "uninit-complete");
 
-        let cache = await this._asyncReadCacheFile();
+        let cache = await this._readCacheFile();
 
         await ensureKnownRegion(this);
-        await this._asyncLoadEngines(cache);
+        await this._loadEngines(cache);
 
         // Typically we'll re-init as a result of a pref observer,
         // so signal to 'callers' that we're done.
         gInitialized = true;
         Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "init-complete");
       } catch (err) {
         LOG("Reinit failed: " + err);
         Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "reinit-failed");
@@ -2944,32 +2909,32 @@ SearchService.prototype = {
   },
 
   /**
    * Read the cache file asynchronously.
    *
    * @returns {Promise} A promise, resolved successfully if retrieveing data
    * succeeds.
    */
-  async _asyncReadCacheFile() {
+  async _readCacheFile() {
     let json;
     try {
       let cacheFilePath = OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME);
       let bytes = await OS.File.read(cacheFilePath, {compression: "lz4"});
       json = JSON.parse(new TextDecoder().decode(bytes));
       if (!json.engines || !json.engines.length)
         throw "no engine in the file";
       // Reset search default expiration on major releases
       if (json.appVersion != Services.appinfo.version &&
           geoSpecificDefaultsEnabled() &&
           json.metaData) {
         json.metaData.searchDefaultExpir = 0;
       }
     } catch (ex) {
-      LOG("_asyncReadCacheFile: Error reading cache file: " + ex);
+      LOG("_readCacheFile: Error reading cache file: " + ex);
       json = {};
     }
     if (!gInitialized && json.metaData)
       this._metaData = json.metaData;
 
     return json;
   },
 
@@ -3119,18 +3084,18 @@ SearchService.prototype = {
   /**
    * Loads engines from a given directory asynchronously.
    *
    * @param aDir the directory.
    *
    * @returns {Promise} A promise, resolved successfully if retrieveing data
    * succeeds.
    */
-  async _asyncLoadEnginesFromDir(aDir) {
-    LOG("_asyncLoadEnginesFromDir: Searching in " + aDir.path + " for search engines.");
+  async _loadEnginesFromDir(aDir) {
+    LOG("_loadEnginesFromDir: Searching in " + aDir.path + " for search engines.");
 
     let iterator = new OS.File.DirectoryIterator(aDir.path);
 
     let osfiles = await iterator.nextBatch();
     iterator.close();
 
     let engines = [];
     for (let osfile of osfiles) {
@@ -3147,82 +3112,76 @@ SearchService.prototype = {
         continue;
       }
 
       let addedEngine = null;
       try {
         let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
         file.initWithPath(osfile.path);
         addedEngine = new Engine(file, false);
-        await checkForSyncCompletion(addedEngine._asyncInitFromFile(file));
+        await addedEngine._initFromFile(file);
         engines.push(addedEngine);
       } catch (ex) {
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
-        }
-        LOG("_asyncLoadEnginesFromDir: Failed to load " + osfile.path + "!\n" + ex);
+        LOG("_loadEnginesFromDir: Failed to load " + osfile.path + "!\n" + ex);
       }
     }
     return engines;
   },
 
   /**
    * Loads engines from Chrome URLs asynchronously.
    *
    * @param aURLs a list of URLs.
    *
    * @returns {Promise} A promise, resolved successfully if loading data
    * succeeds.
    */
-  async _asyncLoadFromChromeURLs(aURLs) {
+  async _loadFromChromeURLs(aURLs) {
     let engines = [];
     for (let url of aURLs) {
       try {
-        LOG("_asyncLoadFromChromeURLs: loading engine from chrome url: " + url);
+        LOG("_loadFromChromeURLs: loading engine from chrome url: " + url);
         let uri = Services.io.newURI(url);
         let engine = new Engine(uri, true);
-        await checkForSyncCompletion(engine._asyncInitFromURI(uri));
+        await engine._initFromURI(uri);
         engines.push(engine);
       } catch (ex) {
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
-        }
-        LOG("_asyncLoadFromChromeURLs: failed to load engine: " + ex);
+        LOG("_loadFromChromeURLs: failed to load engine: " + ex);
       }
     }
     return engines;
   },
 
   /**
    * Loads jar engines asynchronously.
    *
    * @returns {Promise} A promise, resolved successfully if finding jar engines
    * succeeds.
    */
-  async _asyncFindJAREngines() {
-    LOG("_asyncFindJAREngines: looking for engines in JARs");
+  async _findJAREngines() {
+    LOG("_findJAREngines: looking for engines in JARs");
 
     let listURL = APP_SEARCH_PREFIX + "list.json";
     let chan = makeChannel(listURL);
     if (!chan) {
-      LOG("_asyncFindJAREngines: " + APP_SEARCH_PREFIX + " isn't registered");
+      LOG("_findJAREngines: " + APP_SEARCH_PREFIX + " isn't registered");
       return [];
     }
 
     let uris = [];
 
     // Read list.json to find the engines we need to load.
     let request = new XMLHttpRequest();
     request.overrideMimeType("text/plain");
     let list = await new Promise(resolve => {
       request.onload = function(aEvent) {
         resolve(aEvent.target.responseText);
       };
       request.onerror = function(aEvent) {
-        LOG("_asyncFindJAREngines: failed to read " + listURL);
+        LOG("_findJAREngines: failed to read " + listURL);
         resolve();
       };
       request.open("GET", Services.io.newURI(listURL).spec, true);
       request.send();
     });
 
     this._parseListJSON(list, uris);
     return uris;
@@ -3524,26 +3483,27 @@ SearchService.prototype = {
     if (this._initStarted) {
       return this._initObservers.promise;
     }
 
     TelemetryStopwatch.start("SEARCH_SERVICE_INIT_MS");
     this._initStarted = true;
     try {
       // Complete initialization by calling asynchronous initializer.
-      await this._asyncInit();
+      await this._init();
       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");
+        throw ex;
       }
     }
 
     if (!Components.isSuccessCode(this._initRV)) {
       throw this._initRV;
     }
     return this._initRV;
   },
@@ -4325,17 +4285,17 @@ SearchService.prototype = {
         break;
 
       case TOPIC_LOCALES_CHANGE:
         // Locale changed. Re-init. We rely on observers, because we can't
         // return this promise to anyone.
         // FYI, This is also used by the search tests to do an async reinit.
         // Locales are removed during shutdown, so ignore this message
         if (!Services.startup.shuttingDown) {
-          this._asyncReInit();
+          this._reInit();
         }
         break;
     }
   },
 
   // nsITimerCallback
   notify: function SRCH_SVC_notify(aTimer) {
     LOG("_notify: checking for updates");