Bug 1166705 - Don't send a saved-session ping when extended Telemetry is off. r=vladan

This also cleans up the pending pings persistance, putting the control of saving them out of TelemetrySession.

toolkit/components/telemetry/TelemetryController.jsm

@@ -235,27 +235,6 @@ this.TelemetryController = Object.freeze({
     return Impl.getCurrentPingData(aSubsession);
   },
 
-  /**
-   * Add the ping to the pending ping list and save all pending pings.
-   *
-   * @param {String} aType The type of the ping.
-   * @param {Object} aPayload The actual data payload for the ping.
-   * @param {Object} [aOptions] Options object.
-   * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
-   *                  id, false otherwise.
-   * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
-   *                  environment data.
-   * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @returns {Promise} A promise that resolves when the pings are saved.
-   */
-  savePendingPings: function(aType, aPayload, aOptions = {}) {
-    let options = aOptions;
-    options.addClientId = aOptions.addClientId || false;
-    options.addEnvironment = aOptions.addEnvironment || false;
-
-    return Impl.savePendingPings(aType, aPayload, options);
-  },
-
   /**
    * Save a ping to disk.
    *
@@ -659,28 +638,6 @@ let Impl = {
     return promise;
   },
 
-  /**
-   * Saves all the pending pings, plus the passed one, to disk.
-   *
-   * @param {String} aType The type of the ping.
-   * @param {Object} aPayload The actual data payload for the ping.
-   * @param {Object} aOptions Options object.
-   * @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
-   *                  false otherwise.
-   * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
-   *                  environment data.
-   * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   *
-   * @returns {Promise} A promise that resolves when all the pings are saved to disk.
-   */
-  savePendingPings: function savePendingPings(aType, aPayload, aOptions) {
-    this._log.trace("savePendingPings - Type " + aType + ", Server " + this._server +
-                    ", aOptions " + JSON.stringify(aOptions));
-
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
-    return TelemetryStorage.savePendingPings(pingData);
-  },
-
   /**
    * Save a ping to disk.
    *

toolkit/components/telemetry/TelemetrySession.jsm

@@ -1620,42 +1620,44 @@ let Impl = {
     cpmm.sendAsyncMessage(MESSAGE_TELEMETRY_PAYLOAD, payload);
   },
 
-  /**
-   * Save both the "saved-session" and the "shutdown" pings to disk.
-   */
-  savePendingPings: function savePendingPings() {
-    this._log.trace("savePendingPings");
+   /**
+    * Save both the "saved-session" and the "shutdown" pings to disk.
+    */
+  saveShutdownPings: Task.async(function*() {
+    this._log.trace("saveShutdownPings");
 
-    if (!IS_UNIFIED_TELEMETRY) {
-      return this.savePendingPingsClassic();
+    if (IS_UNIFIED_TELEMETRY) {
+      try {
+        let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
+
+        let options = {
+          addClientId: true,
+          addEnvironment: true,
+          overwrite: true,
+        };
+        yield TelemetryController.addPendingPing(getPingType(shutdownPayload), shutdownPayload, options);
+      } catch (ex) {
+        this._log.error("saveShutdownPings - failed to submit shutdown ping", ex);
+      }
+     }
+
+    // As a temporary measure, we want to submit saved-session too if extended Telemetry is enabled
+    // to keep existing performance analysis working.
+    if (Telemetry.canRecordExtended) {
+      try {
+        let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
+
+        let options = {
+          addClientId: true,
+          addEnvironment: true,
+        };
+        yield TelemetryController.addPendingPing(getPingType(payload), payload, options);
+      } catch (ex) {
+        this._log.error("saveShutdownPings - failed to submit saved-session ping", ex);
+      }
     }
+  }),
 
-    let options = {
-      addClientId: true,
-      addEnvironment: true,
-      overwrite: true,
-    };
-
-    let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
-    // Make sure we try to save the pending pings, even though we failed saving the shutdown
-    // ping.
-    return TelemetryController.addPendingPing(getPingType(shutdownPayload), shutdownPayload, options)
-                        .then(() => this.savePendingPingsClassic(),
-                              () => this.savePendingPingsClassic());
-  },
-
-  /**
-   * Save the "saved-session" ping and make TelemetryController save all the pending pings to disk.
-   */
-  savePendingPingsClassic: function savePendingPingsClassic() {
-    this._log.trace("savePendingPingsClassic");
-    let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
-    let options = {
-      addClientId: true,
-      addEnvironment: true,
-    };
-    return TelemetryController.savePendingPings(getPingType(payload), payload, options);
-  },
 
   testSaveHistograms: function testSaveHistograms(file) {
     this._log.trace("testSaveHistograms - Path: " + file.path);
@@ -1844,7 +1846,7 @@ let Impl = {
 
       if (Telemetry.isOfficialTelemetry || testing) {
         return Task.spawn(function*() {
-          yield this.savePendingPings();
+          yield this.saveShutdownPings();
           yield this._stateSaveSerializer.flushTasks();
 
           if (IS_UNIFIED_TELEMETRY) {

toolkit/components/telemetry/TelemetryStorage.jsm

@@ -217,16 +217,6 @@ this.TelemetryStorage = {
     return TelemetryStorageImpl.savePing(ping, overwrite);
   },
 
-  /**
-   * Save all pending pings.
-   *
-   * @param {object} sessionPing The additional session ping.
-   * @returns {promise}
-   */
-  savePendingPings: function(sessionPing) {
-    return TelemetryStorageImpl.savePendingPings(sessionPing);
-  },
-
   /**
    * Add a ping to the saved pings directory so that it gets saved
    * and sent along with other pings.
@@ -460,6 +450,7 @@ let TelemetryStorageImpl = {
   shutdown: Task.async(function*() {
     this._shutdown = true;
     yield this._abortedSessionSerializer.flushTasks();
+    yield this.savePendingPings();
     // If the archive cleaning task is running, block on it. It should bail out as soon
     // as possible.
     yield this._archiveCleanTask;
@@ -758,17 +749,12 @@ let TelemetryStorageImpl = {
   /**
    * Save all pending pings.
    *
-   * @param {object} sessionPing The additional session ping.
    * @returns {promise}
    */
-  savePendingPings: function(sessionPing) {
-    let p = pendingPings.reduce((p, ping) => {
-      // Restore the files with the previous pings if for some reason they have
-      // been deleted, don't overwrite them otherwise.
-      p.push(this.savePing(ping, false));
-      return p;}, [this.savePing(sessionPing, true)]);
-
-    pendingPings = [];
+  savePendingPings: function() {
+    let p = [for (ping of pendingPings) this.savePing(ping, false).catch(ex => {
+      this._log.error("savePendingPings - failed to save pending pings.");
+    })];
     return Promise.all(p);
   },