543788 - [OOPP] CallUpdateWindow hang when switching tabs with a plugin loaded

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Kevin Brosnan]

Load our friend the 36 MB pdf http://acroeng.adobe.com/Test_Files/filesize/Large_Files/0445_MT.pdf

Open a new tab
Switch between the pdf and other tabs, shortly your mouse clicks should be ignored.

Focusing the tab with the pdf and closing it unblocks the mouse. Closing the tab in the background never releases the mouse input. Killing mozilla-runtime.exe allows for the use of the mouse again.

Comment 1

[Tony Chung [:tchung]]

Nominating for blocking alpha1.

In today's testday, there has been many instances of this bug where the loaded plugin (acrobat, flash) is creating the mouse click on tabs to be ignored.  I have found one repro myself:

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100202 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Repro:
1) load 2 tabs, one with a youtube flash video playing
2) click back and forth between tabs
3) Eventually, notice you are unable to click on the 2nd tab anymore.

Expected:
- tab switching via mouse clicks should always work when plugins are running.

Comment 2

[Tony Chung [:tchung]]

cc'ing blizzard, as he can reproduce comment 1.

Comment 3

[Benjamin Smedberg]

Yeah, me too. ->jim the focus-master!

Comment 4

[Kevin Brosnan]

While having the mouse input blocked I am also seeing links from external programs not loading and form submission not processing till the blocking is cleared.

Comment 5

[Jim Mathies [:jimm]]

