Enable unsubmitted crash notification for more users

RESOLVED FIXED in Firefox 51

Status

()

Toolkit
Breakpad Integration
RESOLVED FIXED
7 months ago
2 months ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

49 Branch
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

So we landed bug 1287178, which adds a "Send Always" button to the unsubmitted crash report notification. Then we landed bug 1301722, which will suppress the notification for 30 days if it's not interacted with over several days.

Are we at a point where we're comfortable turning this notification on for more users? I know that blassey is interested in getting this up to _at least_ Beta. Do we want this to go out to Release too?

The notification is activated by the browser.crashReports.unsubmittedCheck.enabled pref, set here: http://searchfox.org/mozilla-central/rev/515bca4fca5e2e8233ed30e6b6d48fcab8497dbf/browser/app/profile/firefox.js#1515 (currently only enabled for Nightly and Aurora).
The questions are:

1) Are we at a point where we're happy to expose this notification bar to more users in order to get those crash reports?
2) Which channels should get the notification?
Flags: needinfo?(shorlander)
Hey blassey,

I understand we want this on at least Beta due to some sandboxing stuff that's going to ride the trains. Is there a bug for that work this should be attached to?
Depends on: 1287178, 1301723, 1301722
Flags: needinfo?(blassey.bugs)
redirecting to jimm. I thought we wanted this in 51, not beta
Flags: needinfo?(blassey.bugs) → needinfo?(jmathies)
Sorry, to clarify what I said in comment 2:

Currently, this notification is held back to just Nightly and Dev Edition users. I understand that we want this on when 51 reaches Beta due to some sandboxing stuff that's going to ride the trains. Is that true? Is the idea also for the notification to reach release channel users when 51 goes out on release?
Yes, I think if the UX has been sorted out we want this to ride the trains so we can start collecting data about hidden crashes on all branches.

Comment 6

7 months ago
Sandboxing isn't blocked on or in need of this reporting.

I think product has to sign off on this UI rolling out to beta/release. Personally I'm not a fan of having this out on release since it tends to show up a lot. Beta sounds like a good idea though.
Flags: needinfo?(jmathies)
(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> The questions are:
> 
> 1) Are we at a point where we're happy to expose this notification bar to
> more users in order to get those crash reports?
> 2) Which channels should get the notification?

With the improvements allowing people to always send and the auto-dismiss I think this will be fine for beta. 

This is kind of a band-aid fix for us to get these crash reports. I don't see this as something we would show release users.
Flags: needinfo?(shorlander)
(In reply to Jim Mathies [:jimm] from comment #6)
> Sandboxing isn't blocked on or in need of this reporting.
> 
> I think product has to sign off on this UI rolling out to beta/release.
> Personally I'm not a fan of having this out on release since it tends to
> show up a lot. Beta sounds like a good idea though.

The point of Mike's work is to make it only show once assuming users will then click always or never submit.

(In reply to Stephen Horlander [:shorlander] from comment #7)
> (In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> > The questions are:
> > 
> > 1) Are we at a point where we're happy to expose this notification bar to
> > more users in order to get those crash reports?
> > 2) Which channels should get the notification?
> 
> With the improvements allowing people to always send and the auto-dismiss I
> think this will be fine for beta. 
> 
> This is kind of a band-aid fix for us to get these crash reports. I don't
> see this as something we would show release users.

band-aid implies that we think this will be fixed in some other, better way in the future, but I'm unaware of what that other work is. Can you link me to a bug?
Flags: needinfo?(shorlander)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #8)
> band-aid implies that we think this will be fixed in some other, better way
> in the future, but I'm unaware of what that other work is. Can you link me
> to a bug?

I think we could (probably should) rethink how the crash experience works. It feels more like an experience of accumulated parts at this point rather than a planned experience. 

A result of that could be a better way to present these choices that we would feel comfortable shipping to everyone. It's also possible that we would still want to segment what UI to show users based on release vs. pre-release channels.

There is no existing initiative to do that work that I am aware of though.

As it stands though I don't think we should be showing this notification bar to general release users because: 1) it highlights Firefox crashiness as a problem when it's mostly an invisible problem to users; 2) we already have an abundance of notification bars that we inundate people with; 3) I am comfortable showing this to pre-release users because the idea of "testing" is (in theory) built in, not so much with release users
Flags: needinfo?(shorlander)
(In reply to Stephen Horlander [:shorlander] from comment #9)
> As it stands though I don't think we should be showing this notification bar
> to general release users because: 1) it highlights Firefox crashiness as a
> problem when it's mostly an invisible problem to users; 2) we already have
> an abundance of notification bars that we inundate people with; 3) I am
> comfortable showing this to pre-release users because the idea of "testing"
> is (in theory) built in, not so much with release users

For clarity, is our beta population included when you say "pre-release" users? In other words (sorry if I'm making you repeat yourself, I'm just trying to be absolutely sure) - can we enable this notification for users on Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev Edition?
Flags: needinfo?(shorlander)
(In reply to Mike Conley (:mconley) - (high latency on reviews and needinfos) from comment #10)
> (In reply to Stephen Horlander [:shorlander] from comment #9)
> > As it stands though I don't think we should be showing this notification bar
> > to general release users because: 1) it highlights Firefox crashiness as a
> > problem when it's mostly an invisible problem to users; 2) we already have
> > an abundance of notification bars that we inundate people with; 3) I am
> > comfortable showing this to pre-release users because the idea of "testing"
> > is (in theory) built in, not so much with release users
> 
> For clarity, is our beta population included when you say "pre-release"
> users? In other words (sorry if I'm making you repeat yourself, I'm just
> trying to be absolutely sure) - can we enable this notification for users on
> Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev
> Edition?

