[iOS WK2] position:fixed inside accelerated overflow:scroll is jumpy
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Jun 2014 14:50:39 +0000 (14:50 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Jun 2014 14:50:39 +0000 (14:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=134426
<rdar://problem/17474523>

Reviewed by Tim Horton.

After committing a new layer tree (with possibly stale position:fixed layer
positions), we need the scrolling tree to update those positions based on
the current scroll offset.

Source/WebCore:
Give ScrollingTreeScrollingNode an implementation of updateLayersAfterAncestorChange()
which is required to update fixed/sticky child nodes.

* WebCore.exp.in:
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::updateLayersAfterAncestorChange):
* page/scrolling/ScrollingTreeScrollingNode.h:

Source/WebKit2:
To achieve that, implement ScrollingTreeOverflowScrollingNodeIOS::updateLayersAfterAncestorChange()
and have it add to the cumulative delta the difference between the last committed scroll
position and the current scroll position.

Also make sure that ScrollingTreeOverflowScrollingNodeIOS doesn't call back to scrollViewDidScroll()
when we're updating its scroll position inside a scrolling tree commit.

* UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.h:
* UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
(WebKit::ScrollingTreeOverflowScrollingNodeIOS::ScrollingTreeOverflowScrollingNodeIOS):
(WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren):
(WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateLayersAfterAncestorChange):
(WebKit::ScrollingTreeOverflowScrollingNodeIOS::scrollViewDidScroll):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@170568 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.h
Source/WebKit2/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm

index a0c9ce4..66967b9 100644 (file)
@@ -1,3 +1,23 @@
+2014-06-27  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed inside accelerated overflow:scroll is jumpy
+        https://bugs.webkit.org/show_bug.cgi?id=134426
+        <rdar://problem/17474523>
+
+        Reviewed by Tim Horton.
+
+        After committing a new layer tree (with possibly stale position:fixed layer
+        positions), we need the scrolling tree to update those positions based on
+        the current scroll offset.
+        
+        Give ScrollingTreeScrollingNode an implementation of updateLayersAfterAncestorChange()
+        which is required to update fixed/sticky child nodes.
+
+        * WebCore.exp.in:
+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
+        (WebCore::ScrollingTreeScrollingNode::updateLayersAfterAncestorChange):
+        * page/scrolling/ScrollingTreeScrollingNode.h:
+
 2014-06-28  Juan A. Suarez Romero  <jasuarez@igalia.com>  and  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Use public getter/setter in GObject DOM bindings properties implementation.
index 8fc2d3b..0df5ad9 100644 (file)
@@ -2878,6 +2878,7 @@ __ZN7WebCore25AsyncScrollingCoordinatorD2Ev
 __ZN7WebCore26ScrollingTreeScrollingNode17setScrollPositionERKNS_10FloatPointE
 __ZN7WebCore26ScrollingTreeScrollingNode19updateAfterChildrenERKNS_18ScrollingStateNodeE
 __ZN7WebCore26ScrollingTreeScrollingNode20updateBeforeChildrenERKNS_18ScrollingStateNodeE
+__ZN7WebCore26ScrollingTreeScrollingNode31updateLayersAfterAncestorChangeERKNS_17ScrollingTreeNodeERKNS_9FloatRectERKNS_9FloatSizeE
 __ZN7WebCore26ScrollingTreeScrollingNode46setScrollPositionWithoutContentEdgeConstraintsERKNS_10FloatPointE
 __ZN7WebCore27ScrollingStateScrollingNode15setScrollOriginERKNS_8IntPointE
 __ZN7WebCore27ScrollingStateScrollingNode17setScrollPositionERKNS_10FloatPointE
@@ -3024,6 +3025,12 @@ __ZN7WebCore8Document35webkitWillEnterFullScreenForElementEPNS_7ElementE
 __ZNK7WebCore7Element25containsFullScreenElementEv
 #endif
 
+#if ENABLE(GAMEPAD)
+__ZN7WebCore15GamepadProvider17setSharedProviderERS0_
+__ZN7WebCore15GamepadProvider6sharedEv
+__ZN7WebCore18HIDGamepadProvider6sharedEv
+#endif
+
 #if ENABLE(GEOLOCATION)
 __ZN7WebCore11Geolocation12setIsAllowedEb
 __ZN7WebCore11GeolocationD1Ev
@@ -3442,9 +3449,3 @@ __ZN7WebCore4Page11setViewModeENS0_8ViewModeE
 #if ENABLE(WEB_TIMING)
 __ZNK7WebCore20ResourceResponseBase18resourceLoadTimingEv
 #endif
-
-#if ENABLE(GAMEPAD)
-__ZN7WebCore15GamepadProvider17setSharedProviderERS0_
-__ZN7WebCore15GamepadProvider6sharedEv
-__ZN7WebCore18HIDGamepadProvider6sharedEv
-#endif
index 776f52e..12c30a5 100644 (file)
@@ -78,6 +78,15 @@ void ScrollingTreeScrollingNode::updateAfterChildren(const ScrollingStateNode& s
         scrollingTree().scrollingTreeNodeRequestsScroll(scrollingNodeID(), scrollingStateNode.requestedScrollPosition(), scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll());
 }
 
+void ScrollingTreeScrollingNode::updateLayersAfterAncestorChange(const ScrollingTreeNode& changedNode, const FloatRect& fixedPositionRect, const FloatSize& cumulativeDelta)
+{
+    if (!m_children)
+        return;
+
+    for (auto& child : *m_children)
+        child->updateLayersAfterAncestorChange(changedNode, fixedPositionRect, cumulativeDelta);
+}
+
 void ScrollingTreeScrollingNode::setScrollPosition(const FloatPoint& scrollPosition)
 {
     FloatPoint newScrollPosition = scrollPosition;
index fc9c4e9..d5be9a0 100644 (file)
@@ -46,8 +46,7 @@ public:
     virtual void updateBeforeChildren(const ScrollingStateNode&) override;
     virtual void updateAfterChildren(const ScrollingStateNode&) override;
 
-    // FIXME: We should implement this when we support ScrollingTreeScrollingNodes as children.
-    virtual void updateLayersAfterAncestorChange(const ScrollingTreeNode& /*changedNode*/, const FloatRect& /*fixedPositionRect*/, const FloatSize& /*cumulativeDelta*/) override { }
+    virtual void updateLayersAfterAncestorChange(const ScrollingTreeNode& changedNode, const FloatRect& fixedPositionRect, const FloatSize& cumulativeDelta) override;
 
     virtual void handleWheelEvent(const PlatformWheelEvent&) = 0;
     virtual void setScrollPosition(const FloatPoint&);
index d67b640..c795269 100644 (file)
@@ -1,3 +1,29 @@
+2014-06-27  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed inside accelerated overflow:scroll is jumpy
+        https://bugs.webkit.org/show_bug.cgi?id=134426
+        <rdar://problem/17474523>
+
+        Reviewed by Tim Horton.
+
+        After committing a new layer tree (with possibly stale position:fixed layer
+        positions), we need the scrolling tree to update those positions based on
+        the current scroll offset.
+        
+        To achieve that, implement ScrollingTreeOverflowScrollingNodeIOS::updateLayersAfterAncestorChange()
+        and have it add to the cumulative delta the difference between the last committed scroll
+        position and the current scroll position.
+        
+        Also make sure that ScrollingTreeOverflowScrollingNodeIOS doesn't call back to scrollViewDidScroll()
+        when we're updating its scroll position inside a scrolling tree commit.
+
+        * UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.h:
+        * UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
+        (WebKit::ScrollingTreeOverflowScrollingNodeIOS::ScrollingTreeOverflowScrollingNodeIOS):
+        (WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren):
+        (WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateLayersAfterAncestorChange):
+        (WebKit::ScrollingTreeOverflowScrollingNodeIOS::scrollViewDidScroll):
+
 2014-06-27  Antti Koivisto  <antti@apple.com>
 
         Flush throttling with remote layers
index cb7440a..c3f53f7 100644 (file)
@@ -58,6 +58,8 @@ private:
     virtual void updateLayersAfterViewportChange(const WebCore::FloatRect& fixedPositionRect, double scale) { }
     virtual void updateLayersAfterDelegatedScroll(const WebCore::FloatPoint& scrollPosition) override;
 
+    virtual void updateLayersAfterAncestorChange(const WebCore::ScrollingTreeNode& changedNode, const WebCore::FloatRect& fixedPositionRect, const WebCore::FloatSize& cumulativeDelta) override;
+
     virtual void handleWheelEvent(const WebCore::PlatformWheelEvent&) override { }
 
     void updateChildNodesAfterScroll(const WebCore::FloatPoint&);
@@ -66,6 +68,7 @@ private:
     RetainPtr<CALayer> m_scrolledContentsLayer;
 
     RetainPtr<WKOverflowScrollViewDelegate> m_scrollViewDelegate;
+    bool m_updatingFromStateNode;
 };
 
 } // namespace WebKit
