Bug 60826 - Video is blank, controller is misplaced on trailers.apple.com movie in fullscreen (with two screens)
Summary: Video is blank, controller is misplaced on trailers.apple.com movie in fullsc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-13 19:51 PDT by Jer Noble
Modified: 2011-06-03 14:06 PDT (History)
1 user (show)

See Also:


Attachments
Patch (12.61 KB, patch)
2011-05-13 19:54 PDT, Jer Noble
simon.fraser: 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-13 19:51:38 PDT
Video is blank, controller is misplaced on trailers.apple.com movie in fullscreen (with two screens)
Comment 1 Jer Noble 2011-05-13 19:54:28 PDT
Created attachment 93544 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-05-13 20:09:09 PDT
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.
Comment 3 Jer Noble 2011-05-16 10:51:07 PDT
Committed r86588: <http://trac.webkit.org/changeset/86588>
Comment 4 Ademar Reis 2011-06-03 14:06:58 PDT
Revision r86588 cherry-picked into qtwebkit-2.2 with commit 9f2e6b1 <http://gitorious.org/webkit/qtwebkit/commit/9f2e6b1>