Bug 1301722 - Unsubmitted crash report notification should go away if not interacted with for some period of time. r?florian
MozReview-Commit-ID: JxTmmesyTYd
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1512,9 +1512,14 @@ pref("webchannel.allowObject.urlWhitelis
// crash reports, and then show a notification for submitting
// those reports.
#ifdef RELEASE_BUILD
pref("browser.crashReports.unsubmittedCheck.enabled", false);
#else
pref("browser.crashReports.unsubmittedCheck.enabled", true);
#endif
+// chancesUntilSuppress is how many times we'll show the unsubmitted
+// crash report notification across different days and shutdown
+// without a user choice before we suppress the notification for
+// some number of days.
+pref("browser.crashReports.unsubmittedCheck.chancesUntilSuppress", 4);
pref("browser.crashReports.unsubmittedCheck.autoSubmit", false);
\ No newline at end of file
--- a/browser/modules/ContentCrashHandlers.jsm
+++ b/browser/modules/ContentCrashHandlers.jsm
@@ -2,16 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
var Cc = Components.classes;
var Ci = Components.interfaces;
var Cu = Components.utils;
+var Cr = Components.results;
this.EXPORTED_SYMBOLS = [ "TabCrashHandler",
"PluginCrashReporter",
"UnsubmittedCrashHandler" ];
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
@@ -33,16 +34,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 DAY = 24 * 60 * 60 * 1000; // milliseconds
+const DAYS_TO_SUPPRESS = 30;
this.TabCrashHandler = {
_crashedTabCount: 0,
get prefs() {
delete this.prefs;
return this.prefs = Services.prefs.getBranch("browser.tabs.crashReporting.");
},
@@ -339,39 +342,105 @@ this.TabCrashHandler = {
/**
* This component is responsible for scanning the pending
* crash report directory for reports, and (if enabled), to
* prompt the user to submit those reports. It might also
* submit those reports automatically without prompting if
* the user has opted in.
*/
this.UnsubmittedCrashHandler = {
+ get prefs() {
+ delete this.prefs;
+ return this.prefs =
+ Services.prefs.getBranch("browser.crashReports.unsubmittedCheck.");
+ },
+
+ // showingNotification is set to true once a notification
+ // is successfully shown, and then set back to false if
+ // the notification is dismissed by an action by the user.
+ 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,
+
init() {
if (this.initialized) {
return;
}
this.initialized = true;
- let pref = "browser.crashReports.unsubmittedCheck.enabled";
- let shouldCheck = Services.prefs.getBoolPref(pref);
+ let shouldCheck = this.prefs.getBoolPref("enabled");
if (shouldCheck) {
+ if (this.prefs.prefHasUserValue("suppressUntilDate")) {
+ if (this.prefs.getCharPref("suppressUntilDate") > this.dateString()) {
+ // We'll be suppressing any notifications until after suppressedDate,
+ // so there's no need to do anything more.
+ this.suppressed = true;
+ return;
+ }
+
+ // We're done suppressing, so we don't need this pref anymore.
+ this.prefs.clearUserPref("suppressUntilDate");
+ }
+
Services.obs.addObserver(this, "browser-delayed-startup-finished",
false);
+ Services.obs.addObserver(this, "profile-before-change",
+ false);
}
},
- observe(subject, topic, data) {
- if (topic != "browser-delayed-startup-finished") {
+ uninit() {
+ if (!this.initialized) {
+ return;
+ }
+
+ this.initialized = false;
+
+ if (this.suppressed) {
+ this.suppressed = false;
+ // No need to do any more clean-up, since we were suppressed.
return;
}
- Services.obs.removeObserver(this, topic);
- this.checkForUnsubmittedCrashReports();
+ if (this.showingNotification) {
+ this.prefs.setBoolPref("shutdownWhileShowing", true);
+ this.showingNotification = false;
+ }
+
+ try {
+ Services.obs.removeObserver(this, "browser-delayed-startup-finished");
+ } catch (e) {
+ // The browser-delayed-startup-finished observer might have already
+ // fired and removed itself, so if this fails, it's okay.
+ if (e.result != Cr.NS_ERROR_FAILURE) {
+ throw e;
+ }
+ }
+
+ Services.obs.removeObserver(this, "profile-before-change");
+ },
+
+ observe(subject, topic, data) {
+ switch (topic) {
+ case "browser-delayed-startup-finished": {
+ Services.obs.removeObserver(this, topic);
+ this.checkForUnsubmittedCrashReports();
+ break;
+ }
+ 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.
*
@@ -390,24 +459,72 @@ this.UnsubmittedCrashHandler = {
} catch (e) {
Cu.reportError(e);
return null;
}
if (reportIDs.length) {
if (CrashNotificationBar.autoSubmit) {
CrashNotificationBar.submitReports(reportIDs);
- } else {
+ } else if (this.shouldShowPendingSubmissionsNotification()) {
return this.showPendingSubmissionsNotification(reportIDs);
}
}
return null;
}),
/**
+ * Returns true if the notification should be shown.
+ * shouldShowPendingSubmissionsNotification makes this decision
+ * by looking at whether or not the user has seen the notification
+ * over several days without ever interacting with it. If this occurs
+ * too many times, we suppress the notification for DAYS_TO_SUPPRESS
+ * days.
+ *
+ * @returns bool
+ */
+ shouldShowPendingSubmissionsNotification() {
+ if (!this.prefs.prefHasUserValue("shutdownWhileShowing")) {
+ return true;
+ }
+
+ let shutdownWhileShowing = this.prefs.getBoolPref("shutdownWhileShowing");
+ this.prefs.clearUserPref("shutdownWhileShowing");
+
+ if (!this.prefs.prefHasUserValue("lastShownDate")) {
+ // This isn't expected, but we're being defensive here. We'll
+ // opt for showing the notification in this case.
+ return true;
+ }
+
+ let lastShownDate = this.prefs.getCharPref("lastShownDate");
+ if (this.dateString() > lastShownDate && shutdownWhileShowing) {
+ // We're on a newer day then when we last showed the
+ // notification without closing it. We don't want to do
+ // this too many times, so we'll decrement a counter for
+ // this situation. Too many of these, and we'll assume the
+ // user doesn't know or care about unsubmitted notifications,
+ // and we'll suppress the notification for a while.
+ let chances = this.prefs.getIntPref("chancesUntilSuppress");
+ if (--chances < 0) {
+ // We're out of chances!
+ this.prefs.clearUserPref("chancesUntilSuppress");
+ // We'll suppress for DAYS_TO_SUPPRESS days.
+ let suppressUntil =
+ this.dateString(new Date(Date.now() + (DAY * DAYS_TO_SUPPRESS)));
+ this.prefs.setCharPref("suppressUntilDate", suppressUntil);
+ return false;
+ }
+ this.prefs.setIntPref("chancesUntilSuppress", chances);
+ }
+
+ return true;
+ },
+
+ /**
* Given an array of unsubmitted crash report IDs, try to open
* up a notification asking the user to submit them.
*
* @param reportIDs (Array<string>)
* The Array of report IDs to offer the user to send.
* @returns The <xul:notification> if one is shown. null otherwise.
*/
showPendingSubmissionsNotification(reportIDs) {
@@ -416,21 +533,44 @@ this.UnsubmittedCrashHandler = {
return null;
}
let messageTemplate =
gNavigatorBundle.GetStringFromName("pendingCrashReports2.label");
let message = PluralForm.get(count, messageTemplate).replace("#1", count);
- return CrashNotificationBar.show({
+ let notification = CrashNotificationBar.show({
notificationID: "pending-crash-reports",
message,
reportIDs,
+ onAction: () => {
+ this.showingNotification = false;
+ },
});
+
+ if (notification) {
+ this.showingNotification = true;
+ this.prefs.setCharPref("lastShownDate", this.dateString());
+ }
+
+ return notification;
+ },
+
+ /**
+ * Returns a string representation of a Date in the format
+ * YYYYMMDD.
+ *
+ * @param someDate (Date, optional)
+ * The Date to convert to the string. If not provided,
+ * defaults to today's date.
+ * @returns String
+ */
+ dateString(someDate = new Date()) {
+ return someDate.toLocaleFormat("%Y%m%d");
},
};
this.CrashNotificationBar = {
/**
* Attempts to show a notification bar to the user in the most
* recent browser window asking them to submit some crash report
* IDs. If a notification cannot be shown (for example, there
@@ -447,19 +587,25 @@ this.CrashNotificationBar = {
* notificationID (string)
* The ID for the notification to be opened.
*
* message (string)
* The message to be displayed in the notification.
*
* reportIDs (Array<string>)
* The array of report IDs to offer to the user.
+ *
+ * onAction (function, optional)
+ * A callback to fire once the user performs an
+ * action on the notification bar (this includes
+ * dismissing the notification).
+ *
* @returns The <xul:notification> if one is shown. null otherwise.
*/
- show({ notificationID, message, reportIDs }) {
+ show({ notificationID, message, reportIDs, onAction }) {
let chromeWin = RecentWindow.getMostRecentBrowserWindow();
if (!chromeWin) {
// Can't show a notification in this case. We'll hopefully
// get another opportunity to have the user submit their
// crash reports later.
return null;
}
@@ -468,23 +614,29 @@ this.CrashNotificationBar = {
if (notification) {
return null;
}
let buttons = [{
label: gNavigatorBundle.GetStringFromName("pendingCrashReports.send"),
callback: () => {
this.submitReports(reportIDs);
+ if (onAction) {
+ onAction();
+ }
},
},
{
label: gNavigatorBundle.GetStringFromName("pendingCrashReports.alwaysSend"),
callback: () => {
this.autoSubmit = true;
this.submitReports(reportIDs);
+ if (onAction) {
+ onAction();
+ }
},
},
{
label: gNavigatorBundle.GetStringFromName("pendingCrashReports.viewAll"),
callback: function() {
chromeWin.openUILinkIn("about:crashes", "tab");
return true;
},
@@ -494,16 +646,19 @@ this.CrashNotificationBar = {
if (eventType == "dismissed") {
// The user intentionally dismissed the notification,
// which we interpret as meaning that they don't care
// to submit the reports. We'll ignore these particular
// reports going forward.
reportIDs.forEach(function(reportID) {
CrashSubmit.ignore(reportID);
});
+ if (onAction) {
+ onAction();
+ }
}
};
return nb.appendNotification(message, notificationID,
"chrome://browser/skin/tab-crashed.svg",
nb.PRIORITY_INFO_HIGH, buttons,
eventCallback);
},
--- a/browser/modules/test/browser_UnsubmittedCrashHandler.js
+++ b/browser/modules/test/browser_UnsubmittedCrashHandler.js
@@ -193,16 +193,22 @@ add_task(function* setup() {
notification.close();
}
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);
+ yield SpecialPowers.pushPrefEnv({
+ set: [
+ ["browser.crashReports.unsubmittedCheck.enabled", true],
+ ],
+ });
+
registerCleanupFunction(function() {
gNotificationBox = null;
clearPendingCrashReports();
env.set("MOZ_CRASHREPORTER_URL", oldServerURL);
});
});
/**
@@ -412,8 +418,249 @@ add_task(function* test_can_ignore() {
yield waitForIgnoredReports(reportIDs);
notification =
yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
Assert.equal(notification, null, "There should be no notification");
clearPendingCrashReports();
});
+
+/**
+ * Tests that if the notification is shown, then the
+ * lastShownDate is set for today.
+ */
+add_task(function* test_last_shown_date() {
+ yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should be a notification");
+
+ let today = new Date().toLocaleFormat("%Y%m%d");
+ let lastShownDate =
+ UnsubmittedCrashHandler.prefs.getCharPref("lastShownDate");
+ Assert.equal(today, lastShownDate,
+ "Last shown date should be today.");
+
+ UnsubmittedCrashHandler.prefs.clearUserPref("lastShownDate");
+ gNotificationBox.removeNotification(notification, true);
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if UnsubmittedCrashHandler is uninit with a
+ * notification still being shown, that
+ * browser.crashReports.unsubmittedCheck.shutdownWhileShowing is
+ * set to true.
+ */
+add_task(function* test_shutdown_while_showing() {
+ yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should be a notification");
+
+ UnsubmittedCrashHandler.uninit();
+ let shutdownWhileShowing =
+ UnsubmittedCrashHandler.prefs.getBoolPref("shutdownWhileShowing");
+ Assert.ok(shutdownWhileShowing,
+ "We should have noticed that we uninitted while showing " +
+ "the notification.");
+ UnsubmittedCrashHandler.prefs.clearUserPref("shutdownWhileShowing");
+ UnsubmittedCrashHandler.init();
+
+ gNotificationBox.removeNotification(notification, true);
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if UnsubmittedCrashHandler is uninit after
+ * the notification has been closed, that
+ * browser.crashReports.unsubmittedCheck.shutdownWhileShowing is
+ * not set in prefs.
+ */
+add_task(function* test_shutdown_while_not_showing() {
+ let reportIDs = yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should be a notification");
+
+ // Dismiss the notification by clicking on the "X" button.
+ let anonyNodes = document.getAnonymousNodes(notification)[0];
+ let closeButton = anonyNodes.querySelector(".close-icon");
+ closeButton.click();
+ yield waitForIgnoredReports(reportIDs);
+
+ UnsubmittedCrashHandler.uninit();
+ Assert.throws(() => {
+ let shutdownWhileShowing =
+ UnsubmittedCrashHandler.prefs.getBoolPref("shutdownWhileShowing");
+ }, "We should have noticed that the notification had closed before " +
+ "uninitting.");
+ UnsubmittedCrashHandler.init();
+
+ gNotificationBox.removeNotification(notification, true);
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if
+ * browser.crashReports.unsubmittedCheck.shutdownWhileShowing is
+ * set and the lastShownDate is today, then we don't decrement
+ * browser.crashReports.unsubmittedCheck.chancesUntilSuppress.
+ */
+add_task(function* test_dont_decrement_chances_on_same_day() {
+ let initChances =
+ UnsubmittedCrashHandler.prefs.getIntPref("chancesUntilSuppress");
+ Assert.ok(initChances > 1, "We should start with at least 1 chance.");
+
+ yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should be a notification");
+
+ UnsubmittedCrashHandler.uninit();
+
+ gNotificationBox.removeNotification(notification, true);
+
+ let shutdownWhileShowing =
+ UnsubmittedCrashHandler.prefs.getBoolPref("shutdownWhileShowing");
+ Assert.ok(shutdownWhileShowing,
+ "We should have noticed that we uninitted while showing " +
+ "the notification.");
+
+ let today = new Date().toLocaleFormat("%Y%m%d");
+ let lastShownDate =
+ UnsubmittedCrashHandler.prefs.getCharPref("lastShownDate");
+ Assert.equal(today, lastShownDate,
+ "Last shown date should be today.");
+
+ UnsubmittedCrashHandler.init();
+
+ notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should still be a notification");
+
+ let chances =
+ UnsubmittedCrashHandler.prefs.getIntPref("chancesUntilSuppress");
+
+ Assert.equal(initChances, chances,
+ "We should not have decremented chances.");
+
+ gNotificationBox.removeNotification(notification, true);
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if
+ * browser.crashReports.unsubmittedCheck.shutdownWhileShowing is
+ * set and the lastShownDate is before today, then we decrement
+ * browser.crashReports.unsubmittedCheck.chancesUntilSuppress.
+ */
+add_task(function* test_decrement_chances_on_other_day() {
+ let initChances =
+ UnsubmittedCrashHandler.prefs.getIntPref("chancesUntilSuppress");
+ Assert.ok(initChances > 1, "We should start with at least 1 chance.");
+
+ yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should be a notification");
+
+ UnsubmittedCrashHandler.uninit();
+
+ gNotificationBox.removeNotification(notification, true);
+
+ let shutdownWhileShowing =
+ UnsubmittedCrashHandler.prefs.getBoolPref("shutdownWhileShowing");
+ Assert.ok(shutdownWhileShowing,
+ "We should have noticed that we uninitted while showing " +
+ "the notification.");
+
+ // Now pretend that the notification was shown yesterday.
+ let yesterday = new Date(Date.now() - DAY).toLocaleFormat("%Y%m%d");
+ UnsubmittedCrashHandler.prefs.setCharPref("lastShownDate", yesterday);
+
+ UnsubmittedCrashHandler.init();
+
+ notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.ok(notification, "There should still be a notification");
+
+ let chances =
+ UnsubmittedCrashHandler.prefs.getIntPref("chancesUntilSuppress");
+
+ Assert.equal(initChances - 1, chances,
+ "We should have decremented our chances.");
+ UnsubmittedCrashHandler.prefs.clearUserPref("chancesUntilSuppress");
+
+ gNotificationBox.removeNotification(notification, true);
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if we've shutdown too many times showing the
+ * notification, and we've run out of chances, then
+ * browser.crashReports.unsubmittedCheck.suppressUntilDate is
+ * set for some days into the future.
+ */
+add_task(function* test_can_suppress_after_chances() {
+ // Pretend that a notification was shown yesterday.
+ let yesterday = new Date(Date.now() - DAY).toLocaleFormat("%Y%m%d");
+ UnsubmittedCrashHandler.prefs.setCharPref("lastShownDate", yesterday);
+ UnsubmittedCrashHandler.prefs.setBoolPref("shutdownWhileShowing", true);
+ UnsubmittedCrashHandler.prefs.setIntPref("chancesUntilSuppress", 0);
+
+ yield createPendingCrashReports(1);
+ let notification =
+ yield UnsubmittedCrashHandler.checkForUnsubmittedCrashReports();
+ Assert.equal(notification, null,
+ "There should be no notification if we've run out of chances");
+
+ // We should have set suppressUntilDate into the future
+ let suppressUntilDate =
+ UnsubmittedCrashHandler.prefs.getCharPref("suppressUntilDate");
+
+ let today = new Date().toLocaleFormat("%Y%m%d");
+ Assert.ok(suppressUntilDate > today,
+ "We should be suppressing until some days into the future.");
+
+ UnsubmittedCrashHandler.prefs.clearUserPref("chancesUntilSuppress");
+ UnsubmittedCrashHandler.prefs.clearUserPref("suppressUntilDate");
+ UnsubmittedCrashHandler.prefs.clearUserPref("lastShownDate");
+ clearPendingCrashReports();
+});
+
+/**
+ * Tests that if there's a suppression date set, then no notification
+ * will be shown even if there are pending crash reports.
+ */
+add_task(function* test_suppression() {
+ let future = new Date(Date.now() + (DAY * 5)).toLocaleFormat("%Y%m%d");
+ UnsubmittedCrashHandler.prefs.setCharPref("suppressUntilDate", future);
+ UnsubmittedCrashHandler.uninit();
+ UnsubmittedCrashHandler.init();
+
+ Assert.ok(UnsubmittedCrashHandler.suppressed,
+ "The UnsubmittedCrashHandler should be suppressed.");
+ UnsubmittedCrashHandler.prefs.clearUserPref("suppressUntilDate");
+
+ UnsubmittedCrashHandler.uninit();
+ UnsubmittedCrashHandler.init();
+});
+
+/**
+ * Tests that if there's a suppression date set, but we've exceeded
+ * it, then we can show the notification again.
+ */
+add_task(function* test_end_suppression() {
+ let yesterday = new Date(Date.now() - DAY).toLocaleFormat("%Y%m%d");
+ UnsubmittedCrashHandler.prefs.setCharPref("suppressUntilDate", yesterday);
+ UnsubmittedCrashHandler.uninit();
+ UnsubmittedCrashHandler.init();
+
+ 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();
+});