Bug 1310703 - Use the ID generated by the crashreporter client when sending a crash ping so that server-side deduplication works correctly; r=Dexter

MozReview-Commit-ID: HfyKTru8QgN

toolkit/components/crashes/CrashManager.jsm

@@ -55,7 +55,9 @@ function parseAndRemoveField(obj, field, parseAsJson = true) {
 
   if (field in obj) {
     if (!parseAsJson) {
-      value = obj[field];
+      // We split extra files on LF characters but Windows-generated ones might
+      // contain trailing CR characters so trim them here.
+      value = obj[field].trim();
     } else {
       try {
         value = JSON.parse(obj[field]);
@@ -638,6 +640,9 @@ this.CrashManager.prototype = Object.freeze({
     let sessionId = parseAndRemoveField(reportMeta, "TelemetrySessionId",
                                         /* parseAsJson */ false);
     let stackTraces = parseAndRemoveField(reportMeta, "StackTraces");
+    // If CrashPingUUID is not present then Telemetry will generate a new UUID
+    let pingId = parseAndRemoveField(reportMeta, "CrashPingUUID",
+                                     /* parseAsJson */ false);
 
     // Filter the remaining annotations to remove privacy-sensitive ones
     reportMeta = this._filterAnnotations(reportMeta);
@@ -657,6 +662,7 @@ this.CrashManager.prototype = Object.freeze({
         addClientId: true,
         addEnvironment: true,
         overrideEnvironment: crashEnvironment,
+        overridePingId: pingId
       }
     );
   },

toolkit/components/telemetry/TelemetryController.jsm

@@ -197,6 +197,8 @@ this.TelemetryController = Object.freeze({
    * @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.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that resolves with the ping id once the ping is stored or sent.
    */
   submitExternalPing(aType, aPayload, aOptions = {}) {
@@ -229,6 +231,8 @@ this.TelemetryController = Object.freeze({
    * @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
    *                  if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
@@ -286,6 +290,8 @@ this.TelemetryController = Object.freeze({
    * @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
    *                  if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
@@ -395,6 +401,8 @@ var Impl = {
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    *
    * @returns {Object} An object that contains the assembled ping data.
    */
@@ -409,7 +417,7 @@ var Impl = {
     // Fill the common ping fields.
     let pingData = {
       type: aType,
-      id: Policy.generatePingId(),
+      id: aOptions.overridePingId || Policy.generatePingId(),
       creationDate: (Policy.now()).toISOString(),
       version: PING_FORMAT_VERSION,
       application: this._getApplicationSection(),
@@ -451,6 +459,8 @@ var Impl = {
    * @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.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   _submitPingLogic: Task.async(function* (aType, aPayload, aOptions) {
@@ -489,6 +499,8 @@ var Impl = {
    * @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.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   submitExternalPing: function send(aType, aPayload, aOptions) {
@@ -534,6 +546,8 @@ var Impl = {
    *                  environment data.
    * @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
@@ -570,6 +584,8 @@ var Impl = {
    *                  environment data.
    * @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
+   * @param {String} [aOptions.overridePingId=null] set to override the
+   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.

toolkit/components/telemetry/tests/unit/head.js

@@ -296,9 +296,18 @@ function setEmptyPrefWatchlist() {
     Cu.import("resource://gre/modules/TelemetryEnvironment.jsm").TelemetryEnvironment;
   return TelemetryEnvironment.onInitialized().then(() => {
     TelemetryEnvironment.testWatchPreferences(new Map());
+
   });
 }
 
+// Generate a UUID, used for the ping ID
+function generateUUID() {
+  let str = Cc["@mozilla.org/uuid-generator;1"]
+              .getService(Ci.nsIUUIDGenerator).generateUUID().toString();
+  // strip {}
+  return str.substring(1, str.length - 1);
+}
+
 if (runningInParent) {
   // Set logging preferences for all the tests.
   Services.prefs.setCharPref("toolkit.telemetry.log.level", "Trace");

toolkit/components/telemetry/tests/unit/test_TelemetryController.js

@@ -35,7 +35,7 @@ const PREF_UNIFIED = PREF_BRANCH + "unified";
 
 var gClientID = null;
 
-function sendPing(aSendClientId, aSendEnvironment) {
+function sendPing(aSendClientId, aSendEnvironment, aOverridePingId) {
   if (PingServer.started) {
     TelemetrySend.setServer("http://localhost:" + PingServer.port);
   } else {
@@ -45,6 +45,7 @@ function sendPing(aSendClientId, aSendEnvironment) {
   let options = {
     addClientId: aSendClientId,
     addEnvironment: aSendEnvironment,
+    overridePingId: aOverridePingId || null,
   };
   return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
 }
@@ -286,6 +287,18 @@ add_task(function* test_pingHasEnvironmentAndClientId() {
   Assert.equal(ping.clientId, gClientID, "The correct clientId must be reported.");
 });
 
+add_task(function* test_pingIdCanBeOverridden() {
+  // Send a ping with an overridden id
+  const myPingId = generateUUID();
+  yield sendPing(/* aSendClientId */ false,
+                 /* aSendEnvironment */ false,
+                 myPingId);
+  let ping = yield PingServer.promiseNextPing();
+  checkPingFormat(ping, TEST_PING_TYPE, false, false);
+
+  Assert.equal(ping.id, myPingId, "The ping id must be the one we provided.");
+});
+
 add_task(function* test_archivePings() {
   let now = new Date(2009, 10, 18, 12, 0, 0);
   fakeNow(now);

toolkit/components/telemetry/tests/unit/test_TelemetrySession.js

@@ -74,12 +74,6 @@ XPCOMUtils.defineLazyGetter(this, "DATAREPORTING_PATH", function() {
 var gClientID = null;
 var gMonotonicNow = 0;
 
-function generateUUID() {
-  let str = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator).generateUUID().toString();
-  // strip {}
-  return str.substring(1, str.length - 1);
-}
-
 function truncateDateToDays(date) {
   return new Date(date.getFullYear(),
                   date.getMonth(),