Bug 60143 - Entering full screen fails >= second time on Vimeo.com.
Summary: Entering full screen fails >= second time on Vimeo.com.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Jer Noble
URL: http://vimeo.com/21567634
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-04 00:02 PDT by Jer Noble
Modified: 2011-05-04 15:07 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2011-05-04 01:54 PDT, Jer Noble
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-05-04 00:02:57 PDT
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.
Comment 1 Jer Noble 2011-05-04 00:03:25 PDT
<rdar://problem/9380650>
Comment 2 Jer Noble 2011-05-04 01:54:05 PDT
Created attachment 92200 [details]
Patch
Comment 3 Darin Adler 2011-05-04 09:12:51 PDT
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.
Comment 4 Collabora GTK+ EWS bot 2011-05-04 09:30:30 PDT
Attachment 92200 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8558538
Comment 5 Jer Noble 2011-05-04 09:37:30 PDT
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.
Comment 6 Jer Noble 2011-05-04 09:38:16 PDT
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).
Comment 7 Jer Noble 2011-05-04 09:42:16 PDT
Filed bug #60180 to cover the compile error.
Comment 8 Jer Noble 2011-05-04 09:44:52 PDT
(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."
Comment 9 Jer Noble 2011-05-04 11:39:46 PDT
Committed r85767: <http://trac.webkit.org/changeset/85767>
Comment 10 WebKit Review Bot 2011-05-04 15:07:25 PDT
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