Yes, by "pre-release" I was referring to Beta, Nightly and Dev Edition.
Flags: needinfo?(shorlander)
Comment hidden (mozreview-request)
Alright. Looking at firefox.js, I don't see us turning a lot of things off just for release. We have EARLY_BETA_OR_EARLIER, which turns on a feature for early betas only, and shuts it off for later betas. I understand that's to make the later betas that we test to be as close to what users receive as possible.

So the patch above uses that same convention and will turn the unsubmitted crash report notification for early betas or earlier.

Comment 14

6 months ago
mozreview-review
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

https://reviewboard.mozilla.org/r/88372/#review87452
Attachment #8804372 - Flags: review?(felipc) → review+
Thanks felipe.

ni?'ing myself to request uplift to Aurora.
Flags: needinfo?(mconley)

Comment 16

6 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07ae41e73a6f
Enable unsubmitted crash notification for more users. r=Felipe

Comment 17

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/07ae41e73a6f
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

Approval Request Comment
[Feature/regressing bug #]:

Unsubmitted crash report notification.

[User impact if declined]:

Users that run early betas will not have the opportunity to submit backlogged crash reports to Mozilla.

[Describe test coverage new/current, TreeHerder]:

This feature has been enabled on Nightly and Aurora for a while, but we want to turn it on for early betas for the next uplift. UX is fine with that, now that we automatically hide the notification if the user doesn't interact with it after a number of sessions.

[Risks and why]: 

Low risk. Just a pref flip that we can flip back if needs be.

[String/UUID change made/needed]:

None.
Flags: needinfo?(mconley)
Attachment #8804372 - Flags: approval-mozilla-aurora?

Updated

6 months ago
status-firefox51: --- → affected
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

Enable unsubmitted crash notification for early beta. Take it in 51 aurora.
Attachment #8804372 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm hitting a conflict uplifting this to aurora, could we get a rebased patch?
Flags: needinfo?(mconley)
Created attachment 8806456 [details] [diff] [review]
Rebased for aurora

MozReview-Commit-ID: F1B0QA8a8wX
Attachment #8806456 - Attachment description: Enable unsubmitted crash notification for more users → Rebased for aurora
Flags: needinfo?(mconley)
(In reply to Wes Kocher (:KWierso) from comment #20)
> I'm hitting a conflict uplifting this to aurora, could we get a rebased
> patch?

Thanks for letting me know. Does this one work better?
Flags: needinfo?(wkocher)
Where "this one" is attachment 8806456 [details] [diff] [review]

Comment 24

6 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/b128f5deb5e4
status-firefox51: affected → fixed
Appears so, thanks.
Flags: needinfo?(wkocher)

Updated

5 months ago
Assignee: nobody → mconley
This change (together with bug 1282776) will make some dashboards and automated notification systems a bit unhappy twice for each cycle (at the beginning of the cycle we'll always see spikes, at the middle of the cycle we'll always see volume reductions).

Comment 27

4 months ago
how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still active in 51.0b7)
Flags: needinfo?(mconley)
(In reply to [:philipp] from comment #27)
> how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still
> active in 51.0b7)

Normally EARLY_BETA_OR_EARLIER is shut off around b5, but 51 has an extended beta cycle for a variety of reasons. The flag was recently unset in a commit landed earlier today (https://hg.mozilla.org/releases/mozilla-beta/rev/d946c3ef0da36339cf3f3988547f4c6aebea37ae), so the next beta (b8) should not have the mechanism enabled.
Flags: needinfo?(mconley)

Comment 29

2 months ago
Hi Mike, Brad, enabling infobar crash submissions during early Beta cycle is making it difficult for relman to compare crash rates during early Beta cycle with past Beta cycles. Is this a necessary change? I was under the impression that we would leave this on during Nightly and Aurora cycle and not in Beta cycle. Is there such a significant benefit here? Relman team would recommend disabling this for the entire Beta cycle if possible.
Flags: needinfo?(mconley)
Flags: needinfo?(blassey.bugs)
As I said at the channel meeting, there are classes of crashes that will go unreported without the info bar. I don't know why we would purposefully not collect that data.
Flags: needinfo?(blassey.bugs)
Why is it only enabled for half the beta cycle? Enabling it for the entire beta cycle would solve the issue of crash rates changing suddenly at the middle and at the beginning of the cycle.
(In reply to Marco Castelluccio [:marco] from comment #31)
> Why is it only enabled for half the beta cycle? Enabling it for the entire
> beta cycle would solve the issue of crash rates changing suddenly at the
> middle and at the beginning of the cycle.

If I recall correctly, it's because the later betas are supposed to be as close as possible to what we finally ship as release.
Flags: needinfo?(mconley)
You need to log in before you can comment on or make changes to this bug.