Bug 107213

Summary: Allow fixed background layers to be moved by the ScrollingCoordinator
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, buildbot, dglazkov, eric, jamesr, jonlee, ojan.autocc, rniwa, simon.fraser, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch thorton: review+

Description Simon Fraser (smfr) 2013-01-17 18:35:03 PST
Allow fixed background layers to be moved by the ScrollingCoordinator
Comment 1 Simon Fraser (smfr) 2013-01-17 18:36:24 PST
Created attachment 183345 [details]
Patch
Comment 2 Build Bot 2013-01-17 19:44:14 PST
Comment on attachment 183345 [details]
Patch

Attachment 183345 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15938424

New failing tests:
fast/parser/append-child-followed-by-document-write.html
Comment 3 Build Bot 2013-01-17 22:19:56 PST
Comment on attachment 183345 [details]
Patch

Attachment 183345 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15938492

New failing tests:
platform/mac/tiled-drawing/fixed-background/fixed-body-background-opacity-html.html
platform/mac/tiled-drawing/fixed-background/fixed-body-background-body-layer.html
platform/mac/tiled-drawing/fixed-background/fixed-body-background-transformed-html.html
fast/parser/append-child-followed-by-document-write.html
platform/mac/tiled-drawing/fixed-background/fixed-body-background-zoomed.html
platform/mac/tiled-drawing/fixed-background/fixed-body-background.html
platform/mac/tiled-drawing/fixed-background/fixed-html-background.html
Comment 4 WebKit Review Bot 2013-01-18 00:49:28 PST
Comment on attachment 183345 [details]
Patch

Attachment 183345 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15944426

New failing tests:
fast/parser/append-child-followed-by-document-write.html
Comment 5 WebKit Review Bot 2013-01-18 02:02:16 PST
Comment on attachment 183345 [details]
Patch

Attachment 183345 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15955111

New failing tests:
fast/parser/append-child-followed-by-document-write.html
Comment 6 James Robinson 2013-01-18 10:31:49 PST
Not really a fan of the concept of a 'counter-scrolling' layer - can't we set this up the same was as position:fixed elements?  We'll have to have special logic for both of these for the overscroll bounce effect.
Comment 7 Simon Fraser (smfr) 2013-01-18 10:36:34 PST
The problem with having it like a position:fixed layer is that it then needs special code to insert it in the hierarchy (since it will not be grouped with the other GraphicsLayers owned by the same RenderLayerBacking), and making it work with zooming was really tricky. The counter-scrolling solution came out easier.
Comment 8 Simon Fraser (smfr) 2013-01-18 15:51:17 PST
Created attachment 183564 [details]
Patch
Comment 9 Tim Horton 2013-01-18 16:01:04 PST
Comment on attachment 183564 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:120
> +    void setCounterScrollingLayerDidChange(bool layerDidChange) { m_counterScrollingLayerDidChange = layerDidChange; } // FIXME: Need this?

Well, do you?

> Source/WebCore/page/scrolling/mac/ScrollingStateScrollingNodeMac.mm:2
> + * Copyright (C) 2012 Apple Inc. All rights reserved.

Nope.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1251
> +        else  {
> +            if (FrameView* frameView = view()->frameView())
> +                viewportRect.setLocation(IntPoint(frameView->scrollOffsetForFixedPosition()));
> +        }

Is this actually our style? I'm not positive.
Comment 10 Simon Fraser (smfr) 2013-01-18 16:02:42 PST
Created attachment 183566 [details]
Patch
Comment 11 WebKit Review Bot 2013-01-18 16:06:06 PST
Attachment 183566 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/mac/tiled-drawing/fixed-background/fixed-body-background-zoomed-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Tim Horton 2013-01-18 16:08:49 PST
Comment on attachment 183564 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        fixed root backgounds (those which have background images all of which have

s/backgounds/backgrounds/
Comment 13 Tim Horton 2013-01-18 16:21:05 PST
Comment on attachment 183566 [details]
Patch

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

> Source/WebCore/ChangeLog:24
> +        (WebCore::ScrollingCoordinator::updateMainFrameScrollPosition): Sync or set the postion of the

s/postion/position/
Comment 14 Simon Fraser (smfr) 2013-01-18 17:22:48 PST
http://trac.webkit.org/changeset/140223