Closed
Bug 539843
Opened 15 years ago
Closed 15 years ago
Need a mechanism for "plugin crashed" UI to trigger crash report submission
Categories
(Toolkit :: Crash Reporting, defect)
Toolkit
Crash Reporting
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a2
Tracking | Status | |
---|---|---|
status1.9.2 | --- | .4-fixed |
People
(Reporter: Dolske, Assigned: ted)
References
Details
(Whiteboard: [fixed-lorentz])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
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.]
Assignee | ||
Comment 1•15 years ago
|
||
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.
Assignee | ||
Comment 2•15 years ago
|
||
I'm refactoring the guts of crashes.js into a jsm.
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Component: IPC → Breakpad Integration
Product: Core → Toolkit
QA Contact: ipc → breakpad.integration
Assignee | ||
Comment 3•15 years ago
|
||
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.
Assignee | ||
Comment 4•15 years ago
|
||
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.
Assignee | ||
Comment 5•15 years ago
|
||
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.
Attachment #422974 -
Attachment is obsolete: true
Attachment #423020 -
Flags: review?(dtownsend)
Reporter | ||
Comment 6•15 years ago
|
||
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.
Reporter | ||
Comment 7•15 years ago
|
||
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.
Assignee | ||
Comment 8•15 years ago
|
||
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•15 years ago
|
||
Comment on attachment 423020 [details] [diff] [review]
Refactor crashes.js into CrashSubmit.jsm, adapt consumers
This seems to be ok to me.
Attachment #423020 -
Flags: review?(dtownsend) → review+
Reporter | ||
Comment 10•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Reporter | ||
Updated•15 years ago
|
Flags: in-testsuite+
Comment 11•15 years ago
|
||
Updated•15 years ago
|
Attachment #426260 -
Flags: review+
Comment 12•15 years ago
|
||
http://hg.mozilla.org/projects/firefox-lorentz/rev/17a8fd8754e9
http://hg.mozilla.org/projects/firefox-lorentz/rev/b1e3467d8fe3
Whiteboard: [fixed-lorentz]
Comment 13•15 years ago
|
||
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•15 years ago
|
||
Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430
status1.9.2:
--- → .4-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•