Closed Bug 550784 Opened 10 years ago Closed 10 years ago

[OOPP] Flash deadlocks during script evals that trigger focus related events

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(2 files)

Attached file stack
This occurs when exiting out of fullscreen through a click on an advertisement. The full screen window is destroyed, and the browser is visible.
Interesting hang. Deferred processing was designed to solve this problem, but it's currently limited to mozilla windows. In this case, what's happening:

1) child creates a full screen popup video window
2) child receives a keydown handle event, posts an event to its window, returns from handle event
3) child makes an eval call to parent with some script, enters waitfornotify
4) parent handles eval, script execution causes a setfocus call on the browser with raise = true
5) parent's setfocus sends sync messages to child popup window
6) child processes these events in waitfornotify's peekmessage calls, delivering them to a flash event procedure
7) child makes ipc calls during event processing, re-enters waitfornotify

This results in a deadlock - parent is stuck in setfocus, child is stuck in waitfornotify, unable to return control to parent.

Two ways to address this afaict, both free the parent thread so it can respond to ipc calls:

1) One-off fix: add certain plugin window classes to the deferred message list. We'd probably need to add other windows for other plugins down the line - silverlight, realplayer, and other windowless video plugins probably have similar issues.  I've confirmed this addresses this hang for flash on hulu.

2) Make generic replymessage calls before entering waitfornotify when the call depth is greater than one. In this particular case, we need to free the parent thread from it's setfocus call. If we've reentered waitfornotify in the child, we know a peek message call is handling sync events that will result in a deadlock. Calling replymessage frees parent so that it can respond to ipc calls made by child.
Try builds, if anyone cares to click on a few ads to test. Seems to be addressed with the patch I'm about to post.

http://build.mozilla.org/tryserver-builds/jmathies@mozilla.com-try-ac780bcdc991
Attached patch fixSplinter Review
I'm not seeing lockups with Flash 10.0.45.2.  The issue from comment 0 looks fixed.  I can click on video from the advertisement from normal and fullscreen mode and it opens a new tab, the video also appears to been paused.  No other bugs appear to have regressed with this either, using Win7 RC.  

I don't know if this fixes issues from comments that Ben X made in bug 550784 so CC'ing him to test try build.
(In reply to comment #4)

> I don't know if this fixes issues from comments that Ben X made in bug 550784
> so CC'ing him to test try build.

oops: make that bug 550322 comment 12.
I've tested the try build. They work on the flash ads in hulu, but it still does not fix the problem that I have, where clicking on another Firefox window while playing a fullscreen flash video causes a browser hang.
(In reply to comment #6)
> I've tested the try build. They work on the flash ads in hulu, but it still
> does not fix the problem that I have, where clicking on another Firefox window
> while playing a fullscreen flash video causes a browser hang.

STR Benjamin? I'm not sure I understand, if you have a full screen window open, how are you clicking on another firefox window? Dual monitors maybe?
Attachment #431480 - Flags: review?(bent.mozilla)
(In reply to comment #5)
> (In reply to comment #4)
> 
> > I don't know if this fixes issues from comments that Ben X made in bug 550784
> > so CC'ing him to test try build.
> 
> oops: make that bug 550322 comment 12.

Sorry, should have read your comments.

Lets file that as a new bug, we'll have to track someone down who can debug that.
Yes I have dual monitors. Did you file the bug yet or shall I?
(In reply to comment #9)
> Yes I have dual monitors. Did you file the bug yet or shall I?

No I haven't - please do, and thanks for helping us test!
Comment on attachment 431480 [details] [diff] [review]
fix

Oh boy, this is a rabbit hole. Let's hope it doesn't go much deeper than this.
Attachment #431480 - Flags: review?(bent.mozilla) → review+
http://hg.mozilla.org/mozilla-central/rev/7d80d8881cbd
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Dual monitor bug filed as bug 551627.
Assignee: nobody → jmathies
You need to log in before you can comment on or make changes to this bug.