Bug 1157359 - Fix profileSubsessionCounter and previousSubsessionId failing to update correctly. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Tue, 05 May 2015 01:33:37 +0200
changeset 273644 f8379fc805bd3720e59a548644afd12f71e96d86
parent 273643 2cf04909a23b96eeffdef7505b6a8d04ff2157bf
child 273645 97299f59d4cabcb4ff554a9185bc9f63383f2ea1
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1157359
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1157359 - Fix profileSubsessionCounter and previousSubsessionId failing to update correctly. r=gfritzsche
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -1527,22 +1527,21 @@ let Impl = {
     // Delay full telemetry initialization to give the browser time to
     // run various late initializers. Otherwise our gathered memory
     // footprint and other numbers would be too optimistic.
     this._delayedInitTaskDeferred = Promise.defer();
     this._delayedInitTask = new DeferredTask(function* () {
       try {
         this._initialized = true;
 
-        let hasLoaded = yield this._loadSessionData();
-        if (!hasLoaded) {
-          // We could not load a valid session data file. Create one.
-          yield this._saveSessionData(this._getSessionDataObject()).catch(() =>
-            this._log.error("setupChromeProcess - Could not write session data to disk."));
-        }
+        yield this._loadSessionData();
+        // Update the session data to keep track of new subsessions created before
+        // the initialization.
+        yield this._saveSessionData(this._getSessionDataObject());
+
         this.attachObservers();
         this.gatherMemory();
 
         Telemetry.asyncFetchTelemetryData(function () {});
 
         if (IS_UNIFIED_TELEMETRY) {
           // Check for a previously written aborted session ping.
           yield this._checkAbortedSessionPing();
@@ -1960,18 +1959,18 @@ let Impl = {
                                 SESSION_STATE_FILE_NAME);
 
     // Try to load the "profileSubsessionCounter" from the state file.
     try {
       let data = yield CommonUtils.readJSON(dataFile);
       if (data &&
           "profileSubsessionCounter" in data &&
           typeof(data.profileSubsessionCounter) == "number" &&
-          "previousSubsessionId" in data) {
-        this._previousSubsessionId = data.previousSubsessionId;
+          "subsessionId" in data) {
+        this._previousSubsessionId = data.subsessionId;
         // Add |_subsessionCounter| to the |_profileSubsessionCounter| to account for
         // new subsession while loading still takes place. This will always be exactly
         // 1 - the current subsessions.
         this._profileSubsessionCounter = data.profileSubsessionCounter +
                                          this._subsessionCounter;
         return true;
       }
     } catch (e) {
@@ -1980,17 +1979,17 @@ let Impl = {
     return false;
   }),
 
   /**
    * Get the session data object to serialise to disk.
    */
   _getSessionDataObject: function() {
     return {
-      previousSubsessionId: this._previousSubsessionId,
+      subsessionId: this._subsessionId,
       profileSubsessionCounter: this._profileSubsessionCounter,
     };
   },
 
   /**
    * Saves session data to disk.
    */
   _saveSessionData: Task.async(function* (sessionData) {
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -1162,17 +1162,17 @@ add_task(function* test_savedPingsOnShut
 add_task(function* test_savedSessionData() {
   // Create the directory which will contain the data file, if it doesn't already
   // exist.
   yield OS.File.makeDir(DATAREPORTING_PATH);
 
   // Write test data to the session data file.
   const dataFilePath = OS.Path.join(DATAREPORTING_PATH, "session-state.json");
   const sessionState = {
-    previousSubsessionId: null,
+    subsessionId: null,
     profileSubsessionCounter: 3785,
   };
   yield CommonUtils.writeJSON(sessionState, dataFilePath);
 
   const PREF_TEST = "toolkit.telemetry.test.pref1";
   Preferences.reset(PREF_TEST);
   const PREFS_TO_WATCH = new Map([
     [PREF_TEST, TelemetryEnvironment.RECORD_PREF_VALUE],
@@ -1204,17 +1204,50 @@ add_task(function* test_savedSessionData
 
   let payload = TelemetrySession.getPayload();
   Assert.equal(payload.info.profileSubsessionCounter, expectedSubsessions);
   yield TelemetrySession.shutdown();
 
   // Load back the serialised session data.
   let data = yield CommonUtils.readJSON(dataFilePath);
   Assert.equal(data.profileSubsessionCounter, expectedSubsessions);
-  Assert.equal(data.previousSubsessionId, expectedUUID);
+  Assert.equal(data.subsessionId, expectedUUID);
+});
+
+add_task(function* test_sessionData_ShortSession() {
+  if (gIsAndroid) {
+    // We don't support subsessions yet on Android, so skip the next checks.
+    return;
+  }
+
+  const SESSION_STATE_PATH = OS.Path.join(DATAREPORTING_PATH, "session-state.json");
+
+  // Shut down Telemetry and remove the session state file.
+  yield TelemetrySession.shutdown();
+  yield OS.File.remove(SESSION_STATE_PATH, { ignoreAbsent: true });
+
+  const expectedUUID = "009fd1ad-b85e-4817-b3e5-000000003785";
+  fakeGenerateUUID(generateUUID, () => expectedUUID);
+
+  // We intentionally don't wait for the setup to complete and shut down to simulate
+  // short sessions. We expect the profile subsession counter to be 1.
+  TelemetrySession.reset();
+  yield TelemetrySession.shutdown();
+
+  // Restore the UUID generation functions.
+  fakeGenerateUUID(generateUUID, generateUUID);
+
+  // Start TelemetrySession so that it loads the session data file. We expect the profile
+  // subsession counter to be incremented by 1 again.
+  yield TelemetrySession.reset();
+
+  // We expect 2 profile subsession counter updates.
+  let payload = TelemetrySession.getPayload();
+  Assert.equal(payload.info.profileSubsessionCounter, 2);
+  Assert.equal(payload.info.previousSubsessionId, expectedUUID);
 });
 
 add_task(function* test_invalidSessionData() {
   // Create the directory which will contain the data file, if it doesn't already
   // exist.
   yield OS.File.makeDir(DATAREPORTING_PATH);
 
   // Write test data to the session data file.
@@ -1233,17 +1266,17 @@ add_task(function* test_invalidSessionDa
   yield TelemetrySession.reset();
   let payload = TelemetrySession.getPayload();
   Assert.equal(payload.info.profileSubsessionCounter, expectedSubsessions);
   yield TelemetrySession.shutdown();
 
   // Load back the serialised session data.
   let data = yield CommonUtils.readJSON(dataFilePath);
   Assert.equal(data.profileSubsessionCounter, expectedSubsessions);
-  Assert.equal(data.previousSubsessionId, null);
+  Assert.equal(data.subsessionId, expectedUUID);
 });
 
 add_task(function* test_abortedSession() {
   if (gIsAndroid || gIsGonk) {
     // We don't have the aborted session ping here.
     return;
   }