index c6e2cf8..31d899d 100644 (file)
@@ -35,6 +35,7 @@
 #import <WebCore/ScrollingTree.h>
 #import <UIKit/UIPanGestureRecognizer.h>
 #import <UIKit/UIScrollView.h>
+#import <wtf/TemporaryChange.h>
 
 using namespace WebCore;
 
@@ -98,6 +99,7 @@ PassRefPtr<ScrollingTreeOverflowScrollingNodeIOS> ScrollingTreeOverflowScrolling
 
 ScrollingTreeOverflowScrollingNodeIOS::ScrollingTreeOverflowScrollingNodeIOS(WebCore::ScrollingTree& scrollingTree, WebCore::ScrollingNodeID nodeID)
     : ScrollingTreeOverflowScrollingNode(scrollingTree, nodeID)
+    , m_updatingFromStateNode(false)
 {
 }
 
@@ -136,6 +138,8 @@ void ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren(const ScrollingS
 {
     ScrollingTreeOverflowScrollingNode::updateAfterChildren(stateNode);
 
+    TemporaryChange<bool> updatingChange(m_updatingFromStateNode, true);
+
     const auto& scrollingStateNode = toScrollingStateOverflowScrollingNode(stateNode);
 
     if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollLayer)
@@ -166,7 +170,7 @@ void ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren(const ScrollingS
             scrollView.contentOffset = scrollingStateNode.scrollPosition() + scrollOrigin();
             recomputeInsets = true;
         }
-        
+
         if (recomputeInsets) {
             UIEdgeInsets insets = UIEdgeInsetsMake(0, 0, 0, 0);
             // With RTL or bottom-to-top scrolling (non-zero origin), we need extra space on the left or top.
@@ -183,6 +187,17 @@ void ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren(const ScrollingS
     }
 }
 
+void ScrollingTreeOverflowScrollingNodeIOS::updateLayersAfterAncestorChange(const ScrollingTreeNode& changedNode, const FloatRect& fixedPositionRect, const FloatSize& cumulativeDelta)
+{
+    if (!m_children)
+        return;
+
+    FloatSize scrollDelta = lastCommittedScrollPosition() - scrollPosition();
+
+    for (auto& child : *m_children)
+        child->updateLayersAfterAncestorChange(changedNode, fixedPositionRect, cumulativeDelta + scrollDelta);
+}
+
 FloatPoint ScrollingTreeOverflowScrollingNodeIOS::scrollPosition() const
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS
@@ -223,6 +238,9 @@ void ScrollingTreeOverflowScrollingNodeIOS::overflowScrollViewWillStartPanGestur
 
 void ScrollingTreeOverflowScrollingNodeIOS::scrollViewDidScroll(const FloatPoint& scrollPosition, bool inUserInteration)
 {
+    if (m_updatingFromStateNode)
+        return;
+
     scrollingTree().scrollPositionChangedViaDelegatedScrolling(scrollingNodeID(), scrollPosition, inUserInteration);
 }