Bug 38856 – Allow compositing layers to be connected across iframe boundaries on Mac

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Here follows a set of patches to hook up compositing layers across Iframes on Mac.

[Simon Fraser (smfr)]

<rdar://problem/7950592>

[Simon Fraser (smfr)]

Created attachment 55576 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 55586 [details]
Add a bunch move compositing iframe tests

[Simon Fraser (smfr)]

Created attachment 55589 [details]
Patch

[Chris Marrin]

Created attachment 55598 [details]
Windows results for new test cases

[Anders Carlsson]

Comment on attachment 55576 [details]
Patch

r=me

[Anders Carlsson]

Comment on attachment 55586 [details]
Add a bunch move compositing iframe tests

rs=me

[Anders Carlsson]

Comment on attachment 55589 [details]
Patch

r=me

[Anders Carlsson]

Comment on attachment 55598 [details]
Windows results for new test cases

rs=me

[Chris Marrin]

Created attachment 55601 [details]
Replace patch for Windows results

[Simon Fraser (smfr)]

Created attachment 55603 [details]
Patch to have RenderLayerCompositor track the type of attachment for the root layer.

[Simon Fraser (smfr)]

Created attachment 55616 [details]
Allow run-time switching of iframe propagation

[Simon Fraser (smfr)]

Created attachment 55656 [details]
Patch

[Anders Carlsson]

Comment on attachment 55603 [details]
Patch to have RenderLayerCompositor track the type of attachment for the root layer.

> 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(SyntheticStyleChange);
> +            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(SyntheticStyleChange);
> +            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

[Anders Carlsson]

Comment on attachment 55616 [details]
Allow run-time switching of iframe propagation

r=me

[Simon Fraser (smfr)]

Created attachment 55659 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 55661 [details]
Patch

[Anders Carlsson]

Comment on attachment 55656 [details]
Patch


> [phone]  Simon Fraser  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=38856
> +        Allow compositing layers to be connected across iframe boundaries on Mac
> +        

Duplicate ChangeLog entry.

> +
> [phone]  Simon Fraser  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Allow compositing layers to be connected across iframe boundaries on Mac
> +        https://bugs.webkit.org/show_bug.cgi?id=38856
> +

Same here?

