Bug 129919 – Set background color of WK2's UIScrollView to the pageExtendedBackgroundColor

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Beth Dakin]

We should set background color of WK2's UIScrollView to the pageExtendedBackgroundColor, and that information should be passed to the UIProcess via a RemoteLayerTreeTransaction.

[Beth Dakin]

*** Bug 129921 has been marked as a duplicate of this bug. ***

[Beth Dakin]

Created attachment 226171 [details]
Patch

[Beth Dakin]

Created attachment 226181 [details]
Patch

Should fix the build failure.

[Benjamin Poulain]

Comment on attachment 226181 [details]
Patch

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

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h:207
> +    WebCore::Color m_pageExtendedBackgroundColor;

Let's move this above with content size.

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:426
> +    encoder << m_pageExtendedBackgroundColor;

And update this accordingly.

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:492
> +    if (!decoder.decode(result.m_pageExtendedBackgroundColor))
> +        return false;
> +

And this.

> Source/WebKit2/UIProcess/WebPageProxy.h:432
> +    void setPageExtendedBackgroundColor(const WebCore::Color& color) { m_pageExtendedBackgroundColor = color; }

You should not need this.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:340
> +    _page->setPageExtendedBackgroundColor(layerTreeTransaction.pageExtendedBackgroundColor());

This should be done in WebPageProxy::didCommitLayerTree().

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:342
> +        if ([self _backgroundExtendsBeyondPage] && pageExtendedBackgroundColor != [_scrollView backgroundColor])

You may not need pageExtendedBackgroundColor != [_scrollView backgroundColor], UIScrollView might do that already.

> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:880
> +    if (m_page->drawingArea())
> +        m_page->drawingArea()->scheduleCompositingLayerFlush();

Do we really need this? I would think a flush must be scheduled already since the style would invalidate the full frame rect.

[Beth Dakin]

(In reply to comment #4)
> (From update of attachment 226181 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226181&action=review
> 
> > Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h:207
> > +    WebCore::Color m_pageExtendedBackgroundColor;
> 
> Let's move this above with content size.
> 

Okay!

> > Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:426
> > +    encoder << m_pageExtendedBackgroundColor;
> 
> And update this accordingly.
> 

Done.

> > Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:492
> > +    if (!decoder.decode(result.m_pageExtendedBackgroundColor))
> > +        return false;
> > +
> 
> And this.
> 

Done.

> > Source/WebKit2/UIProcess/WebPageProxy.h:432
> > +    void setPageExtendedBackgroundColor(const WebCore::Color& color) { m_pageExtendedBackgroundColor = color; }
> 
> You should not need this.
> 

Removed.

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:340
> > +    _page->setPageExtendedBackgroundColor(layerTreeTransaction.pageExtendedBackgroundColor());
> 
> This should be done in WebPageProxy::didCommitLayerTree().
> 

Moved.

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:342
> > +        if ([self _backgroundExtendsBeyondPage] && pageExtendedBackgroundColor != [_scrollView backgroundColor])
> 
> You may not need pageExtendedBackgroundColor != [_scrollView backgroundColor], UIScrollView might do that already.
> 

Okay, I will remove it if it's not needed.

> > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:880
> > +    if (m_page->drawingArea())
> > +        m_page->drawingArea()->scheduleCompositingLayerFlush();
> 
> Do we really need this? I would think a flush must be scheduled already since the style would invalidate the full frame rect.

It's true that in my testing it doesn't seem like we need to do anything here…

[Beth Dakin]

Committed change with http://trac.webkit.org/changeset/165409