Closed Bug 547142 Opened 14 years ago Closed 14 years ago

[OOPP] Exiting full-screen video locks the browser

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(5 files, 3 obsolete files)

(In reply to comment #15)
> While testing this, it goes into full screen ok while watching a video on Win7
> and stays.  But when I escape out of full screen now I'm get a complete browser
> hang after coming back out of Hulu fullscreen and no video, only audio
> continues playing.  I have to kill the browser.

Spin off from bug 539658. More focus hang fun.
going full screen on the second video and then exiting will usually trigger this.
Assignee: nobody → jmathies
Attachment #427782 - Attachment is patch: false
Attachment #427782 - Attachment mime type: text/plain → text/html
Attached patch patch (obsolete) — Splinter Review
Attachment #427785 - Flags: review?(bent.mozilla)
Attached patch patch (obsolete) — Splinter Review
bsmedberg pointed out there's no need for the send message check. According to ReplyMessage docs, there's no effect if we're not in SendMessage.
Attachment #427785 - Attachment is obsolete: true
Attachment #427790 - Flags: review?(bent.mozilla)
Attachment #427785 - Flags: review?(bent.mozilla)
Comment on attachment 427790 [details] [diff] [review]
patch

I like the changes you made to the switch statement but i don't think 0 is the correct thing to reply with in this case.
Attachment #427790 - Flags: review?(bent.mozilla) → review-
This seems to not work.  Using the test-case and playing the 'windowless' video, and clicking into full-screen all is good until you ESC out.  The video/audio continues to play but the area where the small windowless vid should be, or was...is blank, then shortly thereafter the plugin will crash. 

Of course, it appears that the crashes for OOPP are not being sent since the new XUL crash page was implemented a few days ago. 

cset: http://hg.mozilla.org/mozilla-central/rev/23e78dba9f94 
Which should contain both patches.
Seems that possible STR: 

Start video windowless test case or a video from HULU
While the video is playing mouse near the bottom to display playback controls
hit the pause button.
Press ESC, which returns to small window on Hulu page, but its blank..
Note that browser goes Not Responding, having to kill mozilla-runtime.

Win7 x64
oops, sorry for the bug-spam - I was looking at wrong bug, this has not even been checked in yet :(
For comparison, I can get hulu to hang the browser on this page:
http://www.hulu.com/watch/60994/voltron-defender-of-the-universe-my-brother-is-a-robeast#s-p1-so-i0

If I watch the video on hulu, go to fullscreen, esc, go back to fullscreen, esc.  The browser completely hangs.

On the windowless testcase, go fullscreen, pause, esc, I get no video, a slight hang, the plugin crashes, the plugin ui comes up, and the browser comes back.
It's very easy to reproduce and diagnose. We ran into a problem with the first patch submitted but should have something landed here in a couple days that addresses this.
Blocks: LorentzAlpha
Attached patch patchSplinter Review
so getting windows nswindow header included was a bit of a dependency nightmare, so I used the send message trick we did with the focus stuff. Seems to work pretty well.
Attachment #427790 - Attachment is obsolete: true
Attachment #428287 - Flags: review?(bent.mozilla)
Comment on attachment 428287 [details] [diff] [review]
patch

>+  NS_ASSERTION(someWindow->sCallDepth >= 0, "Call depth out of sync.");

Since that's a static var let's use |nsWindow::sCallDepth| instead, otherwise it looks like a nonstatic member.

>+#ifdef MOZ_IPC
>+  someWindow->sCallDepth--;
>+#endif

Nit: indent that sCallDepth-- more.

>+        *(reinterpret_cast<UINT*>(wParam)) = sBaseMsg;

I think you should assert sCallDepth > 0 here, otherwise you may need to null out sBaseMsg when sCallDepth goes to 0.
Attachment #428287 - Flags: review?(bent.mozilla) → review+
http://hg.mozilla.org/mozilla-central/rev/d168e7d597c9
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
V Fixed with latest hourly.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a2pre) Gecko/20100222 Minefield/3.7a2pre (.NET CLR 3.5.30729) ID:20100222175309
I tested the testcase multiple times and didn't see the bug, I tested the Hulu URL from before and didn't run into a problem.  Though I retested the URL again and I went in and out of full screen about 5-10 times trying to reproduce, which finally did.  

Unfortunately, I just tested a completely new video on hulu and saw the bug immediately - so still seeing the problem there.  I tried various timings from start to exit fullscreen, and reproduced the bug almost every time (20 times) except once. Testing Flash 10.0.45.2 - when I get a complete hang, and close Minefield, windows informs me that Adobe script is running slowly.  

Maybe hulu is more of a script based problem causing issue here and probably another bug?  I suppose a stack trace would be helpful.
I can confirm what Dale is saying. I am still getting hangs when exiting fullscreen Hulu videos. I have to manually terminate mozilla-runtime.exe to get the browser back. This bug should be reopened.
Debug builds don't appear to have any problems, but after a few tries a release build did freeze up for me. Looks like a completely different stack from the original posted unfortunately. Will post more info here in a bit.
Sounds like a new bug. Let's not reopen this one.
Hmm, I'm not having much luck, but I'm testing on my pokey laptop this week. I'll see if I can find another computer to test with. If anyone can reproduce this new hang easily, please post stacks of the parent & child if you can.
I can reproduce this everytime. How do I get stacks?
Attached file stack v2.0 (obsolete) —
I was able to catch this, but I'm not sure what I've caught! The frames on the child side below flash were lost in windbg. The parent is handling WM_ACTIVATE, and appears to be sending a win event back to child, which ends up triggering a call back to parent over IPC. (just a guess at this point) Sort of a reverse of what we fixed the first time around.
(In reply to comment #19)
> I can reproduce this everytime. How do I get stacks?

If you're familiar with the windows debugger, you can use that. Download the windows debug tools, open windbg, and attach to firefox.exe. Do the same for mozilla-runtime.exe in another instance of windbg. You'll also need to setup the symbols server before hand - add

SRV*c:\symbols\*http://msdl.microsoft.com/download/symbols;SRV*c:\symbols\*http://symbols.mozilla.org/firefox

under file -> symbol file path... and save that out as a workspace so it's the default.
I downloaded the Windows Debugger, but everytime I try to attach firefox.exe, the browser becomes completely unresponsive so I can't even go to Hulu to create the crash.
When I follow the instructions posted by timeless, the browser doesn't even start up when I press Go. Instead I get this error.

Breakpoint 0 hit
eax=00000000 ebx=00000000 ecx=0017f8d0 edx=76f464f4 esi=fffffffe edi=00000000
eip=76f9e612 esp=0017f8ec ebp=0017f918 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000246
ntdll!LdrpDoDebuggerBreak+0x30:
76f9e612 90              nop
Attached file clean stack v2.0
Able to reproduce this locally now. Seems to help if you run a short video in full screen all the way to the end, then exit.
Attachment #428851 - Attachment is obsolete: true
Attached file firefox.exe stack
I finally managed to get a stack for the new fullscreen video bug. I hope this stack is useful.
Here's the stack for mozilla-runtime.
Attachment #429657 - Attachment mime type: application/octet-stream → text/plain
Attachment #429658 - Attachment mime type: application/octet-stream → text/plain
I have this in a recording now and I've also managed to reproduce this in a debug build through some tricks I've learned. Currently I'm leaning toward hooking ShowWindow api calls in the child to address.  Tomorrow I'll either file off another bug or reopen this one once I'm done debugging the problem.
I've built some try builds of a new test patch in bug 550322 if anyone cares to take them for a spin. The new patch appears to address both hangs. If you can reproduce this easily, I'd appreciate help in testing this new patch to see if it addresses the problem.
I can confirm that your try build works fine on Hulu and on IGN videos. No crashes after repeated fullscreens and exits. Good work!
I agree, I also retested the simplified testcase here with the try server build in bug 550322 and this continues to work also for non hulu fullscreen hangs and video is there too when exiting back from fullscreen.

Everything looks like its working.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: