Bug 42771

Summary: Composited layers don't scroll in WebKit2
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: WebKit2Assignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, fishd, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Patch for some prep work
none
More prep work: single bottleneck method
none
Patch andersca: review+

Description Simon Fraser (smfr) 2010-07-21 10:52:39 PDT
In WebKit2, composited layers don't scroll with the rest of the page.
Comment 1 Simon Fraser (smfr) 2010-07-21 12:20:37 PDT
Created attachment 62217 [details]
Patch for some prep work
Comment 2 Simon Fraser (smfr) 2010-07-21 14:07:47 PDT
Comment on attachment 62217 [details]
Patch for some prep work

http://trac.webkit.org/changeset/63856
Comment 3 James Robinson 2010-07-21 21:54:26 PDT
I tried the patch at http://pastie.org/1054420 in a Chromium mac build and it appears to work fine - the scrolling repaint pixel tests all pass and the scrolling behavior appears correct when manually tested as well.
Comment 4 Simon Fraser (smfr) 2010-07-22 11:54:00 PDT
Created attachment 62322 [details]
More prep work: single bottleneck method
Comment 5 Simon Fraser (smfr) 2010-07-22 13:19:12 PDT
Comment on attachment 62322 [details]
More prep work: single bottleneck method

http://trac.webkit.org/changeset/63907
Comment 6 Simon Fraser (smfr) 2010-07-22 17:29:06 PDT
Created attachment 62365 [details]
Patch
Comment 7 Eric Seidel (no email) 2010-07-22 18:41:28 PDT
Comment on attachment 62217 [details]
Patch for some prep work

Cleared Anders Carlsson's review+ from obsolete attachment 62217 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 8 Eric Seidel (no email) 2010-07-22 18:41:34 PDT
Comment on attachment 62322 [details]
More prep work: single bottleneck method

Cleared Darin Adler's review+ from obsolete attachment 62322 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 9 Anders Carlsson 2010-07-26 10:18:09 PDT
Comment on attachment 62365 [details]
Patch

> diff --git a/WebCore/rendering/RenderLayerCompositor.cpp b/WebCore/rendering/RenderLayerCompositor.cpp
> index 234ad7f270191ef8a7ba5870b072d875bf6f7423..14d45927b3ca31cce19bce537343bbd90ccf8553 100644
> --- a/WebCore/rendering/RenderLayerCompositor.cpp
> +++ b/WebCore/rendering/RenderLayerCompositor.cpp
> -void RenderLayerCompositor::updateContentLayerScrollPosition(const IntPoint& scrollPosition)
> +void RenderLayerCompositor::frameViewDidScroll(const IntPoint& scrollPosition)
>  {
>      if (m_scrollLayer)
>          m_scrollLayer->setPosition(FloatPoint(-scrollPosition.x(), -scrollPosition.y()));
> @@ -1039,6 +1039,9 @@ bool RenderLayerCompositor::shouldPropagateCompositingToEnclosingIFrame() const
>      // On non-Mac platforms, let compositing propagate for all iframes.
>      return true;
>  #else
> +    if (!m_renderView->frameView()->platformWidget())
> +        return true;
> +
Could you add a comment about why we check for a null platform widget here?

Looks fine otherwise!
Comment 10 Simon Fraser (smfr) 2010-07-26 10:48:49 PDT
http://trac.webkit.org/changeset/64054
Comment 11 WebKit Review Bot 2010-07-26 11:58:48 PDT
http://trac.webkit.org/changeset/64054 might have broken Leopard Intel Debug (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/64053
http://trac.webkit.org/changeset/64054
Comment 12 Darin Fisher (:fishd, Google) 2010-08-16 16:36:35 PDT
This patch broke scrolling in Chromium when there is a fixed position element.

Chromium bug:
http://code.google.com/p/chromium/issues/detail?id=52332

It seems like we end up missing some invalidates after scrolling.
Comment 13 Simon Fraser (smfr) 2010-08-16 16:48:56 PDT
(In reply to comment #12)
> This patch broke scrolling in Chromium when there is a fixed position element.
> 
> Chromium bug:
> http://code.google.com/p/chromium/issues/detail?id=52332
> 
> It seems like we end up missing some invalidates after scrolling.

Sounds like bug 42949.