Bug 129919 - Set background color of WK2's UIScrollView to the pageExtendedBackgroundColor
Summary: Set background color of WK2's UIScrollView to the pageExtendedBackgroundColor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 129921 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-03-07 14:33 PST by Beth Dakin
Modified: 2014-03-10 15:43 PDT (History)
8 users (show)

See Also:


Attachments
Patch (11.02 KB, patch)
2014-03-07 15:03 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (11.08 KB, patch)
2014-03-07 16:06 PST, Beth Dakin
benjamin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2014-03-07 14:33:25 PST
We should set background color of WK2's UIScrollView to the pageExtendedBackgroundColor, and that information should be passed to the UIProcess via a RemoteLayerTreeTransaction.
Comment 1 Beth Dakin 2014-03-07 14:37:31 PST
*** Bug 129921 has been marked as a duplicate of this bug. ***
Comment 2 Beth Dakin 2014-03-07 15:03:33 PST
Created attachment 226171 [details]
Patch
Comment 3 Beth Dakin 2014-03-07 16:06:03 PST
Created attachment 226181 [details]
Patch

Should fix the build failure.
Comment 4 Benjamin Poulain 2014-03-07 16:45:04 PST
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.
Comment 5 Beth Dakin 2014-03-10 11:58:34 PDT
(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…
Comment 6 Beth Dakin 2014-03-10 15:43:40 PDT
Committed change with http://trac.webkit.org/changeset/165409