Backed out changeset 5382bd66d8f7 (bug 1702509) for causing failures on browser_UnsubmittedCrashHandler.js . CLOSED TREE

browser/actors/PluginParent.jsm

@@ -92,7 +92,7 @@ const PluginManager = {
     }
 
     let { pluginDumpID } = report;
-    CrashSubmit.submit(pluginDumpID, CrashSubmit.SUBMITTED_FROM_CRASH_TAB, {
+    CrashSubmit.submit(pluginDumpID, {
       recordSubmission: true,
       extraExtraKeyVals: keyVals,
     });

browser/base/content/test/tabcrashed/browser_shown.js

@@ -89,7 +89,6 @@ add_task(async function test_default() {
   await crashTabTestHelper(
     {},
     {
-      SubmittedFrom: "CrashedTab",
       Comments: null,
       URL: "",
     }
@@ -102,7 +101,6 @@ add_task(async function test_default() {
 add_task(async function test_just_a_comment() {
   await crashTabTestHelper(
     {
-      SubmittedFrom: "CrashedTab",
       comments: COMMENTS,
     },
     {
@@ -118,7 +116,6 @@ add_task(async function test_just_a_comment() {
 add_task(async function test_send_URL() {
   await crashTabTestHelper(
     {
-      SubmittedFrom: "CrashedTab",
       includeURL: true,
     },
     {
@@ -134,7 +131,6 @@ add_task(async function test_send_URL() {
 add_task(async function test_send_all() {
   await crashTabTestHelper(
     {
-      SubmittedFrom: "CrashedTab",
       includeURL: true,
       comments: COMMENTS,
     },

browser/modules/ContentCrashHandlers.jsm

@@ -412,10 +412,7 @@ var TabCrashHandler = {
         popup: null,
         callback: async () => {
           if (dumpID) {
-            UnsubmittedCrashHandler.submitReports(
-              [dumpID],
-              CrashSubmit.SUBMITTED_FROM_CRASH_TAB
-            );
+            UnsubmittedCrashHandler.submitReports([dumpID]);
           }
           closeAllNotifications();
         },
@@ -490,10 +487,7 @@ var TabCrashHandler = {
       if (UnsubmittedCrashHandler.autoSubmit) {
         let dumpID = this.childMap.get(childID);
         if (dumpID) {
-          UnsubmittedCrashHandler.submitReports(
-            [dumpID],
-            CrashSubmit.SUBMITTED_FROM_AUTO
-          );
+          UnsubmittedCrashHandler.submitReports([dumpID]);
         }
       } else {
         this.sendToTabCrashedPage(browser);
@@ -630,7 +624,7 @@ var TabCrashHandler = {
       extraExtraKeyVals.URL = "";
     }
 
-    CrashSubmit.submit(dumpID, CrashSubmit.SUBMITTED_FROM_CRASH_TAB, {
+    CrashSubmit.submit(dumpID, {
       recordSubmission: true,
       extraExtraKeyVals,
     }).catch(Cu.reportError);
@@ -896,7 +890,7 @@ var UnsubmittedCrashHandler = {
 
     if (reportIDs.length) {
       if (this.autoSubmit) {
-        this.submitReports(reportIDs, CrashSubmit.SUBMITTED_FROM_AUTO);
+        this.submitReports(reportIDs);
       } else if (this.shouldShowPendingSubmissionsNotification()) {
         return this.showPendingSubmissionsNotification(reportIDs);
       }
@@ -1056,7 +1050,7 @@ var UnsubmittedCrashHandler = {
       {
         label: gNavigatorBundle.GetStringFromName("pendingCrashReports.send"),
         callback: () => {
-          this.submitReports(reportIDs, CrashSubmit.SUBMITTED_FROM_INFOBAR);
+          this.submitReports(reportIDs);
           if (onAction) {
             onAction();
           }
@@ -1068,7 +1062,7 @@ var UnsubmittedCrashHandler = {
         ),
         callback: () => {
           this.autoSubmit = true;
-          this.submitReports(reportIDs, CrashSubmit.SUBMITTED_FROM_INFOBAR);
+          this.submitReports(reportIDs);
           if (onAction) {
             onAction();
           }
@@ -1126,17 +1120,20 @@ var UnsubmittedCrashHandler = {
   },
 
   /**
-   * Attempt to submit reports to the crash report server.
+   * Attempt to submit reports to the crash report server. Each
+   * report will have the "SubmittedFromInfobar" annotation set
+   * to "1".
    *
    * @param reportIDs (Array<string>)
    *        The array of reportIDs to submit.
-   * @param submittedFrom (string)
-   *        One of the CrashSubmit.SUBMITTED_FROM_* constants representing
-   *        how this crash was submitted.
    */
-  submitReports(reportIDs, submittedFrom) {
+  submitReports(reportIDs) {
     for (let reportID of reportIDs) {
-      CrashSubmit.submit(reportID, submittedFrom).catch(Cu.reportError);
+      CrashSubmit.submit(reportID, {
+        extraExtraKeyVals: {
+          SubmittedFromInfobar: "1",
+        },
+      }).catch(Cu.reportError);
     }
   },
 };

browser/modules/test/browser/browser_UnsubmittedCrashHandler.js

@@ -95,8 +95,10 @@ function createPendingCrashReports(howMany, accessDate) {
     let promises = [OS.File.setDates(file.path, lastAccessedDate)];
 
     if (contents) {
+      let encoder = new TextEncoder();
+      let array = encoder.encode(contents);
       promises.push(
-        IOUtils.writeUTF8(file.path, contents, {
+        OS.File.writeAtomic(file.path, array, {
           tmpPath: file.path + ".tmp",
         })
       );
@@ -135,12 +137,9 @@ function createPendingCrashReports(howMany, accessDate) {
  *
  * @param reportIDs (Array<string>)
  *        The IDs for the reports that we expect CrashSubmit to have sent.
- * @param extraCheck (Function, optional)
- *        A function that receives the annotations of the crash report and can
- *        be used for checking them
  * @returns Promise
  */
-function waitForSubmittedReports(reportIDs, extraCheck) {
+function waitForSubmittedReports(reportIDs) {
   let promises = [];
   for (let reportID of reportIDs) {
     let promise = TestUtils.topicObserved(
@@ -150,17 +149,24 @@ function waitForSubmittedReports(reportIDs, extraCheck) {
           let propBag = subject.QueryInterface(Ci.nsIPropertyBag2);
           let dumpID = propBag.getPropertyAsAString("minidumpID");
           if (dumpID == reportID) {
-            if (extraCheck) {
-              let extra = propBag.getPropertyAsInterface(
-                "extra",
-                Ci.nsIPropertyBag2
-              );
-
-              extraCheck(extra);
-            }
-
             return true;
           }
+          let extra = propBag.getPropertyAsInterface(
+            "extra",
+            Ci.nsIPropertyBag2
+          );
+          const blockedAnnotations = [
+            "ServerURL",
+            "TelemetryClientId",
+            "TelemetryServerURL",
+            "TelemetrySessionId",
+          ];
+          for (const key of blockedAnnotations) {
+            Assert.ok(
+              !extra.hasKey(key),
+              "The " + key + " annotation should have been stripped away"
+            );
+          }
         }
         return false;
       }
@@ -186,7 +192,7 @@ function waitForIgnoredReports(reportIDs) {
   for (let reportID of reportIDs) {
     let file = dir.clone();
     file.append(reportID + ".dmp.ignore");
-    promises.push(IOUtils.exists(file.path));
+    promises.push(OS.File.exists(file.path));
   }
   return Promise.all(promises);
 }
@@ -355,23 +361,6 @@ add_task(async function test_several_pending() {
  * Tests that the notification can submit a report.
  */
 add_task(async function test_can_submit() {
-  function extraCheck(extra) {
-    const blockedAnnotations = [
-      "ServerURL",
-      "TelemetryClientId",
-      "TelemetryServerURL",
-      "TelemetrySessionId",
-    ];
-    for (const key of blockedAnnotations) {
-      Assert.ok(
-        !extra.hasKey(key),
-        "The " + key + " annotation should have been stripped away"
-      );
-    }
-
-    Assert.equal(extra.get("SubmittedFrom"), "Infobar");
-  }
-
   let reportIDs = await createPendingCrashReports(1);
   let notification = await UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
   Assert.ok(notification, "There should be a notification");
@@ -383,7 +372,8 @@ add_task(async function test_can_submit() {
   );
   // ...which should be the first button.
   let submit = buttons[0];
-  let promiseReports = waitForSubmittedReports(reportIDs, extraCheck);
+
+  let promiseReports = waitForSubmittedReports(reportIDs);
   info("Sending crash report");
   submit.click();
   info("Sent!");
@@ -470,16 +460,6 @@ add_task(async function test_can_submit_always() {
     "The autoSubmit pref should have been set"
   );
 
-  // Create another report
-  reportIDs = await createPendingCrashReports(1);
-  let result = await UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
-
-  // Check that the crash was auto-submitted
-  Assert.equal(result, null, "The notification should not be shown");
-  promiseReports = await waitForSubmittedReports(reportIDs, extra => {
-    Assert.equal(extra.get("SubmittedFrom"), "Auto");
-  });
-
   // And revert back to default now.
   Services.prefs.clearUserPref(pref);
 

toolkit/crashreporter/CrashAnnotations.yaml

@@ -839,16 +839,12 @@ StorageConnectionNotClosed:
     associated with the connection.
   type: string
 
-SubmittedFrom:
+SubmittedFromInfobar:
   description: >
-    This annotation can hold one of the following five values depending on how
-    this crash was submitted by the user:
-    * Auto: the user had opted-in to auto-submission
-    * Infobar: the user clicked on the infobar to submit the crash
-    * AboutCrashes: the user sent the crash from the about:crashes page
-    * CrashedTab: the user sent the crash from a crashed tab page
-    * Client: the user sent the crash using the crash reporter client
-  type: string
+    This annotation is set to 1 when the crash report has been manually
+    submitted from Firefox info-bar or has been automatically submitted after
+    the user opted-in to auto-submission.
+  type: boolean
 
 SystemMemoryUsePercentage:
   description: >

toolkit/crashreporter/CrashSubmit.jsm

@@ -139,7 +139,7 @@ function Submitter(id, recordSubmission, noThrottle, extraExtraKeyVals) {
   this.recordSubmission = recordSubmission;
   this.noThrottle = noThrottle;
   this.additionalDumps = [];
-  this.extraKeyVals = extraExtraKeyVals;
+  this.extraKeyVals = extraExtraKeyVals || {};
   // mimic deferred Promise behavior
   this.submitStatusPromise = new Promise((resolve, reject) => {
     this.resolveSubmitStatusPromise = resolve;
@@ -434,20 +434,11 @@ Submitter.prototype = {
 // ===================================
 // External API goes here
 var CrashSubmit = {
-  // A set of strings representing how a user subnmitted a given crash
-  SUBMITTED_FROM_AUTO: "Auto",
-  SUBMITTED_FROM_INFOBAR: "Infobar",
-  SUBMITTED_FROM_ABOUT_CRASHES: "AboutCrashes",
-  SUBMITTED_FROM_CRASH_TAB: "CrashedTab",
-
   /**
    * Submit the crash report named id.dmp from the "pending" directory.
    *
    * @param id
    *        Filename (minus .dmp extension) of the minidump to submit.
-   * @param submittedFrom
-   *        One of the SUBMITTED_FROM_* constants representing how the
-   *        user submitted this crash.
    * @param params
    *        An object containing any of the following optional parameters:
    *        - recordSubmission
@@ -467,11 +458,11 @@ var CrashSubmit = {
    *  @return a Promise that is fulfilled with the server crash ID when the
    *          submission succeeds and rejected otherwise.
    */
-  submit: function CrashSubmit_submit(id, submittedFrom, params) {
+  submit: function CrashSubmit_submit(id, params) {
     params = params || {};
     let recordSubmission = false;
     let noThrottle = false;
-    let extraExtraKeyVals = {};
+    let extraExtraKeyVals = null;
 
     if ("recordSubmission" in params) {
       recordSubmission = params.recordSubmission;
@@ -485,8 +476,6 @@ var CrashSubmit = {
       extraExtraKeyVals = params.extraExtraKeyVals;
     }
 
-    extraExtraKeyVals.SubmittedFrom = submittedFrom;
-
     let submitter = new Submitter(
       id,
       recordSubmission,

toolkit/crashreporter/client/crashreporter.cpp

@@ -734,7 +734,6 @@ int main(int argc, char** argv) {
     extraData.removeMember("ServerURL");
     extraData.removeMember("StackTraces");
 
-    extraData["SubmittedFrom"] = "Client";
     extraData["Throttleable"] = "1";
 
     // re-set XUL_APP_FILE for xulrunner wrapped apps

toolkit/crashreporter/content/crashes.js

@@ -150,9 +150,7 @@ function showAppropriateSections() {
 function submitPendingReport(reportId, row, button, buttonText, dateFormatter) {
   button.classList.add("submitting");
   document.getElementById("submitAllUnsubmittedReports").disabled = true;
-  CrashSubmit.submit(reportId, CrashSubmit.SUBMITTED_FROM_ABOUT_CRASHES, {
-    noThrottle: true,
-  })
+  CrashSubmit.submit(reportId, { noThrottle: true })
     .then(
       remoteCrashID => {
         document.getElementById("unsubmitted").removeChild(row);

toolkit/crashreporter/test/browser/browser_aboutCrashesResubmit.js

@@ -73,17 +73,11 @@ function check_submit_pending(tab, crashes) {
         0,
         "correctly sent as non-throttleable"
       );
-      Assert.equal(
-        result.SubmittedFrom,
-        "AboutCrashes",
-        "correctly flagged as sent from about:crashes"
-      );
       // we checked these, they're set by the submission process,
       // so they won't be in the "extra" data.
       delete result.upload_file_minidump;
       delete result.memory_report;
       delete result.Throttleable;
-      delete result.SubmittedFrom;
 
       return { id: CrashID, url: CrashURL, result };
     }).then(({ id, url, result }) => {