Bug 1385396 - Cache early setExperimentActive calls r?gfritzsche draft
authorDoug Thayer <dothayer@mozilla.com>
Wed, 02 Aug 2017 15:10:34 -0700
changeset 621331 fec6e4c6446b3ddd497842987799f4619541efa1
parent 620862 32083f24a1bb2c33050b4c972783f066432194eb
child 640979 98a0b469eb6d86d20ab10ca352f0ed78710ba93c
push id72341
push userbmo:dothayer@mozilla.com
push dateFri, 04 Aug 2017 18:04:02 +0000
reviewersgfritzsche
bugs1385396
milestone57.0a1
Bug 1385396 - Cache early setExperimentActive calls r?gfritzsche Calling setExperimentActive too early during startup can change the order of some initialization. setExperimentActive probably shouldn't have this kind of effect, so simply cache early calls to it until gGlobalEnvironment has been initialized through other functions. Additionally, I am speculatively including work to ensure that setExperimentInactive and getActiveExperiments have the same behavior, while remaining correct by working from the same cache that setExperimentActive uses. MozReview-Commit-ID: IlzT1J0o6gK
toolkit/components/telemetry/TelemetryEnvironment.jsm
--- a/toolkit/components/telemetry/TelemetryEnvironment.jsm
+++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -47,16 +47,21 @@ const MAX_EXPERIMENT_TYPE_LENGTH = 20;
 
 /**
  * This is a policy object used to override behavior for testing.
  */
 var Policy = {
   now: () => new Date(),
 };
 
+// This is used to buffer calls to setExperimentActive and friends, so that we
+// don't prematurely initialize our environment if it is called early during
+// startup.
+var gActiveExperimentStartupBuffer = new Map();
+
 var gGlobalEnvironment;
 function getGlobal() {
   if (!gGlobalEnvironment) {
     gGlobalEnvironment = new EnvironmentCache();
   }
   return gGlobalEnvironment;
 }
 
@@ -87,41 +92,57 @@ this.TelemetryEnvironment = {
    * This triggers a new subsession, subject to throttling.
    *
    * @param {String} id The id of the active experiment.
    * @param {String} branch The experiment branch.
    * @param {Object} [options] Optional object with options.
    * @param {String} [options.type=false] The specific experiment type.
    */
   setExperimentActive(id, branch, options = {}) {
-    return getGlobal().setExperimentActive(id, branch, options);
+    if (gGlobalEnvironment) {
+      gGlobalEnvironment.setExperimentActive(id, branch, options);
+    } else {
+      gActiveExperimentStartupBuffer.set(id, {branch, options});
+    }
   },
 
   /**
    * Remove an experiment annotation from the environment.
    * If the annotation exists, a new subsession will triggered.
    *
    * @param {String} id The id of the active experiment.
    */
   setExperimentInactive(id) {
-    return getGlobal().setExperimentInactive(id);
+    if (gGlobalEnvironment) {
+      gGlobalEnvironment.setExperimentInactive(id);
+    } else {
+      gActiveExperimentStartupBuffer.delete(id);
+    }
   },
 
   /**
    * Returns an object containing the data for the active experiments.
    *
    * The returned object is of the format:
    *
    * {
    *   "<experiment id>": { branch: "<branch>" },
    *   // …
    * }
    */
   getActiveExperiments() {
-    return getGlobal().getActiveExperiments();
+    if (gGlobalEnvironment) {
+      return gGlobalEnvironment.getActiveExperiments();
+    }
+
+    const result = {};
+    for (const [id, {branch}] of gActiveExperimentStartupBuffer.entries()) {
+      result[id] = branch;
+    }
+    return result;
   },
 
   shutdown() {
     return getGlobal().shutdown();
   },
 
   // Policy to use when saving preferences. Exported for using them in tests.
   RECORD_PREF_STATE: 1, // Don't record the preference value
@@ -146,16 +167,17 @@ this.TelemetryEnvironment = {
   },
 
   /**
    * Intended for use in tests only.
    */
   testCleanRestart() {
     getGlobal().shutdown();
     gGlobalEnvironment = null;
+    gActiveExperimentStartupBuffer = new Map();
     return getGlobal();
   },
 };
 
 const RECORD_PREF_STATE = TelemetryEnvironment.RECORD_PREF_STATE;
 const RECORD_PREF_VALUE = TelemetryEnvironment.RECORD_PREF_VALUE;
 const RECORD_DEFAULTPREF_VALUE = TelemetryEnvironment.RECORD_DEFAULTPREF_VALUE;
 const DEFAULT_ENVIRONMENT_PREFS = new Map([
@@ -841,16 +863,21 @@ function EnvironmentCache() {
   p = [ this._addonBuilder.init() ];
 
   this._currentEnvironment.profile = {};
   p.push(this._updateProfile());
   if (AppConstants.MOZ_BUILD_APP == "browser") {
     p.push(this._updateAttribution());
   }
 
+  for (const [id, {branch, options}] of gActiveExperimentStartupBuffer.entries()) {
+    this.setExperimentActive(id, branch, options);
+  }
+  gActiveExperimentStartupBuffer = null;
+
   let setup = () => {
     this._initTask = null;
     this._startWatchingPrefs();
     this._addonBuilder.watchForChanges();
     this._updateGraphicsFeatures();
     return this.currentEnvironment;
   };