A flash plugin crashed and the plugin crash UI was shown. It provides an option to enter a comment and to send the URL. Problem: I am unable to enter text into the textarea or even focus it. Unchecking the URL checkbox is not possible either. It is possible to click the question mark icon, send the crash report or reload the page using the given link. I can reproduce this every time I manually kill the flash plugin on Youtube. The problem occurs in safe-mode as well.

I can reproduce this very easily. I think it's important to fix this or we have unusable UI. STR (Windows): * Go to a page with a large Flash applet (http://benjamin.smedbergs.us/tests/test-flash-blackbackground.html) * In the task manager, kill FlashPluginPlugin_*.exe This seems like it might be a focus issue with the XBL; when I try to tab through the elements, the focus gets stuck on the close button. smaug, do you have any clues?

Oh, and to reproduce this in a non-release build you will need to set MOZ_CRASHREPORTER=1 in your environment.

Hey, I figured something out ;-) The key presses are being thrown out because of a check at this location: http://hg.mozilla.org/mozilla-central/annotate/67f2a2816651/content/events/src/nsEventStateManager.cpp#l3510 if (nsEventStatus_eConsumeNoDefault != *aStatus) And the event has mDefaultPrevented being set on it because of this stack: > xul.dll!nsDOMEvent::PreventDefault() Line 431 C++ xul.dll!nsDOMKeyboardEvent::PreventDefault() Line 25 C++ xul.dll!nsPluginInstanceOwner::DispatchKeyToPlugin(aKeyEvent=0x0569a3f0) Line 1834 C++ xul.dll!nsPluginInstanceOwner::HandleEvent(aEvent=0x0569a3f0) Line 1942 C++ johns/smaug: I don't know a lot about event dispatch, but since this plugin is no longer "active" can we call nsObjectLoadingContent::DoStopPlugin -> nsPluginInstanceOwner::Destroy as soon as the instance crashes?

I don't understand the question since I have no idea what nsObjectLoadingContent::DoStopPlugin -> nsPluginInstanceOwner::Destroy do. But sounds like plugin stuff shouldn't call preventDefault() in this case.

Try: https://tbpl.mozilla.org/?tree=Try&rev=f987a244d468

Comment on attachment 717250 [details] [diff] [review] Call StopPluginInstance from nsPluginCrashedEvent, and deal with ordering issues that result from that, rev. 1 Review of attachment 717250 [details] [diff] [review]: ----------------------------------------------------------------- I don't think this is correct. PluginCrashed is called by pluginhost here: http://dxr.mozilla.org/mozilla-central/dom/plugins/base/nsPluginHost.cpp#l3680 Which then destroys the instance. This is why OBJLC simply drops its reference to the plugin. As the plugin is already presumably stopped, Destroy() shouldn't be a reentrance risk, so we might be able to just re-order the calls in pluginhost.

Added Image of the "Adobe Flash Crash Reporter Dialog". On Firefox Aurora 21.0a2 the "Adobe Crash Reporter's" "Comment Box" is non-clickable (as is the "Privacy Issue" of the "Include the page's URL"). The (?) is clickable and takes you to this URL: http://support.mozilla.org/en-US/kb/send-plugin-crash-reports-help-improve-firefox?redirectlocale=en-US&as=u&redirectslug=Plugin+crash+reports&utm_source=inproduct and fortunately the [Send crash report] Button works (as does the "Reload the page" Link). Thus, the 'Plugin Reporter Area' is not completely 'dead' but has a bit of functionality.

I would add "privacy" to the "Keywords" area (see prior comment 8), but for some reason I do not have permission (and I've been here more than a few years); otherwise I would make the change myself. Thanks whomever. I also can not change "Whiteboard", "QA Contact", "Depends On", "Blocks", "See also" or "Crash Signature", etc. -- a few weeks ago I did have permission to change all those things. Thanks (again) if that is also fixed.

johns, can you take this? I can't figure out a way to get the sequencing right without this ordering, but I'm not 100% sure about what was objectionable about this sequencing.

Calling StopPluginInstance at all is wrong in this case, the plugin is already dead. It calls into PluginHost to stop the mid-destruction plugin. Doing it in an event further complicates ObjectLoadingContent's crazy state changing and re-entrance mess. Ideally, we just want to throw out mInstanceOwner - but we must kill its frame first, which causes XBL to touch the dead plugin's prototype. Previously, for crashes, were just letting the ObjectFrame incidentally remove itself from the instanceowner and never properly calling Destroy(). This also breaks my patches in bug 784131. This applies on top of the patch for bug 851378, which will let us throw out the prototype earlier. I haven't had a chance to test this yet (linux doesn't have this UI) so there might be more issues. See also bug 767635 for cleaning up the start/stop plugin code further.

