Bug 112397 - REGRESSION: -webkit-box-reflect does not show on video elements
Summary: REGRESSION: -webkit-box-reflect does not show on video elements
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: InRadar
Depends on:
Blocks:
 
Reported: 2013-03-14 18:17 PDT by Jer Noble
Modified: 2013-03-22 09:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (13.86 KB, patch)
2013-03-15 08:49 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for committing (41.35 KB, patch)
2013-03-15 09:58 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (2.38 KB, patch)
2013-03-21 17:08 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2013-03-14 18:17:06 PDT
REGRESSION: -webkit-box-reflect does not show on video elements
Comment 1 Jer Noble 2013-03-15 08:49:56 PDT
Created attachment 193317 [details]
Patch

The pixel results are currently broken. I need to figure out why DumpRenderTree and WKTR are both generating empty images.
Comment 2 Simon Fraser (smfr) 2013-03-15 09:18:37 PDT
Comment on attachment 193317 [details]
Patch

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

r=me but please make sure the pixel result is correct.

> Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:277
> +        [newLayer->playerLayer() setPlayer:[playerLayer() player]];

Most ridiculous-sounding line of code ever.

> LayoutTests/ChangeLog:8
> +        * compositing/video/video-reflection-expected.png: Added.

Is this supposed to be all dark gray?
Comment 3 Build Bot 2013-03-15 09:24:01 PDT
Comment on attachment 193317 [details]
Patch

