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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jim Mathies [:jimm]]

Attachment: 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.

Comment 1

[Jim Mathies [:jimm]]

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.

Comment 2

[Jim Mathies [:jimm]]

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/[e-mail]-try-ac780bcdc991

Comment 3

[Jim Mathies [:jimm]]

Attachment: fix

Comment 4

[[not reading bugmail]]

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.

Comment 5

[[not reading bugmail]]

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

Comment 6

[Benjamin Xiao]

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.

Comment 7

[Jim Mathies [:jimm]]

(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?

Comment 8

[Jim Mathies [:jimm]]

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

Comment 9

[Benjamin Xiao]

Yes I have dual monitors. Did you file the bug yet or shall I?

Comment 10

[Jim Mathies [:jimm]]

(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 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 12

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/7d80d8881cbd

Comment 13

[Benjamin Xiao]

Dual monitor bug filed as bug 551627.