801925 - Display an informational banner on subsequent crashes

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect, P3)

Description

[Larissa Co [:lco]]

See pg. 6 of the spec: http://people.mozilla.com/~lco/Crash_Reporting_B2G/ for details

Display the Report Banner on subsequent crashes if the user has not indicated that he would like to "always send" or "never send" crash reports. This is equivalent to the choice of being asked whether to send a report every time a crash occurs. 

This banner provides the option for reporting the current crash. Disregarding the banner means that the crash isn't reported.

Comment 1

[:Margaret Leibovic]

Bug 801809 is crating a dialog we'll show users the first time they experience a crash. This bug is about showing a banner on subsequent crashes.

If the user hasn't chosen to always/never submit crash reports, we should also include a "Report" button in the banner.

Comment 3

[:Margaret Leibovic]

Attachment: WIP

I wrote this patch on top of my patch for bug 801809, but I found that things were easier if I moved the pref logic over to Gaia. We were sending a chrome event to Gaia no matter what, so I figured it should just handle all the logic. This also made it easier to deal with the app crashes.

It's annoying that there are two different events for child process crashes, one that gives me the crashID and one that gives me the app info. For now I decided to just hold onto the crashID in shell.js until we get a message from Gaia telling us to submit it, but that feels fragile. Suggestions on how to do this better are welcome.

I opened a new PR with my Gaia changes here:
https://github.com/mozilla-b2g/gaia/pull/6008

Vivien, maybe you can give me feedback on the Gaia changes? :)

Comment 4

[:Margaret Leibovic]

Attachment: patch

This is a cleaner approach that pushes even more of the logic over to Gaia.

I found that I'm always seeing the fatal "mozbrowsererror" event in Gaia before the "ipc:content-shutdown" notification in shell.js, and I'm wondering if I can rely on that. My latest Gaia patch gets the app name while handling the "mozbrowsererror" event, then uses that in the UI when it gets a "handle-crash" chrome event from shell.js. This would solve bug 801810 in the process of fixing this bug.

Comment 5

[Justin Lebar (not reading bugmail)]

> I found that I'm always seeing the fatal "mozbrowsererror" event in Gaia before the "ipc:content-
> shutdown" notification in shell.js, and I'm wondering if I can rely on that.

Fatal mozbrowsererror is triggered by TabParent::ActorDestroy.  ipc:content-shutdown is triggered by ContentParent::ActorDestroy.  Since ContentParent owns TabParent, ISTM that it wouldn't be sane for ContentParent::ActorDestroy to run before TabParent::ActorDestroy, although cjones would know for sure.

Comment 6

[:Margaret Leibovic]

This is a necessary component of the crash reporter UI, which is a P1 feature.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Justin Lebar [:jlebar] from comment #5)
> > I found that I'm always seeing the fatal "mozbrowsererror" event in Gaia before the "ipc:content-
> > shutdown" notification in shell.js, and I'm wondering if I can rely on that.
> 
> Fatal mozbrowsererror is triggered by TabParent::ActorDestroy. 
> ipc:content-shutdown is triggered by ContentParent::ActorDestroy.  Since
> ContentParent owns TabParent, ISTM that it wouldn't be sane for
> ContentParent::ActorDestroy to run before TabParent::ActorDestroy, although
> cjones would know for sure.

That's correct, the ActorDestroy()s are invoked bottom-up.  That's part of IPDL semantics, you can take that to the bank.

Comment 8

[:Margaret Leibovic]

Attachment: patch v2 (keep automatic crash reporting on gecko side)

Fabrice and I discussed this on IRC. It's safer to keep the automatic crash reporting in gecko, in case there's a crash that prevents us from loading Gaia.

I also updated my Gaia changes:
https://github.com/mozilla-b2g/gaia/pull/6008

Comment 9

[:Margaret Leibovic]

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/6008

Comment 10

[:Margaret Leibovic]

Landed the Gecko part:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9057eef5920a

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9057eef5920a

Comment 13

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ff735cd97e04

Comment 14

[Jeni]

Fix Verified on Build 20130103070201