Bug 1407631 - Have the UnsubmittedCrashReporter ignore reports that are less than an hour old. r?felipe draft
authorMike Conley <mconley@mozilla.com>
Mon, 16 Oct 2017 16:18:47 -0400
changeset 681016 ca1d036903942a1d8df8717bd74820986c0cd6ea
parent 680782 c6a2643362a67cdf7a87ac165454fce4b383debb
child 736055 bcaabee02844700922a6d0296120c71ac689d00b
push id84723
push usermconley@mozilla.com
push dateMon, 16 Oct 2017 20:19:29 +0000
reviewersfelipe
bugs1407631
milestone58.0a1
Bug 1407631 - Have the UnsubmittedCrashReporter ignore reports that are less than an hour old. r?felipe MozReview-Commit-ID: 3xhIpo4WVcn
browser/modules/ContentCrashHandlers.jsm
browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
toolkit/crashreporter/CrashSubmit.jsm
--- a/browser/modules/ContentCrashHandlers.jsm
+++ b/browser/modules/ContentCrashHandlers.jsm
@@ -31,17 +31,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 
 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 PENDING_CRASH_REPORT_MAX_AGE_DAYS = 28;
+const PENDING_CRASH_REPORT_MIN_AGE_MS = 60 * 60 * 1000; /* ms = 1 hour */
 const DAY = 24 * 60 * 60 * 1000; // milliseconds
 const DAYS_TO_SUPPRESS = 30;
 const MAX_UNSEEN_CRASHED_CHILD_IDS = 20;
 
 /**
  * BrowserWeakMap is exactly like a WeakMap, but expects <xul:browser>
  * objects only.
  *
@@ -644,37 +645,39 @@ this.UnsubmittedCrashHandler = {
       case "profile-before-change": {
         this.uninit();
         break;
       }
     }
   },
 
   /**
-   * 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.
+   * Scans the profile directory for unsubmitted crash reports last accessed
+   * anytime between PENDING_CRASH_REPORT_MAX_AGE_DAYS and
+   * PENDING_CRASH_REPORT_MIN_AGE_MS ago. 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
    *          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 now = new Date();
+    let fromDate = new Date().setDate(now.getDate() - PENDING_CRASH_REPORT_MAX_AGE_DAYS);
+    let toDate = new Date(now.getTime() - PENDING_CRASH_REPORT_MIN_AGE_MS);
 
     let reportIDs = [];
     try {
-      reportIDs = await CrashSubmit.pendingIDs(dateLimit);
+      reportIDs = await CrashSubmit.pendingIDs(fromDate, toDate);
     } catch (e) {
       Cu.reportError(e);
       return null;
     }
 
     if (reportIDs.length) {
       if (this.autoSubmit) {
         this.submitReports(reportIDs);
--- a/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
+++ b/browser/modules/test/browser/browser_UnsubmittedCrashHandler.js
@@ -62,17 +62,19 @@ function clearPendingCrashReports() {
  * @param accessDate (Date, optional)
  *        What date to set as the last accessed time on the created
  *        crash reports. This defaults to the current date and time.
  * @returns Promise
  */
 function createPendingCrashReports(howMany, accessDate) {
   let dir = getPendingCrashReportDir();
   if (!accessDate) {
-    accessDate = new Date();
+    // By default, we'll make the accessDate 2 hours from now. That
+    // way, the crash report will be old enough to not be ignored.
+    accessDate = new Date(Date.now() - (60 * 60 * 1000 * 2) /* ms */);
   }
 
   /**
    * Helper function for creating a file in the pending crash report
    * directory.
    *
    * @param fileName (string)
    *        The filename for the crash report, not including the
@@ -722,8 +724,25 @@ add_task(async function test_end_suppres
   Assert.ok(!UnsubmittedCrashHandler.suppressed,
             "The UnsubmittedCrashHandler should not be suppressed.");
   Assert.ok(!UnsubmittedCrashHandler.prefs.prefHasUserValue("suppressUntilDate"),
             "The suppression date should been cleared from preferences.");
 
   UnsubmittedCrashHandler.uninit();
   UnsubmittedCrashHandler.init();
 });
+
+/**
+ * Tests that if there's a very recent crash, that we don't consider it
+ * when scanning for unsubmitted crashes, since the crash reporter client
+ * might still be sending it.
+ *
+ * For now, we'll only send crash reports that are older than an hour.
+ */
+add_task(async function test_recent_crashes() {
+  await createPendingCrashReports(1, new Date());
+
+  let notification =
+    await UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+  Assert.ok(!notification, "There should not be a notification");
+
+  clearPendingCrashReports();
+});
--- a/toolkit/crashreporter/CrashSubmit.jsm
+++ b/toolkit/crashreporter/CrashSubmit.jsm
@@ -472,23 +472,26 @@ this.CrashSubmit = {
     let [dump /* , extra, memory */] = getPendingMinidump(id);
     let file = await OS.File.open(`${dump}.ignore`, { create: true },
                                   { unixFlags: OS.Constants.libc.O_CREAT });
     await file.close();
   },
 
   /**
    * Get the list of pending crash IDs, excluding those marked to be ignored
-   * @param minFileDate
-   *     A Date object. Any files last modified before that date will be ignored
+   * @param fromDate
+   *     A Date object. Any files last modified before that date will be ignored.
+   * @param toDate
+   *     A Date object. Any files last modified on or after that date will be
+   *     ignored.
    *
    * @return a Promise that is fulfilled with an array of string, each
    *         being an ID as expected to be passed to submit() or ignore()
    */
-  pendingIDs: async function CrashSubmit_pendingIDs(minFileDate) {
+  pendingIDs: async function CrashSubmit_pendingIDs(fromDate, toDate) {
     let ids = [];
     let dirIter = null;
     let pendingDir = getDir("pending");
 
     try {
       dirIter = new OS.File.DirectoryIterator(pendingDir);
     } catch (ex) {
       if (ex.becauseNoSuchFile) {
@@ -528,17 +531,18 @@ this.CrashSubmit = {
           }
         }
       });
 
       for (let entry in entries) {
         let entryInfo = await entries[entry];
 
         if (!(entry in ignored) &&
-            entryInfo.lastAccessDate > minFileDate) {
+            entryInfo.lastAccessDate > fromDate &&
+            entryInfo.lastAccessDate < toDate) {
           ids.push(entry);
         }
       }
     } catch (ex) {
       Cu.reportError(ex);
       throw ex;
     } finally {
       dirIter.close();