Bug 1245833 - Don't send empty comments or email addresses in content crash reports. r=felipe

browser/base/content/test/general/browser_aboutTabCrashed.js

@@ -125,9 +125,9 @@ function crashTabTestHelper(fieldValues, expectedExtra) {
  */
 add_task(function* test_default() {
   yield crashTabTestHelper({}, {
-    "Comments": "",
+    "Comments": null,
     "URL": "",
-    "Email": "",
+    "Email": null,
   });
 });
 
@@ -140,7 +140,7 @@ add_task(function* test_just_a_comment() {
   }, {
     "Comments": COMMENTS,
     "URL": "",
-    "Email": "",
+    "Email": null,
   });
 });
 
@@ -152,9 +152,9 @@ add_task(function* test_no_email() {
     email: EMAIL,
     emailMe: false,
   }, {
-    "Comments": "",
+    "Comments": null,
     "URL": "",
-    "Email": "",
+    "Email": null,
   });
 });
 
@@ -166,7 +166,7 @@ add_task(function* test_yes_email() {
     email: EMAIL,
     emailMe: true,
   }, {
-    "Comments": "",
+    "Comments": null,
     "URL": "",
     "Email": EMAIL,
   });
@@ -179,9 +179,9 @@ add_task(function* test_send_URL() {
   yield crashTabTestHelper({
     includeURL: true,
   }, {
-    "Comments": "",
+    "Comments": null,
     "URL": PAGE,
-    "Email": "",
+    "Email": null,
   });
 });
 
@@ -200,3 +200,4 @@ add_task(function* test_send_all() {
     "Email": EMAIL,
   });
 });
+

browser/base/content/test/general/head.js

@@ -1149,19 +1149,24 @@ function getPropertyBagValue(bag, key) {
  * been submitted. This function will also test the crash
  * reports extra data to see if it matches expectedExtra.
  *
- * @param expectedExtra
+ * @param expectedExtra (object)
  *        An Object whose key-value pairs will be compared
  *        against the key-value pairs in the extra data of the
  *        crash report. A test failure will occur if there is
  *        a mismatch.
  *
- *        Note that this will only check the values that exist
+ *        If the value of the key-value pair is "null", this will
+ *        be interpreted as "this key should not be included in the
+ *        extra data", and will cause a test failure if it is detected
+ *        in the crash report.
+ *
+ *        Note that this will ignore any keys that are not included
  *        in expectedExtra. It's possible that the crash report
  *        will contain other extra information that is not
  *        compared against.
  * @returns Promise
  */
-function promiseCrashReport(expectedExtra) {
+function promiseCrashReport(expectedExtra={}) {
   return Task.spawn(function*() {
     info("Starting wait on crash-report-status");
     let [subject, data] =
@@ -1200,8 +1205,12 @@ function promiseCrashReport(expectedExtra) {
       let key = enumerator.getNext().QueryInterface(Ci.nsIProperty).name;
       let value = extra.getPropertyAsAString(key);
       if (key in expectedExtra) {
-        is(value, expectedExtra[key],
-           `Crash report had the right extra value for ${key}`);
+        if (expectedExtra[key] == null) {
+          ok(false, `Got unexpected key ${key} with value ${value}`);
+        } else {
+          is(value, expectedExtra[key],
+             `Crash report had the right extra value for ${key}`);
+        }
       }
     }
   });

browser/modules/ContentCrashHandlers.jsm

@@ -163,13 +163,31 @@ this.TabCrashHandler = {
       URL,
     } = message.data;
 
+    let extraExtraKeyVals = {
+      "Comments": comments,
+      "Email": email,
+      "URL": URL,
+    };
+
+    // For the entries in extraExtraKeyVals, we only want to submit the
+    // extra data values where they are not the empty string.
+    for (let key in extraExtraKeyVals) {
+      let val = extraExtraKeyVals[key].trim();
+      if (!val) {
+        delete extraExtraKeyVals[key];
+      }
+    }
+
+    // URL is special, since it's already been written to extra data by
+    // default. In order to make sure we don't send it, we overwrite it
+    // with the empty string.
+    if (!includeURL) {
+      extraExtraKeyVals["URL"] = "";
+    }
+
     CrashSubmit.submit(dumpID, {
       recordSubmission: true,
-      extraExtraKeyVals: {
-        Comments: comments,
-        Email: email,
-        URL: URL,
-      },
+      extraExtraKeyVals,
     }).then(null, Cu.reportError);
 
     this.prefs.setBoolPref("sendReport", true);