Diff
Modified: trunk/LayoutTests/ChangeLog (243925 => 243926)
--- trunk/LayoutTests/ChangeLog 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/LayoutTests/ChangeLog 2019-04-05 05:18:17 UTC (rev 243926)
@@ -1,3 +1,17 @@
+2019-04-04 Simon Fraser <[email protected]>
+
+ [iOS WK2] REGRESSION (r242687): Programmatic scroll of overflow scroll results in bad rendering
+ https://bugs.webkit.org/show_bug.cgi?id=195584
+
+ Reviewed by Zalan Bujtas.
+
+ Testing of programmatic scrolls in frames is prevented by webkit.org/b/196635.
+
+ * scrollingcoordinator/ios/programmatic-overflow-scroll-expected.html: Added.
+ * scrollingcoordinator/ios/programmatic-overflow-scroll.html: Added.
+ * scrollingcoordinator/ios/programmatic-page-scroll-expected.html: Added.
+ * scrollingcoordinator/ios/programmatic-page-scroll.html: Added.
+
2019-04-04 Shawn Roberts <[email protected]>
Unreviewed, rolling out r243868.
Added: trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll-expected.html (0 => 243926)
--- trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll-expected.html 2019-04-05 05:18:17 UTC (rev 243926)
@@ -0,0 +1,33 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name="viewport" content="width=device-width">
+ <style>
+ #scroller {
+ height: 300px;
+ width: 300px;
+ overflow: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .box {
+ width: 100%;
+ height: 100%;
+ }
+
+ .top {
+ background-color: green;
+ }
+
+ .bottom {
+ background-color: green;
+ }
+ </style>
+</head>
+<body>
+ <div id="scroller">
+ <div class="top box"></div>
+ <div class="bottom box"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll.html (0 => 243926)
--- trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/programmatic-overflow-scroll.html 2019-04-05 05:18:17 UTC (rev 243926)
@@ -0,0 +1,42 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name="viewport" content="width=device-width">
+ <style>
+ #scroller {
+ height: 300px;
+ width: 300px;
+ overflow: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .box {
+ width: 100%;
+ height: 100%;
+ }
+
+ .top {
+ background-color: red;
+ }
+
+ .bottom {
+ background-color: green;
+ }
+
+ </style>
+ <script>
+ function doTest()
+ {
+ scroller.scrollTop = 500;
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="top box"></div>
+ <div class="bottom box"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll-expected.html (0 => 243926)
--- trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll-expected.html 2019-04-05 05:18:17 UTC (rev 243926)
@@ -0,0 +1,52 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name="viewport" content="width=device-width">
+ <style>
+ body {
+ height: 2000px;
+ }
+ #scroller {
+ height: 300px;
+ width: 300px;
+ overflow: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .box {
+ width: 100%;
+ height: 100%;
+ }
+
+ .top {
+ background-color: red;
+ }
+
+ .bottom {
+ background-color: green;
+ }
+
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ function doTest()
+ {
+ setTimeout(() => {
+ document.scrollingElement.scrollTop = 200;
+ }, 2000);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="top box"></div>
+ <div class="bottom box"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll.html (0 => 243926)
--- trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/programmatic-page-scroll.html 2019-04-05 05:18:17 UTC (rev 243926)
@@ -0,0 +1,52 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name="viewport" content="width=device-width">
+ <style>
+ body {
+ height: 2000px;
+ }
+ #scroller {
+ height: 300px;
+ width: 300px;
+ overflow: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .box {
+ width: 100%;
+ height: 100%;
+ }
+
+ .top {
+ background-color: red;
+ }
+
+ .bottom {
+ background-color: green;
+ }
+
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ function doTest()
+ {
+ setTimeout(() => {
+ document.scrollingElement.scrollTop = 200;
+ }, 2000);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="top box"></div>
+ <div class="bottom box"></div>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (243925 => 243926)
--- trunk/Source/WebCore/ChangeLog 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/ChangeLog 2019-04-05 05:18:17 UTC (rev 243926)
@@ -1,3 +1,55 @@
+2019-04-04 Simon Fraser <[email protected]>
+
+ [iOS WK2] REGRESSION (r242687): Programmatic scroll of overflow scroll results in bad rendering
+ https://bugs.webkit.org/show_bug.cgi?id=195584
+
+ Reviewed by Zalan Bujtas.
+
+ Push data to the scrolling tree about whether an overflow:scroll scroll was programmatic, by having
+ RenderLayer::scrollToOffset() call into AsyncScrollingCoordinator::requestScrollPositionUpdate(),
+ just as we do for frames.
+
+ AsyncScrollingCoordinator::requestScrollPositionUpdate() is generalized to take any ScrollableArea.
+
+ Fix an assumption in the ScrollingTree that we only care about programmatic scrolls on the root node.
+ ScrollingTree::commitTreeState() no longer sets isHandlingProgrammaticScroll; instead,
+ callers of ScrollingTreeScrollingNode::scrollTo() pass a ScrollType. Commit functions pass
+ ScrollType::Programmatic when handling RequestedScrollPosition changes as necessary.
+
+ Programmatic scrolls need to get to the scrolling tree in the UI process so that we update
+ the tree's notion of scroll position, and trigger actual UIScrollView scrolls (layers may have
+ already been put in the right locations, but the UI process needs to know that a scroll happened).
+ However, we need to prevent notifications from programmatic scrolls getting back to the
+ web process, because this causes jumpiness. This is done via an early return in
+ RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidScroll().
+
+ Tests: scrollingcoordinator/ios/programmatic-overflow-scroll.html
+ scrollingcoordinator/ios/programmatic-page-scroll.html
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate):
+ * page/scrolling/AsyncScrollingCoordinator.h:
+ * page/scrolling/ScrollingCoordinator.h:
+ (WebCore::ScrollingCoordinator::requestScrollPositionUpdate):
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::commitTreeState):
+ (WebCore::ScrollingTree::isHandlingProgrammaticScroll): Deleted.
+ * page/scrolling/ScrollingTree.h:
+ (WebCore::ScrollingTree::isHandlingProgrammaticScroll const):
+ (WebCore::ScrollingTree::setIsHandlingProgrammaticScroll):
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::scrollBy):
+ (WebCore::ScrollingTreeScrollingNode::scrollTo):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+ * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::commitStateAfterChildren):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::scrollToOffset):
+ (WebCore::RenderLayer::scrollingNodeID const):
+ * rendering/RenderLayer.h:
+ * rendering/RenderMarquee.cpp:
+ (WebCore::RenderMarquee::timerFired):
+
2019-04-04 Yusuke Suzuki <[email protected]>
Unreviewed, speculative fix for build failure
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -204,18 +204,26 @@
node->setHorizontalScrollbarLayer(frameView.layerForHorizontalScrollbar());
}
-bool AsyncScrollingCoordinator::requestScrollPositionUpdate(FrameView& frameView, const IntPoint& scrollPosition)
+bool AsyncScrollingCoordinator::requestScrollPositionUpdate(ScrollableArea& scrollableArea, const IntPoint& scrollPosition)
{
ASSERT(isMainThread());
ASSERT(m_page);
- if (!coordinatesScrollingForFrameView(frameView))
+ auto scrollingNodeID = scrollableArea.scrollingNodeID();
+ if (!scrollingNodeID)
return false;
- bool inPageCache = frameView.frame().document()->pageCacheState() != Document::NotInPageCache;
- bool inProgrammaticScroll = frameView.currentScrollType() == ScrollType::Programmatic;
+ auto* frameView = frameViewForScrollingNode(scrollingNodeID);
+ if (!frameView)
+ return false;
+
+ if (!coordinatesScrollingForFrameView(*frameView))
+ return false;
+
+ bool inPageCache = frameView->frame().document()->pageCacheState() != Document::NotInPageCache;
+ bool inProgrammaticScroll = scrollableArea.currentScrollType() == ScrollType::Programmatic;
if (inProgrammaticScroll || inPageCache)
- updateScrollPositionAfterAsyncScroll(frameView.scrollingNodeID(), scrollPosition, { }, ScrollType::Programmatic, ScrollingLayerPositionAction::Set);
+ updateScrollPositionAfterAsyncScroll(scrollingNodeID, scrollPosition, { }, ScrollType::Programmatic, ScrollingLayerPositionAction::Set);
// If this frame view's document is being put into the page cache, we don't want to update our
// main frame scroll position. Just let the FrameView think that we did.
@@ -222,7 +230,7 @@
if (inPageCache)
return true;
- auto* stateNode = downcast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
+ auto* stateNode = downcast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(scrollingNodeID));
if (!stateNode)
return false;
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -95,7 +95,7 @@
WEBCORE_EXPORT void frameViewRootLayerDidChange(FrameView&) override;
WEBCORE_EXPORT void frameViewEventTrackingRegionsChanged(FrameView&) override;
- WEBCORE_EXPORT bool requestScrollPositionUpdate(FrameView&, const IntPoint&) override;
+ WEBCORE_EXPORT bool requestScrollPositionUpdate(ScrollableArea&, const IntPoint&) override;
WEBCORE_EXPORT void applyScrollingTreeLayerPositions() override;
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -114,7 +114,7 @@
// These virtual functions are currently unique to the threaded scrolling architecture.
virtual void commitTreeStateIfNeeded() { }
- virtual bool requestScrollPositionUpdate(FrameView&, const IntPoint&) { return false; }
+ virtual bool requestScrollPositionUpdate(ScrollableArea&, const IntPoint&) { return false; }
virtual ScrollingEventResult handleWheelEvent(FrameView&, const PlatformWheelEvent&) { return ScrollingEventResult::DidNotHandleEvent; }
// Create an unparented node.
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -166,9 +166,6 @@
m_asyncFrameOrOverflowScrollingEnabled = scrollingStateTree->rootStateNode()->asyncFrameOrOverflowScrollingEnabled();
}
- bool scrollRequestIsProgammatic = rootNode ? rootNode->requestedScrollPositionRepresentsProgrammaticScroll() : false;
- SetForScope<bool> changeHandlingProgrammaticScroll(m_isHandlingProgrammaticScroll, scrollRequestIsProgammatic);
-
// unvisitedNodes starts with all nodes in the map; we remove nodes as we visit them. At the end, it's the unvisited nodes.
// We can't use orphanNodes for this, because orphanNodes won't contain descendants of removed nodes.
HashSet<ScrollingNodeID> unvisitedNodes;
@@ -404,11 +401,6 @@
m_swipeState.rubberBandsAtBottom = canRubberBandAtBottom;
}
-bool ScrollingTree::isHandlingProgrammaticScroll()
-{
- return m_isHandlingProgrammaticScroll;
-}
-
// Can be called from the main thread.
void ScrollingTree::setScrollPinningBehavior(ScrollPinningBehavior pinning)
{
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -118,7 +118,8 @@
// Can be called from any thread. Will update what edges allow rubber-banding.
WEBCORE_EXPORT void setCanRubberBandState(bool canRubberBandAtLeft, bool canRubberBandAtRight, bool canRubberBandAtTop, bool canRubberBandAtBottom);
- bool isHandlingProgrammaticScroll();
+ bool isHandlingProgrammaticScroll() const { return m_isHandlingProgrammaticScroll; }
+ void setIsHandlingProgrammaticScroll(bool isHandlingProgrammaticScroll) { m_isHandlingProgrammaticScroll = isHandlingProgrammaticScroll; }
void setScrollPinningBehavior(ScrollPinningBehavior);
WEBCORE_EXPORT ScrollPinningBehavior scrollPinningBehavior();
@@ -204,8 +205,8 @@
SwipeState m_swipeState;
unsigned m_fixedOrStickyNodeCount { 0 };
+ bool m_isHandlingProgrammaticScroll { false };
bool m_scrollingPerformanceLoggingEnabled { false };
- bool m_isHandlingProgrammaticScroll { false };
bool m_asyncFrameOrOverflowScrollingEnabled { false };
};
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -152,14 +152,16 @@
void ScrollingTreeScrollingNode::scrollBy(const FloatSize& delta, ScrollPositionClamp clamp)
{
- scrollTo(currentScrollPosition() + delta, clamp);
+ scrollTo(currentScrollPosition() + delta, ScrollType::User, clamp);
}
-void ScrollingTreeScrollingNode::scrollTo(const FloatPoint& position, ScrollPositionClamp clamp)
+void ScrollingTreeScrollingNode::scrollTo(const FloatPoint& position, ScrollType scrollType, ScrollPositionClamp clamp)
{
if (position == m_currentScrollPosition)
return;
+ scrollingTree().setIsHandlingProgrammaticScroll(scrollType == ScrollType::Programmatic);
+
m_currentScrollPosition = adjustedScrollPosition(position, clamp);
LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeScrollingNode " << scrollingNodeID() << " scrollTo " << position << " (delta from last committed position " << (m_lastCommittedScrollPosition - m_currentScrollPosition) << ")");
@@ -166,6 +168,8 @@
updateViewportForCurrentScrollPosition();
currentScrollPositionChanged();
+
+ scrollingTree().setIsHandlingProgrammaticScroll(false);
}
void ScrollingTreeScrollingNode::currentScrollPositionChanged()
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -57,7 +57,7 @@
FloatPoint lastCommittedScrollPosition() const { return m_lastCommittedScrollPosition; }
// These are imperative; they adjust the scrolling layers.
- void scrollTo(const FloatPoint&, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
+ void scrollTo(const FloatPoint&, ScrollType = ScrollType::User, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
void scrollBy(const FloatSize&, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
void wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport = { });
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (243925 => 243926)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2019-04-05 05:18:17 UTC (rev 243926)
@@ -137,8 +137,10 @@
const auto& scrollingStateNode = downcast<ScrollingStateScrollingNode>(stateNode);
// Update the scroll position after child nodes have been updated, because they need to have updated their constraints before any scrolling happens.
- if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
- scrollTo(scrollingStateNode.requestedScrollPosition());
+ if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition)) {
+ auto scrollType = scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll() ? ScrollType::Programmatic : ScrollType::User;
+ scrollTo(scrollingStateNode.requestedScrollPosition(), scrollType);
+ }
if (isRootNode()
&& (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (243925 => 243926)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -2346,8 +2346,15 @@
auto previousScrollType = currentScrollType();
setCurrentScrollType(scrollType);
- scrollToOffsetWithoutAnimation(newScrollOffset, clamping);
+ bool handled = false;
+#if ENABLE(ASYNC_SCROLLING)
+ if (ScrollingCoordinator* scrollingCoordinator = page().scrollingCoordinator())
+ handled = scrollingCoordinator->requestScrollPositionUpdate(*this, scrollPositionFromOffset(scrollOffset));
+#endif
+ if (!handled)
+ scrollToOffsetWithoutAnimation(newScrollOffset, clamping);
+
setCurrentScrollType(previousScrollType);
}
@@ -2804,6 +2811,14 @@
return 0;
}
+ScrollingNodeID RenderLayer::scrollingNodeID() const
+{
+ if (!isComposited())
+ return 0;
+
+ return backing()->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
+}
+
IntRect RenderLayer::visibleContentRectInternal(VisibleContentRectIncludesScrollbars scrollbarInclusion, VisibleContentRectBehavior) const
{
IntSize scrollbarSpace;
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (243925 => 243926)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -420,8 +420,8 @@
void scrollToYPosition(int y, ScrollType, ScrollClamping = ScrollClamping::Clamped);
// These are only used by marquee.
- void scrollToXOffset(int x, ScrollClamping clamping = ScrollClamping::Clamped) { scrollToOffset(ScrollOffset(x, scrollOffset().y()), ScrollType::Programmatic, clamping); }
- void scrollToYOffset(int y, ScrollClamping clamping = ScrollClamping::Clamped) { scrollToOffset(ScrollOffset(scrollOffset().x(), y), ScrollType::Programmatic, clamping); }
+ void scrollToXOffset(int x) { scrollToOffset(ScrollOffset(x, scrollOffset().y()), ScrollType::Programmatic, ScrollClamping::Unclamped); }
+ void scrollToYOffset(int y) { scrollToOffset(ScrollOffset(scrollOffset().x(), y), ScrollType::Programmatic, ScrollClamping::Unclamped); }
void setPostLayoutScrollPosition(Optional<ScrollPosition>);
void applyPostLayoutScrollPositionIfNeeded();
@@ -1050,6 +1050,7 @@
IntPoint convertFromContainingViewToScrollbar(const Scrollbar&, const IntPoint&) const override;
int scrollSize(ScrollbarOrientation) const override;
void setScrollOffset(const ScrollOffset&) override;
+ ScrollingNodeID scrollingNodeID() const override;
IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
IntSize overhangAmount() const override;
Modified: trunk/Source/WebCore/rendering/RenderMarquee.cpp (243925 => 243926)
--- trunk/Source/WebCore/rendering/RenderMarquee.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebCore/rendering/RenderMarquee.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -247,9 +247,9 @@
if (m_reset) {
m_reset = false;
if (isHorizontal())
- m_layer->scrollToXOffset(m_start, ScrollClamping::Unclamped);
+ m_layer->scrollToXOffset(m_start);
else
- m_layer->scrollToYOffset(m_start, ScrollClamping::Unclamped);
+ m_layer->scrollToYOffset(m_start);
return;
}
@@ -289,9 +289,9 @@
}
if (isHorizontal())
- m_layer->scrollToXOffset(newPos, ScrollClamping::Unclamped);
+ m_layer->scrollToXOffset(newPos);
else
- m_layer->scrollToYOffset(newPos, ScrollClamping::Unclamped);
+ m_layer->scrollToYOffset(newPos);
}
} // namespace WebCore
Modified: trunk/Source/WebKit/ChangeLog (243925 => 243926)
--- trunk/Source/WebKit/ChangeLog 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/ChangeLog 2019-04-05 05:18:17 UTC (rev 243926)
@@ -1,3 +1,39 @@
+2019-04-04 Simon Fraser <[email protected]>
+
+ [iOS WK2] REGRESSION (r242687): Programmatic scroll of overflow scroll results in bad rendering
+ https://bugs.webkit.org/show_bug.cgi?id=195584
+
+ Reviewed by Zalan Bujtas.
+
+ Push data to the scrolling tree about whether an overflow:scroll scroll was programmatic, by having
+ RenderLayer::scrollToOffset() call into AsyncScrollingCoordinator::requestScrollPositionUpdate(),
+ just as we do for frames.
+
+ AsyncScrollingCoordinator::requestScrollPositionUpdate() is generalized to take any ScrollableArea.
+
+ Fix an assumption in the ScrollingTree that we only care about programmatic scrolls on the root node.
+ ScrollingTree::commitTreeState() no longer sets isHandlingProgrammaticScroll; instead,
+ callers of ScrollingTreeScrollingNode::scrollTo() pass a ScrollType. Commit functions pass
+ ScrollType::Programmatic when handling RequestedScrollPosition changes as necessary.
+
+ Programmatic scrolls need to get to the scrolling tree in the UI process so that we update
+ the tree's notion of scroll position, and trigger actual UIScrollView scrolls (layers may have
+ already been put in the right locations, but the UI process needs to know that a scroll happened).
+ However, we need to prevent notifications from programmatic scrolls getting back to the
+ web process, because this causes jumpiness. This is done via an early return in
+ RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidScroll().
+
+ * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
+ (WebKit::RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidScroll):
+ * UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm:
+ (WebKit::ScrollingTreeFrameScrollingNodeRemoteIOS::commitStateAfterChildren): Subframe nodes have
+ a delegate, and that will take care of the requestedScrollPosition update.
+ * UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.h:
+ * UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
+ (WebKit::ScrollingTreeOverflowScrollingNodeIOS::commitStateAfterChildren):
+ * UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
+ (WebKit::ScrollingTreeScrollingNodeDelegateIOS::commitStateAfterChildren):
+
2019-04-04 Ryan Haddad <[email protected]>
Unreviewed, rolling out r243888.
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp (243925 => 243926)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2019-04-05 05:18:17 UTC (rev 243926)
@@ -216,6 +216,10 @@
#if PLATFORM(IOS_FAMILY)
m_webPageProxy.scrollingNodeScrollViewDidScroll();
#endif
+
+ if (m_scrollingTree->isHandlingProgrammaticScroll())
+ return;
+
m_webPageProxy.send(Messages::RemoteScrollingCoordinator::ScrollPositionChangedForNode(scrolledNodeID, newScrollPosition, scrollingLayerPositionAction == ScrollingLayerPositionAction::Sync));
}
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm (243925 => 243926)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm 2019-04-05 05:18:17 UTC (rev 243926)
@@ -82,12 +82,16 @@
const auto& scrollingStateNode = downcast<ScrollingStateFrameScrollingNode>(stateNode);
+ if (m_scrollingNodeDelegate) {
+ m_scrollingNodeDelegate->commitStateAfterChildren(scrollingStateNode);
+ return;
+ }
+
// Update the scroll position after child nodes have been updated, because they need to have updated their constraints before any scrolling happens.
- if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
- scrollTo(scrollingStateNode.requestedScrollPosition());
-
- if (m_scrollingNodeDelegate)
- m_scrollingNodeDelegate->commitStateAfterChildren(downcast<ScrollingStateScrollingNode>(stateNode));
+ if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition)) {
+ auto scrollType = scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll() ? ScrollType::Programmatic : ScrollType::User;
+ scrollTo(scrollingStateNode.requestedScrollPosition(), scrollType);
+ }
}
FloatPoint ScrollingTreeFrameScrollingNodeRemoteIOS::minimumScrollPosition() const
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.h (243925 => 243926)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.h 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.h 2019-04-05 05:18:17 UTC (rev 243926)
@@ -46,6 +46,7 @@
void repositionScrollingLayers() override;
+ // The delegate is non-null for subframes.
std::unique_ptr<ScrollingTreeScrollingNodeDelegateIOS> m_scrollingNodeDelegate;
};
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.mm (243925 => 243926)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.mm 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeOverflowScrollingNodeIOS.mm 2019-04-05 05:18:17 UTC (rev 243926)
@@ -63,7 +63,9 @@
void ScrollingTreeOverflowScrollingNodeIOS::commitStateAfterChildren(const ScrollingStateNode& stateNode)
{
ScrollingTreeOverflowScrollingNode::commitStateAfterChildren(stateNode);
- m_scrollingNodeDelegate->commitStateAfterChildren(downcast<ScrollingStateScrollingNode>(stateNode));
+
+ const auto& scrollingStateNode = downcast<ScrollingStateScrollingNode>(stateNode);
+ m_scrollingNodeDelegate->commitStateAfterChildren(scrollingStateNode);
}
void ScrollingTreeOverflowScrollingNodeIOS::repositionScrollingLayers()
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm (243925 => 243926)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm 2019-04-05 04:17:44 UTC (rev 243925)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm 2019-04-05 05:18:17 UTC (rev 243926)
@@ -225,6 +225,7 @@
void ScrollingTreeScrollingNodeDelegateIOS::commitStateAfterChildren(const ScrollingStateScrollingNode& scrollingStateNode)
{
SetForScope<bool> updatingChange(m_updatingFromStateNode, true);
+
if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)
|| scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize)
|| scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ReachableContentsSize)
@@ -287,6 +288,11 @@
END_BLOCK_OBJC_EXCEPTIONS
}
+
+ if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition)) {
+ auto scrollType = scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll() ? ScrollType::Programmatic : ScrollType::User;
+ scrollingNode().scrollTo(scrollingStateNode.requestedScrollPosition(), scrollType);
+ }
}
void ScrollingTreeScrollingNodeDelegateIOS::repositionScrollingLayers()
