Bug 1407631 - Wait about 10 minutes after browser session start before scanning for unsubmitted crash reports. r=gsvelto
authorMike Conley <mconley@mozilla.com>
Tue, 24 Oct 2017 16:55:24 -0400
changeset 388100 188ff8b2f7f6d7336bc830ff805b1d3e84ee9901
parent 388099 edc52ff233c42294140b0e22381f7f37cd30c345
child 388101 297434b40ea359a53a735396c1b8854480a5b3ff
push id96545
push useracraciun@mozilla.com
push dateWed, 25 Oct 2017 09:37:39 +0000
treeherdermozilla-inbound@060e68fcb4aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto
bugs1407631
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -1107,19 +1107,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: [