Closed Bug 1264091 Opened 4 years ago Closed 4 years ago

[e10s] yahoo music player window switches between full-screen and default-screen sizes when zoom-In arrows are clicked in low performance laptops

Categories

(Core :: Graphics, defect)

All
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox48 --- fixed

People

(Reporter: Abe_LV, Assigned: xidorn)

References

()

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files, 1 obsolete file)

This is e10s specific issue with low performance laptops (specs are given below). It does not reproduce when e10s is disabled or high performance computer is used instead. It is also not affected by hardware acceleration conditions (checked or unchecked).

Steps to Reproduce:
1. Play music from https://www.yahoo.com/music/tagged/performances
2. Click the zoom arrows at the bottom-right of the movie window to watch in full-screen mode
3. Verify the window switches between full-screen and default-screen sizes before it turns to be a stable full-screen

Actual Result:
Player window does not directly go to full-screen mode.

Expected Result:
The player window should directly go to full-screen mode when zoom-In arrows are clicked.


Test Machines:
Laptop 1)
PROCESSOR: 	Z3735F@1.33GHz		
RAM: 		2GB		
GRAPHICS:	Intel HD Graphics Direct 3D11		
BUILD ARCH: 	Version 10.0 - 10240		

Laptop 2)
PROCESSOR: 	AMD A6-5200 APU @2.00GHZ		
RAM:	 	4GB / 3.45 USEABLE		
GRAPHICS:  	AMD Radeon HD 8400 / R3 Series Direct3D11		
BUILD ARCH: 	Version 10.0 - 10240		

Laptop 3)
PROCESSOR: 	N3050@1.60GHz		
RAM: 		2GB		
GRAPHICS: 	Intel HD Graphics Direct3d11 		
BUILD ARCH: 	Version 10.0 - 10240		

Laptop 4)
PROCESSOR: 	"AMD A4-6210 APU @1.80ghZ
"		
RAM: 		4GB		
GRAPHICS: 	AMD Radeon R3 Graphics Direct 3D11		
BUILD ARCH: 	Version 6.3 - 9600		

Laptop 5)
PROCESSOR: 	Intel Core i3-4005U @1.70GHz		
RAM: 		8GB		
GRAPHICS: 	Intel HD Graphics Family Direct 3D11		
BUILD ARCH: 	Version 10.0 (Build 10240)
Version  48.0a1
Build ID  20160412030231
Update Channel  nightly
User Agent  Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Barbara, should this block rollout? It's a polishy but the experience is kinda yucky.
Flags: needinfo?(bbermes)
FWIW, I can reproduce this on my reasonably high-powered engineering-spec MBP, so I don't think this is restricted to just low-end hardware.
After comment #3, I believe this is a thing to investigate further.

Audio/video remains a big piece to our users and hence should not be worse than before. 

I was able to reproduce the issues as well with my MacBook Pro. I wondered if it's "just" a flash issue but went over to YouTube, played a few videos in full-screen, exited and noticed also all kinds of wacky resizing issues. 

