541362 - [OOPP] hang closing tab with Yahoo web messenger loaded

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[(not currently active) Ted Mielczarek]

Attachment: Parent and child stacks

If I log in to Yahoo web messenger (see URL), and then close the tab, Firefox hangs. Messenger pops up one of those beforeunload "are you sure you want to close the tab" dialogs normally, but here the hang occurs before I see the dialog. If I kill the mozilla-runtime process, I get the dialog, and then I can close the tab. This is 100% reproducible for me. Attached are stacks from the Firefox and mozilla-runtime process at the time of the hang.

Comment 1

[(not currently active) Ted Mielczarek]

I think to reproduce this you need to have an active flash object in another tab as well.

Comment 2

[(not currently active) Ted Mielczarek]

Yeah, 100% reproducible for me with the following steps:
1) Open http://mavra.perilith.com/~luser/flash.html in a tab
2) Open a new tab, load http://webmessenger.yahoo.com
3) Login to messenger
4) Close messenger tab

Comment 3

[Benjamin Smedberg]

I think this is a dup of bug 538918, essentially, but I'll let jimm make the call.

Comment 4

[Jim Mathies [:jimm]]

(In reply to comment #3)
> I think this is a dup of bug 538918, essentially, but I'll let jimm make the
> call.

Looks a little different, in bug 538918 a dialog in a windowless control in the child process calls NtUserSetFocus when it closes. I don't see that in this stack. This child's last call was to CallNPN_GetValue_NPNVWindowNPObject. The parent is then calling NtUserSetFocus on something. Related, but not the same test case.

Comment 5

[Benjamin Smedberg]

* Parent calls nsFocusManager::Blur on the plugin.
* Child gets a sync message WM_KILLFOCUS
* Child (Flash) calls an IPDL message (CallNPN_GetValue_NPNVWindowNPObject) from within its message handler
* Parent is stuck in ::SetFocus and can't respond, hang

Comment 6

[Jim Mathies [:jimm]]

Attachment: kill focus patch v.1

There's really no way for the child process to know the cause of a wm_killfocus event, so we assume it's always the result of a call in parent to the sync call SetFocus.

Comment 7

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

Comment on attachment 423399 [details] [diff] [review]
kill focus patch v.1

>+static const TCHAR kPluginDoublePassMessageId[] = TEXT("MozDoublePassMsg");
>+static const TCHAR kPluginKillFocusBounceMessageId[] = TEXT("MozKillFocusBounceMsg");

Let's use RegisterWindowMessageW inline here. And no more TCHAR/TEXT :)

>+    if (message == WM_KILLFOCUS) {

Should we use InSendMessage() here? Would that effectively determine when the parent was sending the message?

>+        PostMessage(hWnd, self->mKillFocusBounceMsg, wParam, lParam);

I don't know if this is ok... We could change the order of messages that the plugin receives. How about we use InSendMessage in combination with ReplyMessage() to unblock the parent, then process like normal?

>+    else if (message == self->mKillFocusBounceMsg) {

Nit, else after return.

Comment 8

[Jim Mathies [:jimm]]

Attachment: kill focus patch v.2

ReplyMessage FTW!

Comment 9

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

Comment on attachment 423530 [details] [diff] [review]
kill focus patch v.2

Sweet!

Comment 10

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/5207354fa2fe

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Is this Windows 7 only? I can't reproduce this on build 3 of 1.9.2.4 on XP.

Comment 12

[Jim Mathies [:jimm]]

(In reply to comment #11)
> Is this Windows 7 only? I can't reproduce this on build 3 of 1.9.2.4 on XP.

No. The original fix landed months ago, but had to be moved in the patch in bug 563377. So it needs to be verified again I guess.