Bug 1399796 - UnsubmittedCrashHandler should never check for unsubmitted crash reports if disabled or suppressed. r=Felipe, a=lizzard
authorMike Conley <mconley@mozilla.com>
Fri, 15 Sep 2017 09:25:04 -0700
changeset 666736 09f4708df24cde4ce73032d3196f1593bb259ee4
parent 666735 16a31f23b98a98e4cb843658cd4a3189d0c134f4
child 666737 28c16d0c9746f7e5f4a5d01c21bd3fede6c6e1a5
push id80488
push userbmo:mkelly@mozilla.com
push dateTue, 19 Sep 2017 04:42:30 +0000
reviewersFelipe, lizzard
bugs1399796, 1355492
milestone56.0
Bug 1399796 - UnsubmittedCrashHandler should never check for unsubmitted crash reports if disabled or suppressed. r=Felipe, a=lizzard Bug 1355492 moved the logic for scanning for unsubmitted crash reports out of the initialization of UnsubmittedCrashHandler, and the initialization is what decided whether or not it was appropriate to scan in the first place. This was done so that scanning could be deferred until idle after first paint. This patch makes it so that the scanning logic first ensures that the UnsubmittedCrashHandler is actually enabled and not suppressed (which is calculated earlier). I've also taken the liberty of adding a regression test. MozReview-Commit-ID: 3Aihom5Q17R
browser/modules/ContentCrashHandlers.jsm
browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
--- a/browser/modules/ContentCrashHandlers.jsm
+++ b/browser/modules/ContentCrashHandlers.jsm
@@ -653,16 +653,20 @@ this.UnsubmittedCrashHandler = {
    * bar to prompt the user to submit them.
    *
    * @returns Promise
    *          Resolves with the <xul:notification> after it tries to
    *          show a notification on the most recent browser window.
    *          If a notification cannot be shown, will resolve with null.
    */
   async checkForUnsubmittedCrashReports() {
+    if (!this.enabled || this.suppressed) {
+      return null;
+    }
+
     let dateLimit = new Date();
     dateLimit.setDate(dateLimit.getDate() - PENDING_CRASH_REPORT_DAYS);
 
     let reportIDs = [];
     try {
       reportIDs = await CrashSubmit.pendingIDs(dateLimit);
     } catch (e) {
       Cu.reportError(e);
--- a/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
+++ b/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
@@ -201,16 +201,35 @@ add_task(async function setup() {
   // nsBrowserGlue starts up UnsubmittedCrashHandler automatically
   // so at this point, it is initialized. It's possible that it
   // was initialized, but is preffed off, so it's inert, so we
   // shut it down, make sure it's preffed on, and then restart it.
   // Note that making the component initialize even when it's
   // disabled is an intentional choice, as this allows for easier
   // simulation of startup and shutdown.
   UnsubmittedCrashHandler.uninit();
+
+  // While we're here, let's test that we don't show the notification
+  // if we're disabled and something happens to check for unsubmitted
+  // crash reports.
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["browser.crashReports.unsubmittedCheck.enabled", false],
+    ],
+  });
+
+  await createPendingCrashReports(1);
+
+  notification =
+    await UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+  Assert.ok(!notification, "There should not be a notification");
+
+  clearPendingCrashReports();
+  await SpecialPowers.popPrefEnv();
+
   await SpecialPowers.pushPrefEnv({
     set: [
       ["browser.crashReports.unsubmittedCheck.enabled", true],
     ],
   });
   UnsubmittedCrashHandler.init();
 
   registerCleanupFunction(function() {