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

RESOLVED FIXED in mozilla1.9.3a2

Status

()

Toolkit
Crash Reporting
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Dolske, Assigned: ted)

Tracking

Trunk
mozilla1.9.3a2
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 .4-fixed)

Details

(Whiteboard: [fixed-lorentz])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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

8 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

8 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

8 years ago
Created attachment 422974 [details] [diff] [review]
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.
(Assignee)

Comment 4

8 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

8 years ago
Created attachment 423020 [details] [diff] [review]
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.
Attachment #422974 - Attachment is obsolete: true
Attachment #423020 - Flags: review?(dtownsend)
(Reporter)

Comment 6

8 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

8 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

8 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 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

8 years ago
Pushed http://hg.mozilla.org/mozilla-central/rev/059ca2bbaa7f
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
(Reporter)

Updated

8 years ago
Flags: in-testsuite+

Comment 11

8 years ago
Created attachment 426260 [details] [diff] [review]
Clean up properly, rev. 1
Attachment #426260 - Flags: review+
(Reporter)

Updated

8 years ago
Depends on: 545734

Comment 12

8 years ago
http://hg.mozilla.org/projects/firefox-lorentz/rev/17a8fd8754e9
http://hg.mozilla.org/projects/firefox-lorentz/rev/b1e3467d8fe3
Whiteboard: [fixed-lorentz]
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

8 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.