[webkit-reviews] review granted: [Bug 38856] Allow compositing layers to be connected across iframe boundaries on Mac : [Attachment 55603] Patch to have RenderLayerCompositor track the type of attachment for the root layer.

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 38856: Allow compositing layers to be connected across iframe boundaries on
Mac
https://bugs.webkit.org/show_bug.cgi?id=38856

Attachment 55603: Patch to have RenderLayerCompositor track the type of
attachment for the root layer.
https://bugs.webkit.org/attachment.cgi?id=55603&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 925b70f..a6e5806 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -4,6 +4,35 @@
>  
>	   Allow compositing layers to be connected across iframe boundaries on
Mac
>	   https://bugs.webkit.org/show_bug.cgi?id=38856
> +	   
> +	   Use an enum for the type of root layer attachment on a
RenderLayerCompositor, so we can
> +	   determine if the attachemnt is via the ChomeClient, via an enclosing
iframe, or unattached.
> +

Typo - attachemnt.

>  
> +void RenderLayerCompositor::attachRootPlatformLayer(RootLayerAttachment
attachment)
> +{
> +    if (!m_rootPlatformLayer)
> +	   return;
> +
> +    switch (attachment) {
> +    case RootLayerUnattached:
> +	   ASSERT_NOT_REACHED();
> +	   break;
> +    case RootLayerAttachedViaChromeClient: {
> +	       Frame* frame = m_renderView->frameView()->frame();
> +	       Page* page = frame ? frame->page() : 0;
> +	       if (!page)
> +		   return;
> +
> +	       page->chrome()->client()->attachRootGraphicsLayer(frame,
m_rootPlatformLayer.get());
> +	       break;
> +	   }

Is this really how we indent block statements inside case statements?

> +    case RootLayerAttachedViaEnclosingIframe: {
> +	       // The layer will get hooked up via
RenderLayerBacking::updateGraphicsLayerConfiguration()
> +	       // for the iframe's renderer in the parent document.
> +	      
m_renderView->document()->ownerElement()->setNeedsStyleRecalc(SyntheticStyleCha
nge);
> +	       break;
> +	   }
> +    }

Same here.

> +    
> +    m_rootLayerAttachment = attachment;
> +}
> +
> +void RenderLayerCompositor::detachRootPlatformLayer()
> +{
> +    if (!m_rootPlatformLayer || m_rootLayerAttachment ==
RootLayerUnattached)
> +	   return;
> +
> +    switch (m_rootLayerAttachment) {
> +    case RootLayerAttachedViaEnclosingIframe: {
> +	       // The layer will get unhooked up via
RenderLayerBacking::updateGraphicsLayerConfiguration()
> +	       // for the iframe's renderer in the parent document.
> +	      
m_renderView->document()->ownerElement()->setNeedsStyleRecalc(SyntheticStyleCha
nge);
> +	       break;
> +	   }

And here.

> +    case RootLayerAttachedViaChromeClient: {
> +	       Frame* frame = m_renderView->frameView()->frame();
> +	       Page* page = frame ? frame->page() : 0;
> +	       if (!page)
> +		   return;
> +
> +	       page->chrome()->client()->attachRootGraphicsLayer(frame, 0);
> +	   }
> +	   break;

And here.

r=me