Bug 38856 - Allow compositing layers to be connected across iframe boundaries on Mac
Summary: Allow compositing layers to be connected across iframe boundaries on Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-05-10 10:54 PDT by Simon Fraser (smfr)
Modified: 2010-05-10 23:44 PDT (History)
4 users (show)

See Also:


Attachments
Patch (11.50 KB, patch)
2010-05-10 11:15 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Add a bunch move compositing iframe tests (14.27 KB, patch)
2010-05-10 12:04 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2010-05-10 12:20 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Windows results for new test cases (12.68 KB, patch)
2010-05-10 13:49 PDT, Chris Marrin
andersca: review+
Details | Formatted Diff | Diff
Replace patch for Windows results (13.70 KB, patch)
2010-05-10 14:08 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch to have RenderLayerCompositor track the type of attachment for the root layer. (11.93 KB, patch)
2010-05-10 14:25 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Allow run-time switching of iframe propagation (8.05 KB, patch)
2010-05-10 16:03 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (59.01 KB, patch)
2010-05-10 21:22 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (2.99 KB, patch)
2010-05-10 21:44 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (2.27 KB, patch)
2010-05-10 21:47 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-05-10 10:54:38 PDT
Here follows a set of patches to hook up compositing layers across Iframes on Mac.
Comment 1 Simon Fraser (smfr) 2010-05-10 10:55:02 PDT
<rdar://problem/7950592>
Comment 2 Simon Fraser (smfr) 2010-05-10 11:15:53 PDT
Created attachment 55576 [details]
Patch
Comment 3 Simon Fraser (smfr) 2010-05-10 12:04:45 PDT
Created attachment 55586 [details]
Add a bunch move compositing iframe tests
Comment 4 Simon Fraser (smfr) 2010-05-10 12:20:26 PDT
Created attachment 55589 [details]
Patch
Comment 5 Chris Marrin 2010-05-10 13:49:38 PDT
Created attachment 55598 [details]
Windows results for new test cases
Comment 6 Anders Carlsson 2010-05-10 13:53:47 PDT
Comment on attachment 55576 [details]
Patch

r=me
Comment 7 Anders Carlsson 2010-05-10 13:54:19 PDT
Comment on attachment 55586 [details]
Add a bunch move compositing iframe tests

rs=me
Comment 8 Anders Carlsson 2010-05-10 14:02:09 PDT
Comment on attachment 55589 [details]
Patch

r=me
Comment 9 Anders Carlsson 2010-05-10 14:05:02 PDT
Comment on attachment 55598 [details]
Windows results for new test cases

rs=me
Comment 10 Chris Marrin 2010-05-10 14:08:47 PDT
Created attachment 55601 [details]
Replace patch for Windows results
Comment 11 Simon Fraser (smfr) 2010-05-10 14:25:13 PDT
Created attachment 55603 [details]
Patch to have RenderLayerCompositor track the type of attachment for the root layer.
Comment 12 Simon Fraser (smfr) 2010-05-10 16:03:57 PDT
Created attachment 55616 [details]
Allow run-time switching of iframe propagation
Comment 13 Simon Fraser (smfr) 2010-05-10 21:22:04 PDT
Created attachment 55656 [details]
Patch
Comment 14 Anders Carlsson 2010-05-10 21:26:30 PDT
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
Comment 15 Anders Carlsson 2010-05-10 21:29:01 PDT
Comment on attachment 55616 [details]
Allow run-time switching of iframe propagation

r=me
Comment 16 Simon Fraser (smfr) 2010-05-10 21:44:16 PDT
Created attachment 55659 [details]
Patch
Comment 17 Simon Fraser (smfr) 2010-05-10 21:47:16 PDT
Created attachment 55661 [details]
Patch
Comment 18 Anders Carlsson 2010-05-10 21:50:49 PDT
Comment on attachment 55656 [details]
Patch


> +2010-05-10  Simon Fraser  <simon.fraser@apple.com>
> +
> +        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.

> +
> +2010-05-10  Simon Fraser  <simon.fraser@apple.com>
> +
> +        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
Comment 19 Anders Carlsson 2010-05-10 21:53:02 PDT
Comment on attachment 55661 [details]
Patch

r=me!
Comment 20 Anders Carlsson 2010-05-10 22:10:59 PDT
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 @@

> +2010-05-10  Simon Fraser  <simon.fraser@apple.com>
> +
> +        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
Comment 21 Simon Fraser (smfr) 2010-05-10 22:14:05 PDT
Comment on attachment 55576 [details]
Patch

http://trac.webkit.org/changeset/59129
Comment 22 Simon Fraser (smfr) 2010-05-10 22:14:23 PDT
Comment on attachment 55586 [details]
Add a bunch move compositing iframe tests

http://trac.webkit.org/changeset/59130
Comment 23 Simon Fraser (smfr) 2010-05-10 22:23:31 PDT
Comment on attachment 55601 [details]
Replace patch for Windows results

http://trac.webkit.org/changeset/59131
Comment 24 Simon Fraser (smfr) 2010-05-10 22:27:29 PDT
Comment on attachment 55589 [details]
Patch

http://trac.webkit.org/changeset/59132
Comment 25 Simon Fraser (smfr) 2010-05-10 22:33:46 PDT
Comment on attachment 55603 [details]
Patch to have RenderLayerCompositor track the type of attachment for the root layer.

http://trac.webkit.org/changeset/59133
Comment 26 Simon Fraser (smfr) 2010-05-10 22:35:02 PDT
Comment on attachment 55616 [details]
Allow run-time switching of iframe propagation

http://trac.webkit.org/changeset/59134
Comment 27 Simon Fraser (smfr) 2010-05-10 22:44:37 PDT
Comment on attachment 55656 [details]
Patch

http://trac.webkit.org/changeset/59136
Comment 28 Simon Fraser (smfr) 2010-05-10 22:47:27 PDT
Comment on attachment 55661 [details]
Patch

http://trac.webkit.org/changeset/59137
Comment 29 Simon Fraser (smfr) 2010-05-10 22:47:56 PDT
Comment on attachment 55659 [details]
Patch

http://trac.webkit.org/changeset/59137
Comment 30 Simon Fraser (smfr) 2010-05-10 22:48:09 PDT
Comment on attachment 55661 [details]
Patch

http://trac.webkit.org/changeset/59138
Comment 31 Simon Fraser (smfr) 2010-05-10 22:49:15 PDT
All patches landed.
Comment 37 Simon Fraser (smfr) 2010-05-10 23:44:50 PDT
GTK Linux should be fixed by http://trac.webkit.org/changeset/59143