Closed Bug 519541 Opened 11 years ago Closed 11 years ago

Plugins: Need a "plugin crashed" notification

Categories

(Core :: IPC, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- .4-fixed

People

(Reporter: cjones, Assigned: jaas)

References

Details

(Whiteboard: [notacrash][fixed-lorentz])

Attachments

(1 file, 1 obsolete file)

There are two parts to this bug: first, our C++ library code needs to generate a browser-script-observable event when a plugin crashes.

Second, when a plugin crashes, browser script needs to replace all the plugin's instances in all tabs with some kind of "crashed" UI.  I'm not a UI guy, but we might want the UI to have a link to a bug-reporting mechanism for known plugins along with an option to reload the plugin's object/embed frame.
Do we even have the IPDL mechanism for notifying actors when the other side crashed? Lets get some of that filed and marked blocking this bug.
I think we'll want to tie this in to crash reporting somehow, since we'll want to be able to get crash reports from crashed plugin processes.
What Ted said - it would be really really good if we not only had crashing url source pages but the url of the specific plugin that crashed.  Super-handy for partners and ourselves.
See discussion in bug 525849, going to make that one specifically about submitting crash reports from crashed plugins.
Blocks: 525849
No longer depends on: 525849
Blocks: 531142
Depends on: 532400
Whiteboard: [notacrash]
alimi says:

* The first time a plugin crashes, we should drop down a doorhanger.

* The doorhanger should have a checkbox, on by default, for submitting the crash report, and a text, something like "Adobe Flash has crashed." and a reload button.

* If the user reloads in any way (doorhanger, toolbar, or Ctrl-R), we should submit the crash report.

* Any subsequent times the plugin crashes, we should check to see
** is the plugin large enough to display a notification directly within it
** and visible

* If the plugin notification can be displayed directly within the plugin frame, we should do so. If not, we should drop down the doorhanger.

alimi, does this match our conversation?
Depends on: 533891
Doorhanger notifications won't be available on 1.9.2, I'm pretty sure.
Maybe I'm not using the right terminology. What's the thing which drops down which we use for popup-blocked notifications, save-my-password, etc? That's what I thought limi intended.
That's a notification bar.
Using a notification bar to start with and transiting it over later to a door hanger is the best approach since we want to get this done quickly.
This patch will get the content node to send a "PluginCrashed" notification, in response we can hook up an XBL binding with whatever crash UI we want.
Attachment #420782 - Flags: review?(jst)
The way the current notifications work is that we attach XBL using CSS with special -moz-* pseudo-classes:

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/plugins/content/missingPluginBinding.css#39

So I think we'll also need to add another state to nsObjectLoadingContent::ObjectState() (and nsIEventStateManager.h /nsCSSPseudoClassList.h ?), unless we can re-use one of the existing states (don't think we can).
(In reply to comment #11)
> The way the current notifications work is that we attach XBL using CSS with
> special -moz-* pseudo-classes:

Sorry, I misspoke - the notifications are triggered by the event (for which your patch should be sufficient), but the "click on the plugin in the page to do stuff" behavior is handled by the XBL, and we'd want the pseudo-class for that.
This bug is going to get confusing if we do everything here...

I'm going to morph this into just "need a plugin crashed" notification (since that's the patch that's present), and then clone this bug for the UI side of things.
Summary: Plugins: Need a "plugin crashed" UI → Plugins: Need a "plugin crashed" notification
No longer blocks: 535078
Since this is branch-bound, can we avoid changing nsIObjectLoadingContent and do nsIObjectLoadingContent2?
Assignee: nobody → joshmoz
Comment on attachment 420782 [details] [diff] [review]
send "PluginCrashed" notification, v1.0

- In nsPluginHost::PluginCrashed():

+      if (domElement) {
+        nsCOMPtr<nsIObjectLoadingContent> objectContent(do_QueryInterface(domElement));

do_QueryInterface() is null safe, so no need to explicitly check domElement there.

r=jst
Attachment #420782 - Flags: review?(jst) → review+
(In reply to comment #5)
> alimi, does this match our conversation?

Yeah, pretty much. Happy to sync up on wording etc once you have a running version of this, since it depends on how much space we have at our disposal. But; let's keep it short and simple. :)
Attachment #420782 - Attachment is obsolete: true
Attachment #421385 - Flags: review?(benjamin)
Comment on attachment 421385 [details] [diff] [review]
send "PluginCrashed" notification, v1.1 (w/test)

We're definitely going to have to attach additional data to that event so that we can hook up crash reporting, but that can be done in a separate bug... it will involve some end-to-end management with IPDL and crashreporter and the UI.
Attachment #421385 - Flags: review?(benjamin) → review+
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/9c10583d0b66
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
No longer blocks: 525849
Depends on: 540532
No longer depends on: 540532
Flags: in-litmus?
Litmus tests I think I'd like:

* killing mozilla-runtime shows the UI, *without* a "crash report was submitted" link
* the link on the crash report UI goes to a SUMO article
* on Linux, sending SIGSEGV to the mozilla-runtime instance should show the crash report UI *with* a "crash report was submitted" link

Note that we should probably wait for bug 550293 which will change the auto-submit behavior.
http://hg.mozilla.org/projects/firefox-lorentz/rev/d23f37a00fb5
Whiteboard: [notacrash] → [notacrash][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
You need to log in before you can comment on or make changes to this bug.