Title: [242618] tags/Safari-608.1.8/Source/WebCore
- Revision
- 242618
- Author
- kocsen_ch...@apple.com
- Date
- 2019-03-07 16:10:54 -0800 (Thu, 07 Mar 2019)
Log Message
Cherry-pick r242601. rdar://problem/48518959
[iOS WK] REGRESSION (r242132): Fixed position banners flicker and move when scrolling (Apple, Tesla, YouTube, Reddit)
https://bugs.webkit.org/show_bug.cgi?id=195396
rdar://problem/48518959
Reviewed by Antti Koivisto.
r242132 introduced two issues that contributed to jumpiness of position:fixed layers when scrolling.
First, ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling() would early return if the scroll position
hadn't changed. It also needs to check the supplied layoutViewport (if any), but in some cases running the
notifyRelatedNodesAfterScrollPositionChange() code is necessary even without a scroll position change:
if the web process has committed new scrolling tree state (e.g. with new fixed constraints) since
the last call, we have to run the layer positioning code to have fixed layers re-adjust their position relative
to the root. This was the primary bug fix.
Secondly, a layer tree commit can give ScrollingTreeFrameScrollingNode a new layout viewport, but we need to
adjust this by the scrolling tree's current scroll position in case it gets used before the next scroll.
Currently no way to test this, as it's very timing-dependent.
* page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
(WebCore::ScrollingTreeFrameScrollingNode::commitStateBeforeChildren):
(WebCore::ScrollingTreeFrameScrollingNode::scrollPositionAndLayoutViewportMatch):
* page/scrolling/ScrollingTreeFrameScrollingNode.h:
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::scrollPositionAndLayoutViewportMatch):
(WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
* page/scrolling/ScrollingTreeScrollingNode.h:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: tags/Safari-608.1.8/Source/WebCore/ChangeLog (242617 => 242618)
--- tags/Safari-608.1.8/Source/WebCore/ChangeLog 2019-03-07 23:50:57 UTC (rev 242617)
+++ tags/Safari-608.1.8/Source/WebCore/ChangeLog 2019-03-08 00:10:54 UTC (rev 242618)
@@ -1,5 +1,72 @@
2019-03-07 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r242601. rdar://problem/48518959
+
+ [iOS WK] REGRESSION (r242132): Fixed position banners flicker and move when scrolling (Apple, Tesla, YouTube, Reddit)
+ https://bugs.webkit.org/show_bug.cgi?id=195396
+ rdar://problem/48518959
+
+ Reviewed by Antti Koivisto.
+
+ r242132 introduced two issues that contributed to jumpiness of position:fixed layers when scrolling.
+
+ First, ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling() would early return if the scroll position
+ hadn't changed. It also needs to check the supplied layoutViewport (if any), but in some cases running the
+ notifyRelatedNodesAfterScrollPositionChange() code is necessary even without a scroll position change:
+ if the web process has committed new scrolling tree state (e.g. with new fixed constraints) since
+ the last call, we have to run the layer positioning code to have fixed layers re-adjust their position relative
+ to the root. This was the primary bug fix.
+
+ Secondly, a layer tree commit can give ScrollingTreeFrameScrollingNode a new layout viewport, but we need to
+ adjust this by the scrolling tree's current scroll position in case it gets used before the next scroll.
+
+ Currently no way to test this, as it's very timing-dependent.
+
+ * page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
+ (WebCore::ScrollingTreeFrameScrollingNode::commitStateBeforeChildren):
+ (WebCore::ScrollingTreeFrameScrollingNode::scrollPositionAndLayoutViewportMatch):
+ * page/scrolling/ScrollingTreeFrameScrollingNode.h:
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::scrollPositionAndLayoutViewportMatch):
+ (WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-03-07 Simon Fraser <simon.fra...@apple.com>
+
+ [iOS WK] REGRESSION (r242132): Fixed position banners flicker and move when scrolling (Apple, Tesla, YouTube, Reddit)
+ https://bugs.webkit.org/show_bug.cgi?id=195396
+ rdar://problem/48518959
+
+ Reviewed by Antti Koivisto.
+
+ r242132 introduced two issues that contributed to jumpiness of position:fixed layers when scrolling.
+
+ First, ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling() would early return if the scroll position
+ hadn't changed. It also needs to check the supplied layoutViewport (if any), but in some cases running the
+ notifyRelatedNodesAfterScrollPositionChange() code is necessary even without a scroll position change:
+ if the web process has committed new scrolling tree state (e.g. with new fixed constraints) since
+ the last call, we have to run the layer positioning code to have fixed layers re-adjust their position relative
+ to the root. This was the primary bug fix.
+
+ Secondly, a layer tree commit can give ScrollingTreeFrameScrollingNode a new layout viewport, but we need to
+ adjust this by the scrolling tree's current scroll position in case it gets used before the next scroll.
+
+ Currently no way to test this, as it's very timing-dependent.
+
+ * page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
+ (WebCore::ScrollingTreeFrameScrollingNode::commitStateBeforeChildren):
+ (WebCore::ScrollingTreeFrameScrollingNode::scrollPositionAndLayoutViewportMatch):
+ * page/scrolling/ScrollingTreeFrameScrollingNode.h:
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::scrollPositionAndLayoutViewportMatch):
+ (WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
+2019-03-07 Kocsen Chung <kocsen_ch...@apple.com>
+
Revert r242297. rdar://problem/48686655
2019-03-06 Rob Buis <rb...@igalia.com>
Modified: tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.cpp (242617 => 242618)
--- tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.cpp 2019-03-07 23:50:57 UTC (rev 242617)
+++ tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.cpp 2019-03-08 00:10:54 UTC (rev 242618)
@@ -72,8 +72,10 @@
if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::FixedElementsLayoutRelativeToFrame))
m_fixedElementsLayoutRelativeToFrame = state.fixedElementsLayoutRelativeToFrame();
- if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::LayoutViewport))
+ if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::LayoutViewport)) {
m_layoutViewport = state.layoutViewport();
+ updateViewportForCurrentScrollPosition({ });
+ }
if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::MinLayoutViewportOrigin))
m_minLayoutViewportOrigin = state.minLayoutViewportOrigin();
@@ -82,6 +84,11 @@
m_maxLayoutViewportOrigin = state.maxLayoutViewportOrigin();
}
+bool ScrollingTreeFrameScrollingNode::scrollPositionAndLayoutViewportMatch(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport)
+{
+ return position == currentScrollPosition() && (!overrideLayoutViewport || overrideLayoutViewport.value() == m_layoutViewport);
+}
+
FloatRect ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition(const FloatPoint& visibleContentOrigin, float scale) const
{
FloatRect visibleContentRect(visibleContentOrigin, scrollableAreaSize());
Modified: tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h (242617 => 242618)
--- tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h 2019-03-07 23:50:57 UTC (rev 242617)
+++ tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h 2019-03-08 00:10:54 UTC (rev 242618)
@@ -70,6 +70,7 @@
LayoutPoint localToContentsPoint(LayoutPoint) const final;
WEBCORE_EXPORT void updateViewportForCurrentScrollPosition(Optional<FloatRect>) override;
+ bool scrollPositionAndLayoutViewportMatch(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport) override;
void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
Modified: tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (242617 => 242618)
--- tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-03-07 23:50:57 UTC (rev 242617)
+++ tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-03-08 00:10:54 UTC (rev 242618)
@@ -174,10 +174,17 @@
scrollingTree().scrollingTreeNodeDidScroll(*this);
}
+bool ScrollingTreeScrollingNode::scrollPositionAndLayoutViewportMatch(const FloatPoint& position, Optional<FloatRect>)
+{
+ return position == m_currentScrollPosition;
+}
+
void ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport)
{
- if (position == m_currentScrollPosition)
- return;
+ // Even if position and overrideLayoutViewport haven't changed for this node, other nodes may have received new constraint data
+ // via a commit, so the call to notifyRelatedNodesAfterScrollPositionChange() is necessary. We could avoid this if we knew that
+ // no commits had happened.
+ bool scrollPositionChanged = !scrollPositionAndLayoutViewportMatch(position, overrideLayoutViewport);
m_currentScrollPosition = adjustedScrollPosition(position, ScrollPositionClamp::None);
updateViewportForCurrentScrollPosition(overrideLayoutViewport);
@@ -185,7 +192,9 @@
repositionRelatedLayers();
scrollingTree().notifyRelatedNodesAfterScrollPositionChange(*this);
- scrollingTree().scrollingTreeNodeDidScroll(*this);
+
+ if (scrollPositionChanged)
+ scrollingTree().scrollingTreeNodeDidScroll(*this);
}
LayoutPoint ScrollingTreeScrollingNode::parentToLocalPoint(LayoutPoint point) const
Modified: tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (242617 => 242618)
--- tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-03-07 23:50:57 UTC (rev 242617)
+++ tags/Safari-608.1.8/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-03-08 00:10:54 UTC (rev 242618)
@@ -93,6 +93,7 @@
virtual void currentScrollPositionChanged();
WEBCORE_EXPORT virtual void updateViewportForCurrentScrollPosition(Optional<FloatRect> = { }) { }
+ virtual bool scrollPositionAndLayoutViewportMatch(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport);
WEBCORE_EXPORT virtual void repositionScrollingLayers() { }
WEBCORE_EXPORT virtual void repositionRelatedLayers() { }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
