Created attachment 327452 [details] Testcase AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions has the following comment: // FIXME: We'll have to traverse deeper into the tree at some point. for (auto& child : *children) child->reconcileLayerPositionForViewportRect(viewportRect, action); I'm not sure whether this is affecting bug 154399 but the attached testcase shows that reconcileLayerPositionForViewportRect is not called on the fixed node when one scrolls the main frame.
Created attachment 327453 [details] WIP Patch This is patch to recursively calls reconcileLayerPositionForViewportRect but I'm not sure how to emulate non-programmatic scroll on iOS in order to write a test.
Created attachment 331334 [details] Patch
Comment on attachment 331334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331334&action=review > Source/WebCore/page/scrolling/ScrollingStateNode.cpp:103 > +void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action) ScrollingStateStickyNode::reconcileLayerPositionForViewportRect and ScrollingStateFixedNode::reconcileLayerPositionForViewportRect don’t call through to ScrollingStateNode::reconcileLayerPositionForViewportRect. Can those nodes have children? Is it OK to not call reconcileLayerPositionForViewportRect on those children?
Created attachment 331376 [details] Patch
Comment on attachment 331334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331334&action=review >> Source/WebCore/page/scrolling/ScrollingStateNode.cpp:103 >> +void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action) > > ScrollingStateStickyNode::reconcileLayerPositionForViewportRect and ScrollingStateFixedNode::reconcileLayerPositionForViewportRect don’t call through to ScrollingStateNode::reconcileLayerPositionForViewportRect. > > Can those nodes have children? Is it OK to not call reconcileLayerPositionForViewportRect on those children? Yes, I guess sticky/fixed nodes should also call reconcileLayerPositionForViewportRect on their descendants. I modified the code and extended the tests accordingly.
For the record, here is the diff of the graphic layers of the test before and after the patch: diff --git a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt index fc9b542a67d..1b6f6dbeaba 100644 --- a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt +++ b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt @@ -70,44 +70,48 @@ (contentsScale 2.00) (children 2 (GraphicsLayer - (position 7.00 -993.00) + (position 7.00 28.00) + (approximate position 7.00 28.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 0.00 x 0.00) - (coverage rect -7.00, 993.00 200.00 x 245.00) + (visible rect 0.00, 0.00 110.00 x 110.00) + (coverage rect -7.00, -28.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) (children 1 (GraphicsLayer (position 55.00 55.00) + (approximate position 55.00 55.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 0.00 x 0.00) - (coverage rect -62.00, 938.00 200.00 x 245.00) + (visible rect 0.00, 0.00 110.00 x 110.00) + (coverage rect -62.00, -83.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) ) ) ) (GraphicsLayer - (position 105.00 105.00) + (position 105.00 395.00) + (approximate position 105.00 395.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 95.00 x 110.00) - (coverage rect -105.00, -105.00 200.00 x 245.00) + (visible rect 0.00, 0.00 0.00 x 0.00) + (coverage rect -105.00, -395.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) (children 1 (GraphicsLayer (position 55.00 55.00) + (approximate position 55.00 55.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 40.00 x 85.00) - (coverage rect -160.00, -160.00 200.00 x 245.00) + (visible rect 0.00, 0.00 0.00 x 0.00) + (coverage rect -160.00, -450.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) )
@Simon: Can you please review that patch?
Committed r227596: <https://trac.webkit.org/changeset/227596>
<rdar://problem/36857233>