Bug 845842 - Use promises that resolve on later ticks; r=rnewman, a=bajaj
authorGregory Szorc <gps@mozilla.com>
Mon, 18 Mar 2013 20:47:34 -0700
changeset 132475 569a5ca67a5678868fe7000dffb2b56ed8e37cd6
parent 132474 caf6ea63dc25f39140fd6746693944c64922c835
child 132476 0f5eb110cf44aa7c8e9365dea5849b5160d12849
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, bajaj
bugs845842
milestone21.0a2
Bug 845842 - Use promises that resolve on later ticks; r=rnewman, a=bajaj This is meant as a temporary workaround until a built-in promise library offers similar functionality.
services/common/utils.js
services/healthreport/healthreporter.jsm
services/healthreport/providers.jsm
services/metrics/dataprovider.jsm
services/metrics/providermanager.jsm
services/metrics/storage.jsm
--- a/services/common/utils.js
+++ b/services/common/utils.js
@@ -1,16 +1,17 @@
 /* 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 {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 this.EXPORTED_SYMBOLS = ["CommonUtils"];
 
+Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/osfile.jsm")
 Cu.import("resource://services-common/log4moz.js");
 
 this.CommonUtils = {
   exceptionStr: function exceptionStr(e) {
     let message = e.message ? e.message : e;
@@ -101,16 +102,29 @@ this.CommonUtils = {
   nextTick: function nextTick(callback, thisObj) {
     if (thisObj) {
       callback = callback.bind(thisObj);
     }
     Services.tm.currentThread.dispatch(callback, Ci.nsIThread.DISPATCH_NORMAL);
   },
 
   /**
+   * Return a promise resolving on some later tick.
+   *
+   * This a wrapper around Promise.resolve() that prevents stack
+   * accumulation and prevents callers from accidentally relying on
+   * same-tick promise resolution.
+   */
+  laterTickResolvingPromise: function (value, prototype) {
+    let deferred = Promise.defer(prototype);
+    this.nextTick(deferred.resolve.bind(deferred, value));
+    return deferred.promise;
+  },
+
+  /**
    * Spin the event loop and return once the next tick is executed.
    *
    * This is an evil function and should not be used in production code. It
    * exists in this module for ease-of-use.
    */
   waitForNextTick: function waitForNextTick() {
     let cb = Async.makeSyncCallback();
     this.nextTick(cb);
--- a/services/healthreport/healthreporter.jsm
+++ b/services/healthreport/healthreporter.jsm
@@ -390,17 +390,17 @@ AbstractHealthReporter.prototype = Objec
    * Return a promise that is resolved once the service has been initialized.
    */
   onInit: function () {
     if (this._initializeHadError) {
       throw new Error("Service failed to initialize.");
     }
 
     if (this._initialized) {
-      return Promise.resolve(this);
+      return CommonUtils.laterTickResolvingPromise(this);
     }
 
     return this._initializedDeferred.promise;
   },
 
   _performDailyMaintenance: function () {
     this._log.info("Request to perform daily maintenance.");
 
@@ -787,17 +787,17 @@ AbstractHealthReporter.prototype = Objec
   getLastPayload: function () {
     let path = this._lastPayloadPath;
 
     return OS.File.read(path).then(
       function onData(buffer) {
         let decoder = new TextDecoder();
         let json = JSON.parse(decoder.decode(buffer));
 
-        return Promise.resolve(json);
+        return CommonUtils.laterTickResolvingPromise(json);
       },
       function onError(error) {
         return Promise.reject(error);
       }
     );
   },
 
   _now: function _now() {
@@ -1098,17 +1098,17 @@ HealthReporter.prototype = Object.freeze
     }
 
     return result;
   },
 
   _onBagheeraResult: function (request, isDelete, result) {
     this._log.debug("Received Bagheera result.");
 
-    let promise = Promise.resolve(null);
+    let promise = CommonUtils.laterTickResolvingPromise(null);
 
     if (!result.transportSuccess) {
       request.onSubmissionFailureSoft("Network transport error.");
       return promise;
     }
 
     if (!result.serverSuccess) {
       request.onSubmissionFailureHard("Server failure.");
--- a/services/healthreport/providers.jsm
+++ b/services/healthreport/providers.jsm
@@ -454,17 +454,17 @@ CurrentSessionMeasurement.prototype = Ob
     let now = new Date();
     fields.set("startDay", [now, Metrics.dateToDays(sessions.startDate)]);
     fields.set("activeTicks", [now, sessions.activeTicks]);
     fields.set("totalTime", [now, sessions.totalTime]);
     fields.set("main", [now, sessions.main]);
     fields.set("firstPaint", [now, sessions.firstPaint]);
     fields.set("sessionRestored", [now, sessions.sessionRestored]);
 
-    return Promise.resolve({
+    return CommonUtils.laterTickResolvingPromise({
       days: new Metrics.DailyValues(),
       singular: fields,
     });
   },
 
   _serializeJSONSingular: function (data) {
     let result = {"_v": this.version};
 
@@ -689,24 +689,24 @@ AddonsProvider.prototype = Object.freeze
 
     for (let method of this.ADDON_LISTENER_CALLBACKS) {
       listener[method] = this._collectAndStoreAddons.bind(this);
     }
 
     this._listener = listener;
     AddonManager.addAddonListener(this._listener);
 
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   onShutdown: function () {
     AddonManager.removeAddonListener(this._listener);
     this._listener = null;
 
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   collectConstantData: function () {
     return this._collectAndStoreAddons();
   },
 
   _collectAndStoreAddons: function () {
     let deferred = Promise.defer();
--- a/services/metrics/dataprovider.jsm
+++ b/services/metrics/dataprovider.jsm
@@ -581,58 +581,58 @@ Provider.prototype = Object.freeze({
    *
    * If a `Provider` instance needs to register observers, etc, it should
    * implement this function.
    *
    * Implementations should return a promise which is resolved when
    * initialization activities have completed.
    */
   onInit: function () {
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   /**
    * Hook point for shutdown of instances.
    *
    * This is the opposite of `onInit`. If a `Provider` needs to unregister
    * observers, etc, this is where it should do it.
    *
    * Implementations should return a promise which is resolved when
    * shutdown activities have completed.
    */
   onShutdown: function () {
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   /**
    * Collects data that doesn't change during the application's lifetime.
    *
    * Implementations should return a promise that resolves when all data has
    * been collected and storage operations have been finished.
    *
    * @return Promise<>
    */
   collectConstantData: function () {
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   /**
    * Collects data approximately every day.
    *
    * For long-running applications, this is called approximately every day.
    * It may or may not be called every time the application is run. It also may
    * be called more than once per day.
    *
    * Implementations should return a promise that resolves when all data has
    * been collected and storage operations have completed.
    *
    * @return Promise<>
    */
   collectDailyData: function () {
-    return Promise.resolve();
+    return CommonUtils.laterTickResolvingPromise();
   },
 
   /**
    * Queue a deferred storage operation.
    *
    * Deferred storage operations are the preferred method for providers to
    * interact with storage. When collected data is to be added to storage,
    * the provider creates a function that performs the necessary storage
--- a/services/metrics/providermanager.jsm
+++ b/services/metrics/providermanager.jsm
@@ -148,17 +148,17 @@ this.ProviderManager.prototype = Object.
    * @return Promise<null>
    */
   registerProvider: function (provider) {
     if (!(provider instanceof Provider)) {
       throw new Error("Argument must be a Provider instance.");
     }
 
     if (this._providers.has(provider.name)) {
-      return Promise.resolve();
+      return CommonUtils.laterTickResolvingPromise();
     }
 
     let deferred = Promise.defer();
     this._providerInitQueue.push([provider, deferred]);
 
     if (this._providerInitQueue.length == 1) {
       this._popAndInitProvider();
     }
@@ -217,46 +217,46 @@ this.ProviderManager.prototype = Object.
     this._providers.delete(name);
   },
 
   /**
    * Ensure that pull-only providers are registered.
    */
   ensurePullOnlyProvidersRegistered: function () {
     if (this._pullOnlyProvidersRegistered) {
-      return Promise.resolve();
+      return CommonUtils.laterTickResolvingPromise();
     }
 
     let onFinished = function () {
       this._pullOnlyProvidersRegistered = true;
 
-      return Promise.resolve();
+      return CommonUtils.laterTickResolvingPromise();
     }.bind(this);
 
     return Task.spawn(function registerPullProviders() {
       for each (let providerType in this._pullOnlyProviders) {
         try {
           let provider = this._initProviderFromType(providerType);
           yield this.registerProvider(provider);
         } catch (ex) {
           this._recordError("Error registering pull-only provider", ex);
         }
       }
     }.bind(this)).then(onFinished, onFinished);
   },
 
   ensurePullOnlyProvidersUnregistered: function () {
     if (!this._pullOnlyProvidersRegistered) {
-      return Promise.resolve();
+      return CommonUtils.laterTickResolvingPromise();
     }
 
     let onFinished = function () {
       this._pullOnlyProvidersRegistered = false;
 
-      return Promise.resolve();
+      return CommonUtils.laterTickResolvingPromise();
     }.bind(this);
 
     return Task.spawn(function unregisterPullProviders() {
       for (let provider of this.providers) {
         if (!provider.pullOnly) {
           continue;
         }
 
@@ -368,17 +368,17 @@ this.ProviderManager.prototype = Object.
           try {
             onCollect(entry, result);
           } catch (ex) {
             this._log.warn("onCollect callback threw: " +
                            CommonUtils.exceptionStr(ex));
           }
         }
 
-        return Promise.resolve(result);
+        return CommonUtils.laterTickResolvingPromise(result);
       });
 
       promises.push([provider.name, promise]);
     }
 
     return this._handleCollectionPromises(promises);
   },
 
--- a/services/metrics/storage.jsm
+++ b/services/metrics/storage.jsm
@@ -975,17 +975,18 @@ MetricsStorageSqliteBackend.prototype = 
    *        (string) Name of the provider this measurement belongs to.
    * @param name
    *        (string) Name of this measurement.
    * @param version
    *        (Number) Integer version of this measurement.
    */
   registerMeasurement: function (provider, name, version) {
     if (this.hasMeasurement(provider, name, version)) {
-      return Promise.resolve(this.measurementID(provider, name, version));
+      return CommonUtils.laterTickResolvingPromise(
+        this.measurementID(provider, name, version));
     }
 
     // Registrations might not be safe to perform in parallel with provider
     // operations. So, we queue them.
     let self = this;
     return this.enqueueOperation(function createMeasurementOperation() {
       return Task.spawn(function createMeasurement() {
         let providerID = self._providerIDs.get(provider);
@@ -1062,17 +1063,18 @@ MetricsStorageSqliteBackend.prototype = 
     if (this.hasFieldFromMeasurement(measurementID, field)) {
       let id = this.fieldIDFromMeasurement(measurementID, field);
       let existingType = this._fieldsByID.get(id)[2];
 
       if (valueType != existingType) {
         throw new Error("Field already defined with different type: " + existingType);
       }
 
-      return Promise.resolve(this.fieldIDFromMeasurement(measurementID, field));
+      return CommonUtils.laterTickResolvingPromise(
+        this.fieldIDFromMeasurement(measurementID, field));
     }
 
     let self = this;
     return Task.spawn(function createField() {
       let params = {
         measurement_id: measurementID,
         field: field,
         value_type: typeID,