Bug 60826 – Video is blank, controller is misplaced on trailers.apple.com movie in fullscreen (with two screens)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Video is blank, controller is misplaced on trailers.apple.com movie in fullscreen (with two screens)

[Jer Noble]

Created attachment 93544 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 93544 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1238
> +    PlatformLayer* layer = m_qtVideoLayer.get();
> +    do {
> +        if (rootLayer != layer)
> +            continue;
> +
> +        // We own a child layer of a layer which has switched contexts.  
> +        // Tear down our layer, and set m_visible to false, so that the 
> +        // next time setVisible(true) is called, the layer will be re-
> +        // created in the correct context.
> +        tearDownVideoRendering();
> +        m_visible = false;
> +        break;
> +    } while((layer = [layer superlayer]));

This code is confusing. I think you want to have a loop that walks up the tree, and to pull the tearDownVideoRendering() code out of the loop.

> Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:145
> +        [[NSNotificationCenter defaultCenter] postNotificationName:@"CAContextChanged" object:nil userInfo:[NSDictionary dictionaryWithObject:m_fullScreenRootLayer.get() forKey:@"RootLayer"]];
> +        m_fullScreenRootLayer = 0;

You should add a comment to say who listens for this notification. I also think you should use a more unique name than "CAContextChanged", since it would be easy to imagine CA adding a notification with the same name in future.

[Jer Noble]

Committed r86588: <http://trac.webkit.org/changeset/86588>

[Ademar Reis]

Revision r86588 cherry-picked into qtwebkit-2.2 with commit 9f2e6b1 <http://gitorious.org/webkit/qtwebkit/commit/9f2e6b1>