Attachment 193317 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17173588
Comment 4 Jer Noble 2013-03-15 09:42:58 PDT
(In reply to comment #2)
> (From update of attachment 193317 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193317&action=review
> 
> r=me but please make sure the pixel result is correct.
> 
> > Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:277
> > +        [newLayer->playerLayer() setPlayer:[playerLayer() player]];
> 
> Most ridiculous-sounding line of code ever.

Very true.  But, it was worse when there was a lot more casting involved:

[(AVPlayerLayer*)newLayer->m_layer.get() setPlayer:[(AVPlayerLayer*)m_layer.get() player]];

Hence the addition of AVPlayerLayer* PlatformCALayer::playerLayer().

> > LayoutTests/ChangeLog:8
> > +        * compositing/video/video-reflection-expected.png: Added.
> 
> Is this supposed to be all dark gray?

No it's not; I'm working on generating the results on another OS build.  That and the win/ port is busted, so I'll fix that as well.
Comment 5 Jer Noble 2013-03-15 09:58:13 PDT
Created attachment 193326 [details]
Patch for committing

Fixed the pixel test results and the mac/ build.
Comment 6 Jer Noble 2013-03-15 11:00:21 PDT
Committed r145915: <http://trac.webkit.org/changeset/145915>
Comment 7 Jer Noble 2013-03-15 11:07:03 PDT
Committed windows build fix in r145918: <https://trac.webkit.org/changeset/145918>
Comment 8 Simon Fraser (smfr) 2013-03-15 21:44:49 PDT
This is causing an assertion via a weird re-entrancy stack:

http://build.webkit.org/results/Apple%20Lion%20Debug%20WK2%20(Tests)/r145980%20(8104)/compositing/reflections/load-video-in-reflection-crash-log.txt

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010590b705 WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool) + 229 (RenderLayerCompositor.cpp:342)
1   com.apple.WebCore             	0x0000000104badd9f WebCore::FrameView::flushCompositingStateForThisFrame(WebCore::Frame*) + 255 (FrameView.cpp:908)
2   com.apple.WebCore             	0x0000000104bb80e5 WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&) + 741 (FrameView.cpp:3399)
3   com.apple.WebCore             	0x0000000105b56d89 WebCore::ScrollView::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 777 (ScrollView.cpp:1081)
4   com.apple.WebKit2             	0x00000001021cfd34 WebKit::WebPage::drawRect(WebCore::GraphicsContext&, WebCore::IntRect const&) + 292 (WebPage.cpp:1108)
5   com.apple.WebKit2             	0x0000000101fca3a8 WebKit::LayerTreeHostMac::paintContents(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, unsigned int, WebCore::IntRect const&) + 88 (LayerTreeHostMac.mm:254)
6   com.apple.WebKit2             	0x0000000101fca435 non-virtual thunk to WebKit::LayerTreeHostMac::paintContents(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, unsigned int, WebCore::IntRect const&) + 69
7   com.apple.WebCore             	0x0000000104c85926 WebCore::GraphicsLayer::paintGraphicsLayerContents(WebCore::GraphicsContext&, WebCore::IntRect const&) + 294 (GraphicsLayer.cpp:328)
8   com.apple.WebCore             	0x0000000104c93460 WebCore::GraphicsLayerCA::platformCALayerPaintContents(WebCore::GraphicsContext&, WebCore::IntRect const&) + 48 (GraphicsLayerCA.cpp:1059)
9   com.apple.WebCore             	0x0000000104c934a7 non-virtual thunk to WebCore::GraphicsLayerCA::platformCALayerPaintContents(WebCore::GraphicsContext&, WebCore::IntRect const&) + 55
10  com.apple.WebCore             	0x0000000105f2b700 drawLayerContents(CGContext*, CALayer*, WebCore::PlatformCALayer*) + 1744 (WebLayer.mm:116)
11  com.apple.WebCore             	0x0000000105f2c5e6 -[WebLayer drawInContext:] + 70 (WebLayer.mm:233)
12  com.apple.QuartzCore          	0x00007fff8f1e7225 CABackingStoreUpdate_ + 3221
13  com.apple.QuartzCore          	0x00007fff8f1e613a CA::Layer::display_() + 1086
14  com.apple.WebCore             	0x0000000105f2c536 -[WebLayer display] + 54 (WebLayer.mm:223)
15  com.apple.QuartzCore          	0x00007fff8f1de00a CA::Layer::display_if_needed(CA::Transaction*) + 560
16  com.apple.QuartzCore          	0x00007fff8f1dcfbf CA::Context::commit_transaction(CA::Transaction*) + 319
17  com.apple.QuartzCore          	0x00007fff8f1dcd3c CA::Transaction::commit() + 274
18  com.apple.MediaToolbox        	0x00007fff8f3990e3 0x7fff8f34d000 + 311523
19  com.apple.MediaToolbox        	0x00007fff8f399945 0x7fff8f34d000 + 313669
20  com.apple.MediaToolbox        	0x00007fff8f399d60 0x7fff8f34d000 + 314720
21  com.apple.MediaToolbox        	0x00007fff8f399e62 0x7fff8f34d000 + 314978
22  com.apple.MediaToolbox        	0x00007fff8f38e79a 0x7fff8f34d000 + 268186
23  com.apple.MediaToolbox        	0x00007fff8f38d057 0x7fff8f34d000 + 262231
24  com.apple.avfoundation        	0x00007fff9441ae2f -[AVPlayerLayer _updateContentLayerConnectionToPlayer] + 187
25  com.apple.avfoundation        	0x00007fff94419e76 -[AVPlayerLayer setPlayer:] + 442
26  com.apple.WebCore             	0x0000000105745642 WebCore::PlatformCALayer::clone(WebCore::PlatformCALayerClient*) const + 1202 (PlatformCALayerMac.mm:277)
27  com.apple.WebCore             	0x0000000104c9c7a3 WebCore::GraphicsLayerCA::cloneLayer(WebCore::PlatformCALayer*, WebCore::GraphicsLayerCA::CloneLevel) + 67 (GraphicsLayerCA.cpp:2892)
28  com.apple.WebCore             	0x0000000104c9c675 WebCore::GraphicsLayerCA::findOrMakeClone(WTF::String, WebCore::PlatformCALayer*, WTF::HashMap<WTF::String, WTF::RefPtr<WebCore::PlatformCALayer>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WebCore::PlatformCALayer> > >*, WebCore::GraphicsLayerCA::CloneLevel) + 261 (GraphicsLayerCA.cpp:2740)
29  com.apple.WebCore             	0x0000000104c9cc34 WebCore::GraphicsLayerCA::ensureCloneLayers(WTF::String, WTF::RefPtr<WebCore::PlatformCALayer>&, WTF::RefPtr<WebCore::PlatformCALayer>&, WTF::RefPtr<WebCore::PlatformCALayer>&, WebCore::GraphicsLayerCA::CloneLevel) + 916 (GraphicsLayerCA.cpp:2766)
Comment 9 Simon Fraser (smfr) 2013-03-15 21:46:21 PDT
...only on Lion. I'm going to disable media layer cloning on Lion for now.
Comment 10 Simon Fraser (smfr) 2013-03-15 21:58:04 PDT
See bug 112490.
Comment 11 Simon Fraser (smfr) 2013-03-16 12:29:55 PDT
Gonna have to undo this, sorry.
Comment 12 Simon Fraser (smfr) 2013-03-16 12:35:09 PDT
Disabled this in http://trac.webkit.org/changeset/146001
Comment 13 Simon Fraser (smfr) 2013-03-16 12:36:18 PDT
<rdar://problem/10562776>
Comment 14 Jer Noble 2013-03-21 17:08:03 PDT
Created attachment 194391 [details]
Patch
Comment 15 WebKit Review Bot 2013-03-22 09:27:13 PDT
Comment on attachment 194391 [details]
Patch

Clearing flags on attachment: 194391

Committed r146622: <http://trac.webkit.org/changeset/146622>
Comment 16 WebKit Review Bot 2013-03-22 09:27:17 PDT
All reviewed patches have been landed.  Closing bug.