Comment on attachment 725218 [details] [diff] [review] Fix plugin instance owner teardown Review of attachment 725218 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsObjectLoadingContent.cpp @@ +2553,5 @@ > > + // This can/will re-enter > + DoStopPlugin(ownerGrip, delayedStop); > + > + ownerGrip->Destroy(); Arrgh, this will break delayed stop. Which is probably already broken. We should probably just tear it out until(if) bug 818785 lands

johns, is this something which we can land on aurora? It seems that bug 851378 is pretty risky, and not something that we'd normally think of taking on Aurora, and I really don't want to have to back out the comment/URL UI, I'm looking for something that solves the issue with minimum risk.

Looks good on try now, despite orange factor https://tbpl.mozilla.org/?tree=Try&rev=dea28098a353 (In reply to Benjamin Smedberg [:bsmedberg] from comment #13) > johns, is this something which we can land on aurora? It seems that bug > 851378 is pretty risky, and not something that we'd normally think of taking > on Aurora, and I really don't want to have to back out the comment/URL UI, > I'm looking for something that solves the issue with minimum risk. I'm not sure if 851378 is too risky for aurora - it's mainly just moving a block of code between files and changing ordering - the latter being the far riskier part, but messing with ordering is what this bug is all about unfortunately :-/ The comment 5 approach leaves us in |mType == Plugin| state post-crash, and leaves us open to events on the loop doing bad things -- including touching the dead plugin prototype or trying to respawn the plugin - we'd have to add some kind of mPluginCrashing flag and riddle checks around the class, which is probably easier to get wrong than 851378

(In reply to John Schoenick [:johns] from comment #12) > Arrgh, this will break delayed stop. Which is probably already broken. We > should probably just tear it out until(if) bug 818785 lands Nevermind, StopPluginRunnable keeps this reference itself, it doesn't look back at the class for it.

Comment on attachment 725218 [details] [diff] [review] Fix plugin instance owner teardown This does seem to fix the issue for me on windows

Ack, move InstanceOwner->Destroy() into DoStopPlugin so it is not called early for delayed stop...

Rebase on updated bug 851378

Comment on attachment 726952 [details] [diff] [review] Fix plugin instance owner teardown. r=bsmedberg https://hg.mozilla.org/integration/mozilla-inbound/rev/cc1be1a068fb

Comment on attachment 726952 [details] [diff] [review] Fix plugin instance owner teardown. r=bsmedberg [Approval Request Comment] Note: This requires bug 851378 also be uplifted, which is where most of the significant changes are. Bug caused by (feature/regressing bug #): N/A, bug was not relevant until advent of plugin crashed UI User impact if declined: Cannot interact with comment field in plugin crashed UI, which would need to be disabled on aurora w/o this patch. Testing completed (on m-c, etc.): On m-c Risk to taking this patch (and alternatives if risky): Very low, bug 851378 has most of the significant changes. String or UUID changes made by this patch: None

I neglected to rev the nsIObjectLoadingContent IID, followup landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9bc9047e04d

[I was waiting to get some stability on nightly before uplifting on aurora] As discussed in crash-kill meeting today, there were no new plugin related stability issues on nightly coinciding the timing of landing of the patch here & 851378 .Approving the uplift for aurora & requesting QA verification here.

I rebased these for aurora, but 21 lacks the webidl conversion for object tags, so I'm going to wait for a try run before I burn the tree: https://tbpl.mozilla.org/?tree=Try&rev=bf80ea01d352

I can confirm that the ability to type a comment in the plugin crash reporter is now fixed. However, I'm not seeing the comment make it through to my crash report: https://crash-stats.mozilla.com/report/index/7488846e-85cb-4124-af45-62b9a2130325 For this report I entered a comment of "here's a test comment" but the User Comment field appears to be empty. Should I file a new bug on that or is that something this bug should address?

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #26) > I can confirm that the ability to type a comment in the plugin crash > reporter is now fixed. However, I'm not seeing the comment make it through > to my crash report: > https://crash-stats.mozilla.com/report/index/7488846e-85cb-4124-af45- > 62b9a2130325 > > For this report I entered a comment of "here's a test comment" but the User > Comment field appears to be empty. Should I file a new bug on that or is > that something this bug should address? This would be a new bug

Confirmed that I can enter comments in Aurora 21.0a2 2013-03-28.