>          Rename the static shouldPropagateCompositingToIFrameParent() to shouldPropagateCompositingToEnclosingIFrame(),
>          to pave the way for runtime switches in the propagation behavior. We have to make sure we call it on
>          the correct RenderLayerCompositor (that belonging to the iframe's content document).
> diff --git a/WebCore/page/FrameView.cpp b/WebCore/page/FrameView.cpp
> index 9b93bebb60ff00ae7bf5276cd7f89a4b99e97913..3f34261079b9e5db674f4098f7ae1f1f7141d44c 100644
> --- a/WebCore/page/FrameView.cpp
> +++ b/WebCore/page/FrameView.cpp
> @@ -478,6 +478,15 @@ void FrameView::setNeedsOneShotDrawingSynchronization()
>  
>  #endif // USE(ACCELERATED_COMPOSITING)
>  
> +bool FrameView::hasCompositedContent() const
> +{
> +#if USE(ACCELERATED_COMPOSITING)
> +    if (RenderView* view = m_frame->contentRenderer())
> +        return view->compositor()->inCompositingMode();
> +#endif
> +    return false;
> +}
> +
>  bool FrameView::isEnclosedInCompositingLayer() const
>  {
>  #if USE(ACCELERATED_COMPOSITING)
> @@ -897,6 +906,15 @@ void FrameView::setIsOverlapped(bool isOverlapped)
>  
>      m_isOverlapped = isOverlapped;
>      setCanBlitOnScroll(!useSlowRepaints());
> +    
> +#if USE(ACCELERATED_COMPOSITING)
> +    // Overlap can affect compositing tests, so if it changes, we need to trigger
> +    // a recalcStyle in the parent document.
> +    if (hasCompositedContent()) {
> +        if (Element* ownerElement = m_frame->document()->ownerElement())
> +            ownerElement->setNeedsStyleRecalc(SyntheticStyleChange);
> +    }
> +#endif    
>  }
>  
> +    RenderLayer* layer = m_renderView->layer();
> +    RenderLayerBacking* backing = layer ? layer->backing() : 0;
> +    if (backing)
> +        backing->updateDrawsContent();

You can move the "backing" variable declaration into the if statement.

r=me

[Anders Carlsson]

Comment on attachment 55661 [details]
Patch

r=me!

[Anders Carlsson]

Comment on attachment 55659 [details]
Patch

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 704c10956fe981ae131789fb56ee12c17af52890..84c82a0fef270d0a254399a8ea6ba002dab1a072 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -4,6 +4,25 @@

> [phone]  Simon Fraser  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Allow compositing layers to be connected across iframe boundaries on Mac
> +        https://bugs.webkit.org/show_bug.cgi?id=38856

Duplicate ChangeLog entry!

r=me

[Simon Fraser (smfr)]

Comment on attachment 55576 [details]
Patch

http://trac.webkit.org/changeset/59129

[Simon Fraser (smfr)]

Comment on attachment 55586 [details]
Add a bunch move compositing iframe tests

http://trac.webkit.org/changeset/59130

[Simon Fraser (smfr)]

Comment on attachment 55601 [details]
Replace patch for Windows results

http://trac.webkit.org/changeset/59131

[Simon Fraser (smfr)]

Comment on attachment 55589 [details]
Patch

http://trac.webkit.org/changeset/59132

[Simon Fraser (smfr)]

Comment on attachment 55603 [details]
Patch to have RenderLayerCompositor track the type of attachment for the root layer.

http://trac.webkit.org/changeset/59133

[Simon Fraser (smfr)]

Comment on attachment 55616 [details]
Allow run-time switching of iframe propagation

http://trac.webkit.org/changeset/59134

[Simon Fraser (smfr)]

Comment on attachment 55656 [details]
Patch

http://trac.webkit.org/changeset/59136

[Simon Fraser (smfr)]

Comment on attachment 55661 [details]
Patch

http://trac.webkit.org/changeset/59137

[Simon Fraser (smfr)]

Comment on attachment 55659 [details]
Patch

http://trac.webkit.org/changeset/59137

[Simon Fraser (smfr)]

Comment on attachment 55661 [details]
Patch

http://trac.webkit.org/changeset/59138

[Simon Fraser (smfr)]

All patches landed.

[WebKit Review Bot]

http://trac.webkit.org/changeset/59137 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59137
http://trac.webkit.org/changeset/59138
http://trac.webkit.org/changeset/59131
http://trac.webkit.org/changeset/59132
http://trac.webkit.org/changeset/59133
http://trac.webkit.org/changeset/59134
http://trac.webkit.org/changeset/59135

[WebKit Review Bot]

http://trac.webkit.org/changeset/59138 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59137
http://trac.webkit.org/changeset/59138
http://trac.webkit.org/changeset/59131
http://trac.webkit.org/changeset/59132
http://trac.webkit.org/changeset/59133
http://trac.webkit.org/changeset/59134
http://trac.webkit.org/changeset/59135

[WebKit Review Bot]

http://trac.webkit.org/changeset/59132 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59137
http://trac.webkit.org/changeset/59138
http://trac.webkit.org/changeset/59131
http://trac.webkit.org/changeset/59132
http://trac.webkit.org/changeset/59133
http://trac.webkit.org/changeset/59134
http://trac.webkit.org/changeset/59135

[WebKit Review Bot]

http://trac.webkit.org/changeset/59133 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59137
http://trac.webkit.org/changeset/59138
http://trac.webkit.org/changeset/59131
http://trac.webkit.org/changeset/59132
http://trac.webkit.org/changeset/59133
http://trac.webkit.org/changeset/59134
http://trac.webkit.org/changeset/59135

[WebKit Review Bot]

http://trac.webkit.org/changeset/59134 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59137
http://trac.webkit.org/changeset/59138
http://trac.webkit.org/changeset/59131
http://trac.webkit.org/changeset/59132
http://trac.webkit.org/changeset/59133
http://trac.webkit.org/changeset/59134
http://trac.webkit.org/changeset/59135

[Simon Fraser (smfr)]

GTK Linux should be fixed by http://trac.webkit.org/changeset/59143