I'd say this blocks rollout.
Flags: needinfo?(bbermes)
Hmph, none of these videos play for me on Windows 7 in Nightly. The ads transition fine though.
(In reply to Jim Mathies [:jimm] from comment #5)
> Hmph, none of these videos play for me on Windows 7 in Nightly. The ads
> transition fine though.

Filed bug 1265238.
(In reply to Jim Mathies [:jimm] from comment #6)
> (In reply to Jim Mathies [:jimm] from comment #5)
> > Hmph, none of these videos play for me on Windows 7 in Nightly. The ads
> > transition fine though.
> 
> Filed bug 1265238.

Managed to track this down and get these playing. Still not able to see anything on my main Windows workstation or my Windows laptop though.

Tracy, could you try to reproduce and if so, post a screen capture video of this so we can all see it?
Flags: needinfo?(twalker)
Xidorn, any chance you can take a look at this?
Flags: needinfo?(bugzilla)
I guess this is because the fullscreen fade-through-black transition ends earlier than the content is painted correctly on e10s, which is probably due to the same issue mentioned in the dev-platform post about MozAfterPaint with e10s [1].

Fullscreen transition listen to this event for unblack the screen, so if that event is received earlier, it could be an issue.

I'll have a look.

[1] https://groups.google.com/forum/#!topic/mozilla.dev.platform/pCLwWdYc_GY
Assignee: nobody → bugzilla
Flags: needinfo?(bugzilla)
Attachment #8742576 - Flags: feedback?(mconley)
Comment on attachment 8742576 [details]
MozReview Request: Bug 1264091 - Ensure we unblack the screen for a right after-paint event. r?dao

A comment on the transactionId check would be nice. I have a rough idea what this is (more like a guess based your patch description) but I don't really know.
Attachment #8742576 - Flags: review?(dao+bmo) → review+
I'd be interested in knowing if this fixes the issue that Abe was seeing. At least for OS X, I'm still seeing the same behaviour with this patch (the fullscreen transition ends, but after the fade-up, we see the web content at normal scale for some seconds before the video transitions to fullscreen).

If this fixes it for Abe on Windows, perhaps we should open a separate bug for the OS X case (which barbara was hitting too in comment 4).

It's been a while since I dealt with the fullscreen stuff, but perhaps we need to wait until content has finished its fullscreen transition before doing the fade-up?
Attached video full screen switching unsmooth (obsolete) —
not sure this is gonna work. but here goes. :)
Flags: needinfo?(twalker)
Comment on attachment 8742949 [details]
full screen switching unsmooth

yep. no good.  have to do some research on how to get a video of this up.

But I can reproduce on window, fwiw.
Attachment #8742949 - Attachment is obsolete: true
Attachment #8742949 - Attachment mime type: text/plain → video/quicktime
Are you able to reproduce with xidorn's patch? Here's a try build you can try (when it's done building):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=984ae03eb2fc
Flags: needinfo?(twalker)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #12)
> I'd be interested in knowing if this fixes the issue that Abe was seeing. At
> least for OS X, I'm still seeing the same behaviour with this patch (the
> fullscreen transition ends, but after the fade-up, we see the web content at
> normal scale for some seconds before the video transitions to fullscreen).
> 
> If this fixes it for Abe on Windows, perhaps we should open a separate bug
> for the OS X case (which barbara was hitting too in comment 4).
> 
> It's been a while since I dealt with the fullscreen stuff, but perhaps we
> need to wait until content has finished its fullscreen transition before
> doing the fade-up?

We wait until that, but with a timeout of 0.5s (was 1s), because there are some cases we cannot guarantee to have that done, and also a too long black screen would annoy user as well.

The reflow time could sometimes take several hundreds of milliseconds, so the boundary is not too hard to be broken. This specific page could take ~3s on my local debug build to do fullscreen reflow. To check the effectiveness of this patch, you may want to set pref "full-screen-api.transition.timeout" to something like 5000 (5s) so that the time limit is never reached.

There is another e10s-specific fullscreen issue that the message between processes could sometimes be very slow (up to ~100ms). I'm going to rewrite that part with native IPC call in bug 1251122 with expectation that it could reduce code complexity and improve performance here.
Attached video yahoo music player
The issue still reproduces in the try-build from Mike (comment 15). 

---
Version 	48.0a1
Build ID 	20160419153036
Update Channel 	default
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
The try build is a little better in that the transition goes directly to and from full screen without hitting an intermediate sized window in between.  However, and it may just be stressed machine, painting just after transition is choppy and the audio is choppy for a second or two, as well.
Flags: needinfo?(twalker)
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #16)
> The reflow time could sometimes take several hundreds of milliseconds, so
> the boundary is not too hard to be broken. This specific page could take ~3s
> on my local debug build to do fullscreen reflow. To check the effectiveness
> of this patch, you may want to set pref "full-screen-api.transition.timeout"
> to something like 5000 (5s) so that the time limit is never reached.
> 
> There is another e10s-specific fullscreen issue that the message between
> processes could sometimes be very slow (up to ~100ms). I'm going to rewrite
> that part with native IPC call in bug 1251122 with expectation that it could
> reduce code complexity and improve performance here.

This is possible, and I think what needs to be investigated. I don't think the patch in this bug actually addresses the issue being reported here.

We need to figure out where the delay is in transitioning the web content to full screen mode, when compared against non-e10s.
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #19)
> (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #16)
> > The reflow time could sometimes take several hundreds of milliseconds, so
> > the boundary is not too hard to be broken. This specific page could take ~3s
> > on my local debug build to do fullscreen reflow. To check the effectiveness
> > of this patch, you may want to set pref "full-screen-api.transition.timeout"
> > to something like 5000 (5s) so that the time limit is never reached.
> > 
> > There is another e10s-specific fullscreen issue that the message between
> > processes could sometimes be very slow (up to ~100ms). I'm going to rewrite
> > that part with native IPC call in bug 1251122 with expectation that it could
> > reduce code complexity and improve performance here.
> 
> This is possible, and I think what needs to be investigated. I don't think
> the patch in this bug actually addresses the issue being reported here.

Well, there are two issues:
1. MozAfterPaint could be dispatched before we finish the reflow;
2. the message could be dispatched too slow.

To know which is issue here, you can set "full-screen-api.transition.timeout" to a large value (e.g. 5000). If it doesn't happen anymore, then it is just 2), however if it still happens, it could be 1) or a mix of 1) and 2).

The patch in this bug addresses the first issue, and the second issue is relatively non-trivial.

I set feedback? you because I hope to confirm with you whether this is the right way to use transitionId given you sent that email to dev-platform. If that is the right way, let's close this bug with this patch, and probably open another bug for the second issue. Do you agree?
Flags: needinfo?(mconley)
Comment on attachment 8742576 [details]
MozReview Request: Bug 1264091 - Ensure we unblack the screen for a right after-paint event. r?dao

Yep, okay. Sounds good.
Flags: needinfo?(mconley)
Attachment #8742576 - Flags: feedback?(mconley) → feedback+
https://hg.mozilla.org/mozilla-central/rev/2f34f0a20c58
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.