Bug 1164007 - Make TelemetrySession._removeAbortedSessionPing correctly catch exceptions. r=gfritzsche a=sledru
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Thu, 21 May 2015 03:10:00 -0400
changeset 275123 7f2d9815d78f84fa7878308a25b9c6c106f2a058
parent 275122 f79a0036add4078437e99b24895ae187b339a313
child 275124 f6d297bd0d03a1e9453576bf9d888c537f6be6b7
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, sledru
bugs1164007
milestone40.0a2
Bug 1164007 - Make TelemetrySession._removeAbortedSessionPing correctly catch exceptions. r=gfritzsche a=sledru
toolkit/components/telemetry/TelemetryStorage.jsm
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
--- a/toolkit/components/telemetry/TelemetryStorage.jsm
+++ b/toolkit/components/telemetry/TelemetryStorage.jsm
@@ -1024,18 +1024,18 @@ let TelemetryStorageImpl = {
       this._log.error("loadAbortedSessionPing - error removing ping", ex)
     }
     return ping;
   }),
 
   removeAbortedSessionPing: function() {
     return this._abortedSessionSerializer.enqueueTask(Task.async(function*() {
       try {
+        yield OS.File.remove(gAbortedSessionFilePath, { ignoreAbsent: false });
         this._log.trace("removeAbortedSessionPing - success");
-        yield OS.File.remove(gAbortedSessionFilePath);
       } catch (ex if ex.becauseNoSuchFile) {
         this._log.trace("removeAbortedSessionPing - no such file");
       } catch (ex) {
         this._log.error("removeAbortedSessionPing - error removing ping", ex)
       }
     }.bind(this)));
   },
 };
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -1358,16 +1358,49 @@ add_task(function* test_abortedSession()
   let request = yield gRequestIterator.next();
   let receivedPing = decodeRequestPayload(request);
   Assert.equal(receivedPing.payload.info.reason, REASON_ABORTED_SESSION);
   Assert.equal(receivedPing.id, abortedSessionPing.id);
 
   yield TelemetrySession.shutdown();
 });
 
+add_task(function* test_abortedSession_Shutdown() {
+  if (gIsAndroid || gIsGonk) {
+    // We don't have the aborted session ping here.
+    return;
+  }
+
+  const ABORTED_FILE = OS.Path.join(DATAREPORTING_PATH, ABORTED_PING_FILE_NAME);
+
+  let schedulerTickCallback = null;
+  let now = fakeNow(2040, 1, 1, 0, 0, 0);
+  // Fake scheduler functions to control aborted-session flow in tests.
+  fakeSchedulerTimer(callback => schedulerTickCallback = callback, () => {});
+  yield TelemetrySession.reset();
+
+  Assert.ok((yield OS.File.exists(DATAREPORTING_PATH)),
+            "Telemetry must create the aborted session directory when starting.");
+
+  // Fake now again so that the scheduled aborted-session save takes place.
+  now = fakeNow(futureDate(now, ABORTED_SESSION_UPDATE_INTERVAL_MS));
+  // The first aborted session checkpoint must take place right after the initialisation.
+  Assert.ok(!!schedulerTickCallback);
+  // Execute one scheduler tick.
+  yield schedulerTickCallback();
+  // Check that the aborted session is due at the correct time.
+  Assert.ok((yield OS.File.exists(ABORTED_FILE)), "There must be an aborted session ping.");
+
+  // Remove the aborted session file and then shut down to make sure exceptions (e.g file
+  // not found) do not compromise the shutdown.
+  yield OS.File.remove(ABORTED_FILE);
+
+  yield TelemetrySession.shutdown();
+});
+
 add_task(function* test_abortedDailyCoalescing() {
   if (gIsAndroid || gIsGonk) {
     // We don't have the aborted session or the daily ping here.
     return;
   }
 
   const ABORTED_FILE = OS.Path.join(DATAREPORTING_PATH, ABORTED_PING_FILE_NAME);