(In reply to comment #4)
> While having the mouse input blocked I am also seeing links from external
> programs not loading and form submission not processing till the blocking is
> cleared.

bug 543479

Comment 6

[Tony Chung [:tchung]]

Its not just flash.  the same thing happens with silverlight video loaded.  Need to force-kill process to resume browsing.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100205 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Silverlight Plug-In
    File: npctrl.dll
    Version: 3.0.50106.0
    3.0.50106.0

Comment 7

[Jim Mathies [:jimm]]

Attachment: CallUpdateWindow hang stack

Comment 9

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

(In reply to comment #7)
> Created an attachment (id=425717) [details]
> CallUpdateWindow hang stack

This could be fine, depending on the child process' stack.

Comment 10

[Benjamin Smedberg]

I have this in recording: it appears that the child isn't responding correctly to the messages, but I'm still investigating.

Comment 11

[Benjamin Smedberg]

What appears to be happening is an infinite loop:

Browser process is in the main event loop, no real trickery, and pretty much every message is in nsWindow::OnPaint -> PPluginInstanceParent::CallUpdateWindow.

At least for me, the child is processing the IPDL message perfectly correctly and calling ::UpdateWindow.

When the browser process unwinds it paints again. There are no deferred messages, or at least DeferredMessageHook and ProcessOrDeferMessage are never entered.

Can a debugger, or Spy++, know whether the plugin process is posting any WM_PAINT messages (directly or indirectly) and get stacks from that location?

Comment 12

[Benjamin Smedberg]

I caught one case where we deferred WM_NCPAINT and then WM_ERASEBKGND... I wonder if that can loop. I'm going to add some printfs which don't affect focus, since reproducing it with focus changes to the debugger is really hard.

Comment 13

[Benjamin Smedberg]

[time:1265669649151000][PPluginInstanceParent] Sending Msg_UpdateWindow()
[time:1265669649156000][PPluginInstanceChild] Received Msg_UpdateWindow()
0[760590]: bool __thiscall mozilla::plugins::PluginInstanceChild::AnswerUpdateWi
ndow(void)
[time:1265669649157000][PPluginInstanceChild] Sending reply Reply_UpdateWindow()

[time:1265669649157000][PPluginInstanceParent] Received reply Reply_UpdateWindow
()
[time:1265669649163000][PPluginInstanceParent] Sending Msg_UpdateWindow()

etc... Really, no deferred messages at all, no races. Still no clue.

Comment 14

[Benjamin Smedberg]

Attachment: Don't paint the browser plugin window container, rev. 1

What I think is happening: because we don't set WS_CLIPCHILDREN, windows asks us to paint the plugin container window. This is an unusual code path to begin with, and bug 542337 "worked around" this issue by having WM_PAINT for the container window cause us to issue an ::UpdateWindow for the plugin window. This is unnecessary, because Windows should be tracking the invalidate region for the plugin window separately and issuing WM_PAINT as needed. So far, this is just an unnecessary synchronous paint.

Then I think the windowed plugin is causing some invalidate on the container window while it is painting (within AnswerUpdateWindow)... this dispatches an asynchronous WM_PAINT message. Then after AnswerUpdateWindow returns, we call ::ValidateWindow, but the WM_PAINT message has already been dispatched, so we iloop.

I haven't been able to create a test for this situation, which is kinda scary, but at least this seems to fix the bug and paint correctly.

Comment 15

[nemo]

This sounds very similar to what I encountered, so I'll describe it here.

Went to:
http://treasurydirect.gov/govt/charts/charts_debt.htm

Opened a 2nd tab.
Clicked on dropdown to right in awesomebar to pick a recent site.  Clicked away from awesomebar, then clicked again. 2nd time, the dropdown was completely unresponsive, as was clicking on tabs, until I closed the flash page.

Only reason I'm mentioning is that the unresponsive awesomebar seems new.

Comment 16

[nemo]

Oh, and I opened link to verify while I was typing this.  I then hit commit.  The tab switched to the loading spinner, which didn't move, until I used ctrl-pageup to move back to the flash tab, at which point it submitted fine.

Switching back, navigation hung once more until I suspended it again.

Comment 18

[Benjamin Smedberg]

Attachment: Ignore an empty update rect from WM_PAINT, rev. 2

Comment 19

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

Comment on attachment 426308 [details] [diff] [review]
Ignore an empty update rect from WM_PAINT, rev. 2

>   if (mWindowType == eWindowType_plugin) {
>+    RECT updateRect;
>+    if (GetUpdateRect(mWnd, &updateRect, FALSE)

GetUpdateRect can return a boolean indicating whether or not the update rect is empty, so I'd make this:

  if (mWindowType == eWindowType_plugin &&
      GetUpdateRect(mWnd, NULL, FALSE)) {
  ...

Comment 20

[Benjamin Smedberg]

Comment on attachment 426308 [details] [diff] [review]
Ignore an empty update rect from WM_PAINT, rev. 2

bent, in the test I did, GetUpdateRect returned TRUE with a 0,0,0,0 rect, so I had to do it this way.

Comment 21

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

Comment on attachment 426308 [details] [diff] [review]
Ignore an empty update rect from WM_PAINT, rev. 2

Oh wow. MSDN loses.

Comment 22

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/f3ebaefe79b2

Comment 23

[Benjamin Smedberg]

and http://hg.mozilla.org/mozilla-central/rev/f8ef68a72d9f

Comment 24

[Jim Jeffery not reading bug-mail 1/2/11]

Using the latest hourly build which contains this fix, and the 2ns follow up push, I can still lock up the browser following the steps provided by the OP in comment #0

Win7 X-64 
cset: http://hg.mozilla.org/mozilla-central/rev/cc3751898ccc

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a2pre) Gecko/20100211 Minefield/3.7a2pre  ID:20100211150154

Comment 25

[IU]

I'm thinking my bug 544173 might be a dupe of this.  I guess I'll find out once this finally gets fixed.

Comment 26

[Ria Klaassen (not reading all bugmail)]

Confirmed not fixed. The bug stops if you continue with ctrl+tab. Then also clicking on tabs works again for a while.

Comment 27

[Jim Mathies [:jimm]]

(In reply to comment #26)
> Confirmed not fixed. The bug stops if you continue with ctrl+tab. Then also
> clicking on tabs works again for a while.

The call update window hang was an app freeze issue, at least for me when I experienced it. I've been seeing what Ria describes, which isn't a true hang - It's a tab switching problem. For example, when I experience it, I can still see the little tab height highlight draw on the tab I click, even though it doesn't come forward. This might be bug 545239, or it might be something else. But I believe the call update window lockup is fixed. Can anyone else confirm that?

Comment 28

[Jim Mathies [:jimm]]

another observation on this, when i break out in a debugger, the stacks look fine. neither process is frozen on a call to setfocus, or call update window.

Comment 29

[Benjamin Smedberg]

I screwed up this fix, I can reproduce in debug and can fix it with an extra ValidateWindow call. I'm trying to construct a testcase now... I was able to reliably reproduce this by:

* opening the PDF file and therefore acrobat
* create a new tab, load about:config
* right-click on about:config to get a popup menu

Comment 30

[Benjamin Smedberg]

Attachment: dummy BeginPaint/EndPaint

Comment 31

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/3e467aaa7fe9

Comment 33

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/360cba5cf39c
http://hg.mozilla.org/projects/firefox-lorentz/rev/11fa3864f2d9

Comment 34

[Mike Beltzner [:beltzner, not reading bugmail]]

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

Comment 35

[Benjamin Smedberg]

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430