Closed Bug 801925 Opened 8 years ago Closed 8 years ago

Display an informational banner on subsequent crashes

Categories

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

defect

Tracking

(blocking-basecamp:+, firefox18 fixed)

VERIFIED FIXED
blocking-basecamp +
Tracking Status
firefox18 --- fixed

People

(Reporter: lco, Assigned: Margaret)

References

()

Details

Attachments

(2 files, 2 obsolete files)

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.
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.
Assignee: nobody → margaret.leibovic
Depends on: 801809
Summary: As a user, I want to be able to report a subsequent crash on my FirefoxOS phone if I don't ask it to always send or to never send a crash report, so that I can choose whether to send a report or not. → Display an informational banner on subsequent crashes
Duplicate of this bug: 801926
Attached patch WIP (obsolete) — Splinter Review
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? :)
Attachment #675383 - Flags: feedback?(fabrice)
Attachment #675383 - Flags: feedback?(21)
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #675383 - Attachment is obsolete: true
Attachment #675383 - Flags: feedback?(fabrice)
Attachment #675383 - Flags: feedback?(21)
Attachment #675639 - Flags: review?(fabrice)
> 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.
Attachment #675639 - Flags: review?(fabrice) → review+
This is a necessary component of the crash reporter UI, which is a P1 feature.
blocking-basecamp: --- → ?
(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.
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
Attachment #675639 - Attachment is obsolete: true
Attachment #675716 - Flags: review?(fabrice)
Attachment #675716 - Flags: review?(fabrice) → review+
Duplicate of this bug: 801810
blocking-basecamp: ? → +
Priority: -- → P3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Fix Verified on Build 20130103070201
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.