From 2e651bf46f36ebeb66e27a73bcd4f8e0aa5d4834 Mon Sep 17 00:00:00 2001 From: Mike Conley Date: Fri, 15 Sep 2017 09:25:04 -0700 Subject: [PATCH] Bug 1399796 - UnsubmittedCrashHandler should never check for unsubmitted crash reports if disabled or suppressed. r=Felipe 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 --HG-- extra : rebase_source : 0430dc2464acfd7b648ba9d4658ab3fb01606570 extra : source : 593ef0951840f01384def383c2690cc90767a118 --- browser/modules/ContentCrashHandlers.jsm | 4 ++++ .../browser_UnsubmittedCrashHandler.js | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/browser/modules/ContentCrashHandlers.jsm b/browser/modules/ContentCrashHandlers.jsm index a1e33dd1a3e9..36e60ede4f03 100644 --- a/browser/modules/ContentCrashHandlers.jsm +++ b/browser/modules/ContentCrashHandlers.jsm @@ -660,6 +660,10 @@ this.UnsubmittedCrashHandler = { * 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); diff --git a/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js b/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js index fcaae8d90322..1f216c991a71 100644 --- a/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js +++ b/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js @@ -206,6 +206,25 @@ add_task(async function setup() { // 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],