Backed out changeset bd0060c41a3d (bug 1140558) for xpcshell bustage on a CLOSED TREE

toolkit/components/telemetry/TelemetryPing.jsm

@@ -345,10 +345,16 @@ let Impl = {
     }
 
     if (aOptions.addEnvironment) {
-      pingData.environment = TelemetryEnvironment.currentEnvironment;
+      return TelemetryEnvironment.getEnvironmentData().then(environment => {
+        pingData.environment = environment;
+        return pingData;
+      },
+      error => {
+        this._log.error("assemblePing - Rejection", error);
+      });
     }
 
-    return pingData;
+    return Promise.resolve(pingData);
   },
 
   popPayloads: function popPayloads() {
@@ -391,7 +397,8 @@ let Impl = {
     this._log.trace("send - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
 
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
+    return this.assemblePing(aType, aPayload, aOptions)
+        .then(pingData => {
           // Once ping is assembled, send it along with the persisted ping in the backlog.
           let p = [
             // Persist the ping if sending it fails.
@@ -401,6 +408,8 @@ let Impl = {
           ];
           return Promise.all(p);
         },
+        error => this._log.error("send - Rejection", error));
+  },
 
   /**
    * Send the persisted pings to the server.
@@ -431,8 +440,9 @@ let Impl = {
     this._log.trace("savePendingPings - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
 
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
-    return TelemetryFile.savePendingPings(pingData);
+    return this.assemblePing(aType, aPayload, aOptions)
+        .then(pingData => TelemetryFile.savePendingPings(pingData),
+              error => this._log.error("savePendingPings - Rejection", error));
   },
 
   /**
@@ -455,8 +465,9 @@ let Impl = {
     this._log.trace("savePing - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
 
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
-    return TelemetryFile.savePing(pingData, aOptions.overwrite);
+    return this.assemblePing(aType, aPayload, aOptions)
+        .then(pingData => TelemetryFile.savePing(pingData, aOptions.overwrite),
+              error => this._log.error("savePing - Rejection", error));
   },
 
   /**
@@ -481,7 +492,8 @@ let Impl = {
   testSavePingToFile: function testSavePingToFile(aType, aPayload, aOptions) {
     this._log.trace("testSavePingToFile - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
+    return this.assemblePing(aType, aPayload, aOptions)
+        .then(pingData => {
             if (aOptions.filePath) {
               return TelemetryFile.savePingToFile(pingData, aOptions.filePath, aOptions.overwrite)
                                   .then(() => { return pingData.id; });
@@ -489,6 +501,7 @@ let Impl = {
               return TelemetryFile.savePing(pingData, aOptions.overwrite)
                                   .then(() => { return pingData.id; });
             }
+        }, error => this._log.error("testSavePing - Rejection", error));
   },
 
   finishPingRequest: function finishPingRequest(success, startTime, ping, isPersisted) {
@@ -696,6 +709,8 @@ let Impl = {
       try {
         this._initialized = true;
 
+        yield TelemetryEnvironment.init();
+
         yield TelemetryFile.loadSavedPings();
         // If we have any TelemetryPings lying around, we'll be aggressive
         // and try to send them all off ASAP.
@@ -749,7 +764,8 @@ let Impl = {
         this._initialized = false;
         this._initStarted = false;
       };
-      this._shutdownBarrier.wait().then(reset, reset);
+      return this._shutdownBarrier.wait().then(
+               () => TelemetryEnvironment.shutdown().then(reset, reset));
     };
 
     // We can be in one the following states here:

toolkit/components/telemetry/TelemetrySession.jsm

@@ -1170,12 +1170,12 @@ let Impl = {
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
         this._rescheduleDailyTimer();
         TelemetryEnvironment.registerChangeListener(ENVIRONMENT_CHANGE_LISTENER,
-                                                    (reason, data) => this._onEnvironmentChange(reason, data));
+                                                    () => this._onEnvironmentChange());
 #endif
 
         this._delayedInitTaskDeferred.resolve();
       } catch (e) {
-        this._delayedInitTaskDeferred.reject(e);
+        this._delayedInitTaskDeferred.reject();
       } finally {
         this._delayedInitTask = null;
         this._delayedInitTaskDeferred = null;
@@ -1643,8 +1643,8 @@ let Impl = {
     }
   }),
 
-  _onEnvironmentChange: function(reason, data) {
-    this._log.trace("_onEnvironmentChange", reason);
+  _onEnvironmentChange: function() {
+    this._log.trace("_onEnvironmentChange");
     let payload = this.getSessionPayload(REASON_ENVIRONMENT_CHANGE, true);
 
     let options = {

toolkit/components/telemetry/docs/environment.rst

@@ -36,11 +36,8 @@ Structure::
           autoDownload: <bool>, // true on failure
         },
         userPrefs: {
-          // Only prefs which are changed from the default value are listed
-          // in this block
-          "pref.name.value": value // some prefs send the value
-          "pref.name.url": "<user-set>" // For some privacy-sensitive prefs
-            // only the fact that the value has been changed is recorded
+          // Two possible behaviours: values of the whitelisted prefs, or for some prefs we
+          // only record they are present with value being set to null.
         },
       },
       profile: { // This section is not available on Android.
@@ -182,8 +179,3 @@ Structure::
         persona: <string>, // id of the current persona, null on GONK
       },
     }
-
-Some parts of the environment must be fetched asynchronously at startup. If a session is very short or terminates early, the following items may be missing from the environment:
-
-- profile
-- addons

toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js

@@ -613,16 +613,69 @@ add_task(function* asyncSetup() {
   yield spoofProfileReset();
 });
 
+add_task(function* test_initAndShutdown() {
+  // Check that init and shutdown work properly.
+  TelemetryEnvironment.init();
+  yield TelemetryEnvironment.shutdown();
+  TelemetryEnvironment.init();
+  yield TelemetryEnvironment.shutdown();
+
+  // A double init should be silently handled.
+  TelemetryEnvironment.init();
+  TelemetryEnvironment.init();
+
+  // getEnvironmentData should return a sane result.
+  let data = yield TelemetryEnvironment.getEnvironmentData();
+  Assert.ok(!!data);
+
+  // The change listener registration should silently fail after shutdown.
+  yield TelemetryEnvironment.shutdown();
+  TelemetryEnvironment.registerChangeListener("foo", () => {});
+  TelemetryEnvironment.unregisterChangeListener("foo");
+
+  // Shutting down again should be ignored.
+  yield TelemetryEnvironment.shutdown();
+
+  // Getting the environment data should reject after shutdown.
+  Assert.ok(yield isRejected(TelemetryEnvironment.getEnvironmentData()));
+});
+
+add_task(function* test_changeNotify() {
+  TelemetryEnvironment.init();
+
+  // Register some listeners
+  let results = new Array(4).fill(false);
+  for (let i=0; i<results.length; ++i) {
+    let k = i;
+    TelemetryEnvironment.registerChangeListener("test"+k, () => results[k] = true);
+  }
+  // Trigger environment change notifications.
+  // TODO: test with proper environment changes, not directly.
+  TelemetryEnvironment._onEnvironmentChange("foo");
+  Assert.ok(results.every(val => val), "All change listeners should have been notified.");
+  results.fill(false);
+  TelemetryEnvironment._onEnvironmentChange("bar");
+  Assert.ok(results.every(val => val), "All change listeners should have been notified.");
+
+  // Unregister listeners
+  for (let i=0; i<4; ++i) {
+    TelemetryEnvironment.unregisterChangeListener("test"+i);
+  }
+});
+
 add_task(function* test_checkEnvironment() {
-  let environmentData = yield TelemetryEnvironment.onInitialized();
+  yield TelemetryEnvironment.init();
+  let environmentData = yield TelemetryEnvironment.getEnvironmentData();
+
   checkEnvironmentData(environmentData);
+
+  yield TelemetryEnvironment.shutdown();
 });
 
 add_task(function* test_prefWatchPolicies() {
   const PREF_TEST_1 = "toolkit.telemetry.test.pref_new";
   const PREF_TEST_2 = "toolkit.telemetry.test.pref1";
   const PREF_TEST_3 = "toolkit.telemetry.test.pref2";
-  const PREF_TEST_4 = "toolkit.telemetry.test.pref_old";
 
   const expectedValue = "some-test-value";
 
@@ -630,18 +683,12 @@ add_task(function* test_prefWatchPolicies() {
   prefsToWatch[PREF_TEST_1] = TelemetryEnvironment.RECORD_PREF_VALUE;
   prefsToWatch[PREF_TEST_2] = TelemetryEnvironment.RECORD_PREF_STATE;
   prefsToWatch[PREF_TEST_3] = TelemetryEnvironment.RECORD_PREF_STATE;
-  prefsToWatch[PREF_TEST_4] = TelemetryEnvironment.RECORD_PREF_VALUE;
 
-  Preferences.set(PREF_TEST_4, expectedValue);
+  yield TelemetryEnvironment.init();
 
   // Set the Environment preferences to watch.
   TelemetryEnvironment._watchPreferences(prefsToWatch);
   let deferred = PromiseUtils.defer();
-
-  // Check that the pref values are missing or present as expected
-  Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST_1], undefined);
-  Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST_4], expectedValue);
-
   TelemetryEnvironment.registerChangeListener("testWatchPrefs", deferred.resolve);
 
   // Trigger a change in the watched preferences.
@@ -653,14 +700,18 @@ add_task(function* test_prefWatchPolicies() {
   TelemetryEnvironment.unregisterChangeListener("testWatchPrefs");
 
   // Check environment contains the correct data.
-  let userPrefs = TelemetryEnvironment.currentEnvironment.settings.userPrefs;
+  let environmentData = yield TelemetryEnvironment.getEnvironmentData();
+
+  let userPrefs = environmentData.settings.userPrefs;
 
   Assert.equal(userPrefs[PREF_TEST_1], expectedValue,
                "Environment contains the correct preference value.");
-  Assert.equal(userPrefs[PREF_TEST_2], "<user-set>",
-               "Report that the pref was user set but the value is not shown.");
+  Assert.equal(userPrefs[PREF_TEST_2], null,
+               "Report that the pref was user set and has no value.");
   Assert.ok(!(PREF_TEST_3 in userPrefs),
             "Do not report if preference not user set.");
+
+  yield TelemetryEnvironment.shutdown();
 });
 
 add_task(function* test_prefWatch_prefReset() {
@@ -671,21 +722,20 @@ add_task(function* test_prefWatch_prefReset() {
   // Set the preference to a non-default value.
   Preferences.set(PREF_TEST, false);
 
+  yield TelemetryEnvironment.init();
+
   // Set the Environment preferences to watch.
   TelemetryEnvironment._watchPreferences(prefsToWatch);
   let deferred = PromiseUtils.defer();
   TelemetryEnvironment.registerChangeListener("testWatchPrefs_reset", deferred.resolve);
 
-  Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST], "<user-set>");
-
   // Trigger a change in the watched preferences.
   Preferences.reset(PREF_TEST);
   yield deferred.promise;
 
-  Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST], undefined);
-
   // Unregister the listener.
   TelemetryEnvironment.unregisterChangeListener("testWatchPrefs_reset");
+  yield TelemetryEnvironment.shutdown();
 });
 
 add_task(function* test_addonsWatch_InterestingChange() {
@@ -694,13 +744,13 @@ add_task(function* test_addonsWatch_InterestingChange() {
   // We only expect a single notification for each install, uninstall, enable, disable.
   const EXPECTED_NOTIFICATIONS = 4;
 
+  yield TelemetryEnvironment.init();
   let deferred = PromiseUtils.defer();
   let receivedNotifications = 0;
 
   let registerCheckpointPromise = (aExpected) => {
     return new Promise(resolve => TelemetryEnvironment.registerChangeListener(
-      "testWatchAddons_Changes" + aExpected, (reason, data) => {
-        Assert.equal(reason, "addons-changed");
+      "testWatchAddons_Changes" + aExpected, () => {
         receivedNotifications++;
         resolve();
       }));
@@ -716,26 +766,24 @@ add_task(function* test_addonsWatch_InterestingChange() {
   yield AddonTestUtils.installXPIFromURL(ADDON_INSTALL_URL);
   yield checkpointPromise;
   assertCheckpoint(1);
-  Assert.ok(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons);
   
   checkpointPromise = registerCheckpointPromise(2);
   let addon = yield AddonTestUtils.getAddonById(ADDON_ID);
   addon.userDisabled = true;
   yield checkpointPromise;
   assertCheckpoint(2);
-  Assert.ok(!(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons));
 
   checkpointPromise = registerCheckpointPromise(3);
   addon.userDisabled = false;
   yield checkpointPromise;
   assertCheckpoint(3);
-  Assert.ok(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons);
 
   checkpointPromise = registerCheckpointPromise(4);
   yield AddonTestUtils.uninstallAddonByID(ADDON_ID);
   yield checkpointPromise;
   assertCheckpoint(4);
-  Assert.ok(!(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons));
+
+  yield TelemetryEnvironment.shutdown();
 
   Assert.equal(receivedNotifications, EXPECTED_NOTIFICATIONS,
                "We must only receive the notifications we expect.");
@@ -747,16 +795,15 @@ add_task(function* test_pluginsWatch_Add() {
     return;
   }
 
-  Assert.equal(TelemetryEnvironment.currentEnvironment.addons.activePlugins.length, 1);
+  yield TelemetryEnvironment.init();
 
   let newPlugin = new PluginTag(PLUGIN2_NAME, PLUGIN2_DESC, PLUGIN2_VERSION, true);
   gInstalledPlugins.push(newPlugin);
 
   let deferred = PromiseUtils.defer();
   let receivedNotifications = 0;
-  let callback = (reason, data) => {
+  let callback = () => {
     receivedNotifications++;
-    Assert.equal(reason, "addons-changed");
     deferred.resolve();
   };
   TelemetryEnvironment.registerChangeListener("testWatchPlugins_Add", callback);
@@ -764,9 +811,8 @@ add_task(function* test_pluginsWatch_Add() {
   Services.obs.notifyObservers(null, PLUGIN_UPDATED_TOPIC, null);
   yield deferred.promise;
 
-  Assert.equal(TelemetryEnvironment.currentEnvironment.addons.activePlugins.length, 2);
-
   TelemetryEnvironment.unregisterChangeListener("testWatchPlugins_Add");
+  yield TelemetryEnvironment.shutdown();
 
   Assert.equal(receivedNotifications, 1, "We must only receive one notification.");
 });
@@ -777,6 +823,8 @@ add_task(function* test_pluginsWatch_Remove() {
     return;
   }
 
+  yield TelemetryEnvironment.init();
+
   // Find the test plugin.
   let plugin = gInstalledPlugins.find(plugin => (plugin.name == PLUGIN2_NAME));
   Assert.ok(plugin, "The test plugin must exist.");
@@ -796,6 +844,7 @@ add_task(function* test_pluginsWatch_Remove() {
   yield deferred.promise;
 
   TelemetryEnvironment.unregisterChangeListener("testWatchPlugins_Remove");
+  yield TelemetryEnvironment.shutdown();
 
   Assert.equal(receivedNotifications, 1, "We must only receive one notification.");
 });
@@ -804,25 +853,19 @@ add_task(function* test_addonsWatch_NotInterestingChange() {
   // We are not interested to dictionary addons changes.
   const DICTIONARY_ADDON_INSTALL_URL = gDataRoot + "dictionary.xpi";
   const INTERESTING_ADDON_INSTALL_URL = gDataRoot + "restartless.xpi";
+  yield TelemetryEnvironment.init();
 
-  let receivedNotification = false;
-  let deferred = PromiseUtils.defer();
+  let receivedNotifications = 0;
   TelemetryEnvironment.registerChangeListener("testNotInteresting",
-    () => {
-      Assert.ok(!receivedNotification, "Should not receive multiple notifications");
-      receivedNotification = true;
-      deferred.resolve();
-    });
+                                              () => receivedNotifications++);
 
   yield AddonTestUtils.installXPIFromURL(DICTIONARY_ADDON_INSTALL_URL);
   yield AddonTestUtils.installXPIFromURL(INTERESTING_ADDON_INSTALL_URL);
 
-  yield deferred.promise;
-  Assert.ok(!("telemetry-dictionary@tests.mozilla.org" in
-              TelemetryEnvironment.currentEnvironment.addons.activeAddons),
-            "Dictionaries should not appear in active addons.");
+  Assert.equal(receivedNotifications, 1, "We must receive only one notification.");
 
   TelemetryEnvironment.unregisterChangeListener("testNotInteresting");
+  yield TelemetryEnvironment.shutdown();
 });
 
 add_task(function* test_addonsAndPlugins() {
@@ -853,10 +896,12 @@ add_task(function* test_addonsAndPlugins() {
     clicktoplay: true,
   };
 
+  yield TelemetryEnvironment.init();
+
   // Install an addon so we have some data.
   yield AddonTestUtils.installXPIFromURL(ADDON_INSTALL_URL);
 
-  let data = TelemetryEnvironment.currentEnvironment;
+  let data = yield TelemetryEnvironment.getEnvironmentData();
   checkEnvironmentData(data);
 
   // Check addon data.
@@ -886,6 +931,8 @@ add_task(function* test_addonsAndPlugins() {
 
   let personaId = (gIsGonk) ? null : PERSONA_ID;
   Assert.equal(data.addons.persona, personaId, "The correct Persona Id must be reported.");
+
+  yield TelemetryEnvironment.shutdown();
 });
 
 add_task(function*() {

toolkit/components/telemetry/tests/unit/test_TelemetrySession.js

@@ -1064,7 +1064,7 @@ add_task(function* test_savedSessionData() {
 
   // We expect one new subsession when starting TelemetrySession and one after triggering
   // an environment change.
-  const expectedSubsessions = sessionState.profileSubsessionCounter + 3;
+  const expectedSubsessions = sessionState.profileSubsessionCounter + 2;
   const expectedUUID = "009fd1ad-b85e-4817-b3e5-000000003785";
   fakeGenerateUUID(generateUUID, () => expectedUUID);
 
@@ -1076,8 +1076,6 @@ add_task(function* test_savedSessionData() {
   // Start TelemetrySession so that it loads the session data file.
   yield TelemetrySession.reset();
   // Watch a test preference, trigger and environment change and wait for it to propagate.
-
-  // _watchPreferences triggers a subsession notification
   TelemetryEnvironment._watchPreferences(prefsToWatch);
   let changePromise = new Promise(resolve =>
     TelemetryEnvironment.registerChangeListener("test_fake_change", resolve));