Bug 60143 – Entering full screen fails >= second time on Vimeo.com.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

Entering full screen succeeds the first time when watching a video on Vimeo, but fails the second and subsequent times for a given page.  The video is missing, and the full screen window shows the remaining, non-fullscreen portion of the page.

[Jer Noble]

<rdar://problem/9380650>

[Jer Noble]

Created attachment 92200 [details]
Patch

[Darin Adler]

Comment on attachment 92200 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92200&action=review

> Source/WebCore/rendering/RenderFullScreen.cpp:42
> +        // Force our layer to be updated by clearing its backing:

Strange non-standard use of a colon here.

I like that the comment says at least something the code does not. But It would be better to have a comment that said something a bit closer to what the change log note says. It’s not entirely clear how “clearing backing” is related to correctly reparenting, and that’s what the comment could help with.

[Collabora GTK+ EWS bot]

Attachment 92200 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8558538

[Jer Noble]

gtk is failing because RenderLayerCompositor inherits from GraphicsLayerClient.  The header is included from RenderLayerCompositor.h -> RenderLayerBacking.h -> GraphicsLayerClient.h.

Unfortunately, the include in RenderLayerBacking.h is wrapped in a #if USE(ACCELERATED_COMPOSITING), so presumably gtk does not have that flag set.

I'll wrap the #include "RenderLayerCompositor.h" statement in a #if USE(ACCELERATED_COMPOSITING) conditional before checking in.

[Jer Noble]

In the long run, however, the entirety of RenderLayerCompositor.h should be wrapped in a #if USE(ACCELERATED_COMPOSITING) if it is truly dependent on that flag (as it seems it is).

[Jer Noble]

Filed bug #60180 to cover the compile error.

[Jer Noble]

(In reply to comment #3)
> (From update of attachment 92200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92200&action=review
> 
> > Source/WebCore/rendering/RenderFullScreen.cpp:42
> > +        // Force our layer to be updated by clearing its backing:
> 
> Strange non-standard use of a colon here.
> 
> I like that the comment says at least something the code does not. But It would be better to have a comment that said something a bit closer to what the change log note says. It’s not entirely clear how “clearing backing” is related to correctly reparenting, and that’s what the comment could help with.

I've changed this comment to read: 

"Clearing the layer's backing will force the compositor to reparent the layer the next time layers are synchronized."

[Jer Noble]

Committed r85767: <http://trac.webkit.org/changeset/85767>

[WebKit Review Bot]

http://trac.webkit.org/changeset/85767 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
http/tests/navigation/response204.html