Bug 1407631 - Wait about 10 minutes after browser session start before scanning for unsubmitted crash reports. r?gsvelto draft
authorMike Conley <mconley@mozilla.com>
Tue, 24 Oct 2017 16:55:24 -0400
changeset 685566 c579223f6f262717645f49d571377dfe98d90be8
parent 683444 7ca5c2de815431caaba0e5b4c8d660f34c0e3f74
child 737186 a096f9e1ede1ec236db77eeeb26c5313955eb9f7
push id85973
push usermconley@mozilla.com
push dateTue, 24 Oct 2017 21:01:13 +0000
reviewersgsvelto
bugs1407631
milestone58.0a1
Bug 1407631 - Wait about 10 minutes after browser session start before scanning for unsubmitted crash reports. r?gsvelto While the crash reporter client is submitting a crash report, the report itself stays in the crashes directory. We suspect that in some cases, if the browser starts up while the crash reporter client is still sending the report, the unsubmitted crash report handler will also attempt to send the same report. This patch makes the unsubmitted crash report handler wait approximately 10 minutes after the session starts before doing the unsubmitted crash report scan. MozReview-Commit-ID: KkrPDa1Qwv1
browser/components/nsBrowserGlue.js
browser/modules/ContentCrashHandlers.jsm
browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1104,19 +1104,17 @@ BrowserGlue.prototype = {
 
     // It's important that SafeBrowsing is initialized reasonably
     // early, so we use a maximum timeout for it.
     Services.tm.idleDispatchToMainThread(() => {
       SafeBrowsing.init();
     }, 5000);
 
     if (AppConstants.MOZ_CRASHREPORTER) {
-      Services.tm.idleDispatchToMainThread(() => {
-        UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
-      });
+      UnsubmittedCrashHandler.scheduleCheckForUnsubmittedCrashReports();
     }
 
     if (AppConstants.platform == "win") {
       Services.tm.idleDispatchToMainThread(() => {
         // For Windows 7, initialize the jump list module.
         const WINTASKBAR_CONTRACTID = "@mozilla.org/windows-taskbar;1";
         if (WINTASKBAR_CONTRACTID in Cc &&
             Cc[WINTASKBAR_CONTRACTID].getService(Ci.nsIWinTaskbar).available) {
--- a/browser/modules/ContentCrashHandlers.jsm
+++ b/browser/modules/ContentCrashHandlers.jsm
@@ -23,29 +23,36 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "RemotePages",
   "resource://gre/modules/RemotePageManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionStore",
   "resource:///modules/sessionstore/SessionStore.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
   "resource:///modules/RecentWindow.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
   "resource://gre/modules/PluralForm.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "setTimeout",
+  "resource://gre/modules/Timer.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "clearTimeout",
+  "resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "gNavigatorBundle", function() {
   const url = "chrome://browser/locale/browser.properties";
   return Services.strings.createBundle(url);
 });
 
 // We don't process crash reports older than 28 days, so don't bother
 // submitting them
 const PENDING_CRASH_REPORT_DAYS = 28;
 const DAY = 24 * 60 * 60 * 1000; // milliseconds
 const DAYS_TO_SUPPRESS = 30;
 const MAX_UNSEEN_CRASHED_CHILD_IDS = 20;
 
+// Time after which we will begin scanning for unsubmitted crash reports
+const CHECK_FOR_UNSUBMITTED_CRASH_REPORTS_DELAY_MS = 60 * 10000; // 10 minutes
+
 /**
  * BrowserWeakMap is exactly like a WeakMap, but expects <xul:browser>
  * objects only.
  *
  * Under the hood, BrowserWeakMap keys the map off of the <xul:browser>
  * permanentKey. If, however, the browser has never gotten a permanentKey,
  * it falls back to keying on the <xul:browser> element itself.
  */
@@ -580,16 +587,18 @@ this.UnsubmittedCrashHandler = {
   showingNotification: false,
   // suppressed is true if we've determined that we've shown
   // the notification too many times across too many days without
   // user interaction, so we're suppressing the notification for
   // some number of days. See the documentation for
   // shouldShowPendingSubmissionsNotification().
   suppressed: false,
 
+  _checkTimeout: null,
+
   init() {
     if (this.initialized) {
       return;
     }
 
     this.initialized = true;
 
     // UnsubmittedCrashHandler can be initialized but still be disabled.
@@ -616,16 +625,21 @@ this.UnsubmittedCrashHandler = {
 
   uninit() {
     if (!this.initialized) {
       return;
     }
 
     this.initialized = false;
 
+    if (this._checkTimeout) {
+      clearTimeout(this._checkTimeout);
+      this._checkTimeout = null;
+    }
+
     if (!this.enabled) {
       return;
     }
 
     if (this.suppressed) {
       this.suppressed = false;
       // No need to do any more clean-up, since we were suppressed.
       return;
@@ -643,16 +657,24 @@ this.UnsubmittedCrashHandler = {
     switch (topic) {
       case "profile-before-change": {
         this.uninit();
         break;
       }
     }
   },
 
+  scheduleCheckForUnsubmittedCrashReports() {
+    this._checkTimeout = setTimeout(() => {
+      Services.tm.idleDispatchToMainThread(() => {
+        this.checkForUnsubmittedCrashReports();
+      });
+    }, CHECK_FOR_UNSUBMITTED_CRASH_REPORTS_DELAY_MS);
+  },
+
   /**
    * Scans the profile directory for unsubmitted crash reports
    * within the past PENDING_CRASH_REPORT_DAYS days. If it
    * finds any, it will, if necessary, attempt to open a notification
    * bar to prompt the user to submit them.
    *
    * @returns Promise
    *          Resolves with the <xul:notification> after it tries to
--- a/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
+++ b/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
@@ -194,21 +194,27 @@ add_task(async function setup() {
   }
 
   let env = Cc["@mozilla.org/process/environment;1"]
               .getService(Components.interfaces.nsIEnvironment);
   let oldServerURL = env.get("MOZ_CRASHREPORTER_URL");
   env.set("MOZ_CRASHREPORTER_URL", SERVER_URL);
 
   // 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
+  // on a timer, so at this point, it can be in one of several states:
+  //
+  // 1. The timer hasn't yet finished, and an automatic scan for crash
+  //    reports is pending.
+  // 2. The timer has already gone off and the scan has already completed.
+  // 3. The handler is disabled.
+  //
+  // To collapse all of these possibilities, we uninit the UnsubmittedCrashHandler
+  // to cancel the timer, make sure it's preffed on, and then restart it (which
+  // doesn't restart the timer). 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: [