Bug 1119281 - Fix missing telemetry client id in saved session pings. r=vladan
☠☠ backed out by 9739ef7c3d2f ☠ ☠
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Fri, 09 Jan 2015 22:42:52 +0100
changeset 248901 7475b716558d7543e6762b7d61f4be1951cd790e
parent 248900 115690326c5eb417d374f0b0978657662b79a0fd
child 248902 ac811920efd696a27e80ffd15218794169f91e47
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvladan
bugs1119281
milestone37.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 1119281 - Fix missing telemetry client id in saved session pings. r=vladan
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
@@ -209,34 +209,43 @@ 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.setupChromeProcess(true);
   },
+
   /**
    * Used only for testing purposes.
    */
   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
    */
@@ -1020,20 +1029,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();
-        }
+        this.shutdown();
       }.bind(this));
 
     Services.obs.addObserver(this, "sessionstore-windows-restored", false);
 #ifdef MOZ_WIDGET_ANDROID
     Services.obs.addObserver(this, "application-background", false);
 #endif
     Services.obs.addObserver(this, "xul-window-visible", false);
     this._hasWindowRestoredObserver = true;
@@ -1170,17 +1176,16 @@ let Impl = {
     }
     if (this._hasXulWindowVisibleObserver) {
       Services.obs.removeObserver(this, "xul-window-visible");
       this._hasXulWindowVisibleObserver = false;
     }
 #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;
@@ -1303,9 +1308,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";
@@ -616,16 +617,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);
+  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;