Bug 1119281 - Fix missing telemetry client id in saved session pings. r=vladan,a=sledru
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Fri, 09 Jan 2015 22:42:52 +0100
changeset 243708 ced08a06e2d1
parent 243705 e07bd3f1fd4b
child 243709 222d4a9f7ed3
push id4446
push usergeorg.fritzsche@googlemail.com
push date2015-02-09 13:32 +0000
treeherdermozilla-beta@ced08a06e2d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvladan, sledru
bugs1119281
milestone36.0
Bug 1119281 - Fix missing telemetry client id in saved session pings. r=vladan,a=sledru
toolkit/components/telemetry/TelemetryPing.jsm
toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
--- a/toolkit/components/telemetry/TelemetryPing.jsm
+++ b/toolkit/components/telemetry/TelemetryPing.jsm
@@ -190,16 +190,17 @@ this.TelemetryPing = Object.freeze({
     PREF_SERVER: PREF_SERVER,
     PREF_PREVIOUS_BUILDID: PREF_PREVIOUS_BUILDID,
   }),
   /**
    * Used only for testing purposes.
    */
   reset: function() {
     this.uninstall();
+    Impl._clientID = null;
     return this.setup();
   },
   /**
    * Used only for testing purposes.
    */
   setup: function() {
     return Impl.setup(true);
   },
@@ -209,16 +210,22 @@ this.TelemetryPing = Object.freeze({
   uninstall: function() {
     try {
       Impl.uninstall();
     } catch (ex) {
       // Ignore errors
     }
   },
   /**
+   * Used only for testing purposes.
+   */
+  shutdown: function() {
+    return Impl.shutdown(true);
+  },
+  /**
    * Descriptive metadata
    *
    * @param  reason
    *         The reason for the telemetry ping, this will be included in the
    *         returned metadata,
    * @return The metadata as a JS object
    */
   getMetadata: function(reason) {
@@ -945,20 +952,17 @@ let Impl = {
     // id from disk.
     // We try to cache it in prefs to avoid this, even though this may
     // lead to some stale client ids.
     this._clientID = Preferences.get(PREF_CACHED_CLIENTID, null);
 
     AsyncShutdown.sendTelemetry.addBlocker(
       "Telemetry: shutting down",
       function condition(){
-        this.uninstall();
-        if (Telemetry.canSend) {
-          return this.savePendingPings();
-        }
+        return this.shutdown();
       }.bind(this));
 
     Services.obs.addObserver(this, "sessionstore-windows-restored", false);
     Services.obs.addObserver(this, "quit-application-granted", false);
 #ifdef MOZ_WIDGET_ANDROID
     Services.obs.addObserver(this, "application-background", false);
 #endif
     Services.obs.addObserver(this, "xul-window-visible", false);
@@ -1050,17 +1054,16 @@ let Impl = {
     if (this._hasXulWindowVisibleObserver) {
       Services.obs.removeObserver(this, "xul-window-visible");
       this._hasXulWindowVisibleObserver = false;
     }
     Services.obs.removeObserver(this, "quit-application-granted");
 #ifdef MOZ_WIDGET_ANDROID
     Services.obs.removeObserver(this, "application-background", false);
 #endif
-    this._clientID = null;
   },
 
   getPayload: function getPayload() {
     // This function returns the current Telemetry payload to the caller.
     // We only gather startup info once.
     if (Object.keys(this._slowSQLStartup).length == 0) {
       this.gatherStartupHistograms();
       this._slowSQLStartup = Telemetry.slowSQL;
@@ -1170,9 +1173,21 @@ let Impl = {
       break;
 #endif
     }
   },
 
   get clientID() {
     return this._clientID;
   },
+
+  /**
+   * This tells TelemetryPing to uninitialize and save any pending pings.
+   * @param testing Optional. If true, always saves the ping whether Telemetry
+   *                can send pings or not, which is used for testing.
+   */
+  shutdown: function(testing = false) {
+    this.uninstall();
+    if (Telemetry.canSend || testing) {
+      return this.savePendingPings();
+    }
+  },
 };
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ -17,16 +17,17 @@ Cu.import("resource://testing-common/htt
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/LightweightThemeManager.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/TelemetryPing.jsm", this);
 Cu.import("resource://gre/modules/TelemetryFile.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 Cu.import("resource://gre/modules/Promise.jsm", this);
 Cu.import("resource://gre/modules/Preferences.jsm");
+Cu.import("resource://gre/modules/osfile.jsm", this);
 
 const IGNORE_HISTOGRAM = "test::ignore_me";
 const IGNORE_HISTOGRAM_TO_CLONE = "MEMORY_HEAP_ALLOCATED";
 const IGNORE_CLONED_HISTOGRAM = "test::ignore_me_also";
 const ADDON_NAME = "Telemetry test addon";
 const ADDON_HISTOGRAM = "addon-histogram";
 // Add some unicode characters here to ensure that sending them works correctly.
 const FLASH_VERSION = "\u201c1.1.1.1\u201d";
@@ -587,16 +588,30 @@ add_task(function* test_runOldPingFile()
   do_check_true(histogramsFile.exists());
   let mtime = histogramsFile.lastModifiedTime;
   histogramsFile.lastModifiedTime = mtime - (14 * 24 * 60 * 60 * 1000 + 60000); // 14 days, 1m
 
   yield TelemetryPing.testLoadHistograms(histogramsFile);
   do_check_false(histogramsFile.exists());
 });
 
+add_task(function* test_savedSessionClientID() {
+  // Assure that we store the ping properly when saving sessions on shutdown.
+  // We make the TelemetryPings shutdown to trigger a session save.
+  const dir = TelemetryFile.pingDirectoryPath;
+  yield OS.File.removeDir(dir, {ignoreAbsent: true});
+  yield OS.File.makeDir(dir);
+  yield TelemetryPing.shutdown();
+
+  yield TelemetryFile.loadSavedPings();
+  Assert.equal(TelemetryFile.pingsLoaded, 1);
+  let ping = TelemetryFile.popPendingPings().next();
+  Assert.equal(ping.value.payload.clientID, gDataReportingClientID);
+});
+
 add_task(function* stopServer(){
   gHttpServer.stop(do_test_finished);
 });
 
 // An iterable sequence of http requests
 function Request() {
   let defers = [];
   let current = 0;