539843 - Need a mechanism for "plugin crashed" UI to trigger crash report submission

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Justin Dolske [:Dolske]]

Bug 538910 is implementing the UI for informing the user that a plugin has crashed, and allowing them to submit a crash report (or not). This code needs to call something to actually submit a report, not sure what that API is going to be.

[Maybe this bug belongs in Toolkit :: Breakpad Integration? Feel free to move.]

Comment 1

[(not currently active) Ted Mielczarek]

bsmedberg's code from bug 539048 just repurposes crashes.js from about:crashes, which is probably the right way to go. We could factor it out into a jsm or something, but the current impl uses an iframe with a <form> to submit so I didn't have to write my own multipart/form-data implementation.

Comment 2

[(not currently active) Ted Mielczarek]

I'm refactoring the guts of crashes.js into a jsm.

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: WIP CrashSubmit.jsm

This works, but my browser-chrome test fails if I run with --autorun. If I run Mochitest browser-chrome without --autorun, it passes, and if I bump the setTimeout inside the harness from 0 to 2000, it passes, so there's got to be something fishy going on at startup.

Comment 4

[(not currently active) Ted Mielczarek]

On further review, I see that same problem without this patch. I think it's just a browser-chrome harness problem. I'll file a separate bug.

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: Refactor crashes.js into CrashSubmit.jsm, adapt consumers

dolske: This should do what you need, feel free to comment if you think it's missing something. This pulls the bulk of crashes.js (the script behind about:crashes) into CrashSubmit.jsm, which provides one external API:
CrashSubmit.submit(id, element, submitSuccess, submitFail)

There's javadoc in the .jsm, so I won't go into it in detail here. "id" is exactly what you get in the event from my patch in bug 541076 (although we'll need to sort out where the dumps wind up, currently they're in $profile/minidumps, but CrashSubmit.jsm assumes 'id' is in Crash Reports/pending. I'll get that fixed.)

Note that I "hg cp"ed crashes.js to CrashSubmit.jsm, which is why the patch looks the way it does. I also didn't add any new unit tests, browser_aboutCrashesResubmit.js already exercises this code (and passes). I'd love to add an xpcshell test for this, but since it still relies on an iframe for submission, I think that'd be painful.

Comment 6

[Justin Dolske [:Dolske]]

Ugh. I think there are a few problems with the current approach (beyond just this patch).

Right now everything is driven by the PluginCrashed event bubbling up from a content plugin instance to the <browser>. But since there can be multiple events fired from a crash, something has to take steps to deal with only submitting the report once. Each instance could try blindly submitting it, but that seems really inefficient (could involve a fair chunk of disk IO checking submission prefs and looking for a minidump).

I think what we should do is have some singleton take care of submitting the report, and then the PluginCrashed events can just deal with hooking up the UI to show what happened. Probably first fire a notification for an observer in the nsBrowserGlue component, and have it pass back a flag saying what happened to be included in the PluginCrashed notifications.

But this leads to a couple other problems/questions:

1) Since we'd be observing from a component, we're not going to have a handy place to stick an <iframe> for this API. Can the submission be done with an XHR, or would it be less pain to enumerate windows, and pick one to jam an iframe into (hacky)? [And who does it, nsBrowserGlue or this JSM?]

2) How to handle failed submissions? If it fails, can we still just say a report was submitted (perhaps with slightly vague wording), and rely on it being retried retried (does it do that)? This would have the benefit of not having to wait around for success/failure or updating all the content-UI after the fact to show what happened.

Comment 7

[Justin Dolske [:Dolske]]

Just to followup on the last comment... Bug 541076 adds an observer to take care of notifying the browser *once* per plugin crash, and in bug I have that observer calling CrashSubmit.jsm with the last browser window as a host for the parasitic <iframe> needed for submission. So, I think we're all good for now.

Comment 8

[(not currently active) Ted Mielczarek]

Ok, good, I've been thinking about this for a while, trying to figure out answers to your questions in comment 6, and coming up with nothing. Hopefully someone will implement bug 528524 at some point and we can stop using an iframe, but this should get us through for now.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 423020 [details] [diff] [review]
Refactor crashes.js into CrashSubmit.jsm, adapt consumers

This seems to be ok to me.

Comment 10

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/059ca2bbaa7f

Comment 11

[Benjamin Smedberg]

Attachment: Clean up properly, rev. 1

Comment 12

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/17a8fd8754e9
http://hg.mozilla.org/projects/firefox-lorentz/rev/b1e3467d8fe3

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed

Comment 14

[Benjamin Smedberg]

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430