Diff
Modified: trunk/Source/WebCore/ChangeLog (243854 => 243855)
--- trunk/Source/WebCore/ChangeLog 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/ChangeLog 2019-04-04 03:57:22 UTC (rev 243855)
@@ -1,3 +1,34 @@
+2019-04-03 Simon Fraser <[email protected]>
+
+ Simplify some "programmaticScroll" code paths
+ https://bugs.webkit.org/show_bug.cgi?id=196589
+
+ Reviewed by Zalan Bujtas.
+
+ AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll() just returned early if programmaticScroll
+ was true, so instead, just never call it. This means we can remove the "programmaticScroll" argument from
+ scheduleUpdateScrollPositionAfterAsyncScroll(). Also change some callers to use the ScrollType enum
+ instead of a bool.
+
+ Now, ThreadedScrollingTree::scrollingTreeNodeDidScroll() just returns early. Programmatic scrolls
+ update state on the main thread before updating the scrolling tree, so this makes sense.
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate):
+ (WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll):
+ (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired):
+ (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll):
+ (WebCore::AsyncScrollingCoordinator::reconcileScrollingState):
+ * page/scrolling/AsyncScrollingCoordinator.h:
+ (WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::ScheduledScrollUpdate):
+ (WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::matchesUpdateType const):
+ * page/scrolling/ScrollingCoordinator.cpp:
+ (WebCore::operator<<):
+ * page/scrolling/ScrollingCoordinator.h:
+ (WebCore::ScrollingCoordinator::reconcileScrollingState):
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):
+
2019-04-03 Youenn Fablet <[email protected]>
Add logging and ASSERTs to investigate issue with VPModuleInitialize
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (243854 => 243855)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-04-04 03:57:22 UTC (rev 243855)
@@ -212,13 +212,14 @@
if (!coordinatesScrollingForFrameView(frameView))
return false;
- bool isProgrammaticScroll = frameView.inProgrammaticScroll();
- if (isProgrammaticScroll || frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
- updateScrollPositionAfterAsyncScroll(frameView.scrollingNodeID(), scrollPosition, WTF::nullopt, isProgrammaticScroll, ScrollingLayerPositionAction::Set);
+ bool inPageCache = frameView.frame().document()->pageCacheState() != Document::NotInPageCache;
+ bool inProgrammaticScroll = frameView.inProgrammaticScroll();
+ if (inProgrammaticScroll || inPageCache)
+ updateScrollPositionAfterAsyncScroll(frameView.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.
- if (frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
+ if (inPageCache)
return true;
auto* stateNode = downcast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
@@ -225,7 +226,7 @@
if (!stateNode)
return false;
- stateNode->setRequestedScrollPosition(scrollPosition, isProgrammaticScroll);
+ stateNode->setRequestedScrollPosition(scrollPosition, inProgrammaticScroll);
return true;
}
@@ -234,14 +235,10 @@
m_scrollingTree->applyLayerPositions();
}
-void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction scrollingLayerPositionAction)
+void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, ScrollingLayerPositionAction scrollingLayerPositionAction)
{
- ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, layoutViewportOrigin, programmaticScroll, scrollingLayerPositionAction);
+ ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction);
- // For programmatic scrolls, requestScrollPositionUpdate() has already called updateScrollPositionAfterAsyncScroll().
- if (programmaticScroll)
- return;
-
if (m_updateNodeScrollPositionTimer.isActive()) {
if (m_scheduledScrollUpdate.matchesUpdateType(scrollUpdate)) {
m_scheduledScrollUpdate.scrollPosition = scrollPosition;
@@ -251,8 +248,8 @@
// If the parameters don't match what was previously scheduled, dispatch immediately.
m_updateNodeScrollPositionTimer.stop();
- updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, m_scheduledScrollUpdate.isProgrammaticScroll, m_scheduledScrollUpdate.updateLayerPositionAction);
- updateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, programmaticScroll, scrollingLayerPositionAction);
+ updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
+ updateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, ScrollType::User, scrollingLayerPositionAction);
return;
}
@@ -262,7 +259,7 @@
void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired()
{
- updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, m_scheduledScrollUpdate.isProgrammaticScroll, m_scheduledScrollUpdate.updateLayerPositionAction);
+ updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
}
FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
@@ -297,7 +294,7 @@
return nullptr;
}
-void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNodeID scrollingNodeID, const FloatPoint& scrollPosition, Optional<FloatPoint> layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction scrollingLayerPositionAction)
+void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNodeID scrollingNodeID, const FloatPoint& scrollPosition, Optional<FloatPoint> layoutViewportOrigin, ScrollType scrollType, ScrollingLayerPositionAction scrollingLayerPositionAction)
{
ASSERT(isMainThread());
@@ -313,7 +310,7 @@
auto& frameView = *frameViewPtr;
if (scrollingNodeID == frameView.scrollingNodeID()) {
- reconcileScrollingState(frameView, scrollPosition, layoutViewportOrigin, programmaticScroll, ViewportRectStability::Stable, scrollingLayerPositionAction);
+ reconcileScrollingState(frameView, scrollPosition, layoutViewportOrigin, scrollType, ViewportRectStability::Stable, scrollingLayerPositionAction);
#if PLATFORM(COCOA)
if (m_page->expectsWheelEventTriggers()) {
@@ -344,12 +341,12 @@
}
}
-void AsyncScrollingCoordinator::reconcileScrollingState(FrameView& frameView, const FloatPoint& scrollPosition, const LayoutViewportOriginOrOverrideRect& layoutViewportOriginOrOverrideRect, bool programmaticScroll, ViewportRectStability viewportRectStability, ScrollingLayerPositionAction scrollingLayerPositionAction)
+void AsyncScrollingCoordinator::reconcileScrollingState(FrameView& frameView, const FloatPoint& scrollPosition, const LayoutViewportOriginOrOverrideRect& layoutViewportOriginOrOverrideRect, ScrollType scrollType, ViewportRectStability viewportRectStability, ScrollingLayerPositionAction scrollingLayerPositionAction)
{
bool oldProgrammaticScroll = frameView.inProgrammaticScroll();
- frameView.setInProgrammaticScroll(programmaticScroll);
+ frameView.setInProgrammaticScroll(scrollType == ScrollType::Programmatic);
- LOG_WITH_STREAM(Scrolling, stream << getCurrentProcessID() << " AsyncScrollingCoordinator " << this << " reconcileScrollingState scrollPosition " << scrollPosition << " programmaticScroll " << programmaticScroll << " stability " << viewportRectStability << " " << scrollingLayerPositionAction);
+ LOG_WITH_STREAM(Scrolling, stream << getCurrentProcessID() << " AsyncScrollingCoordinator " << this << " reconcileScrollingState scrollPosition " << scrollPosition << " type " << scrollType << " stability " << viewportRectStability << " " << scrollingLayerPositionAction);
Optional<FloatRect> layoutViewportRect;
@@ -372,7 +369,7 @@
frameView.setConstrainsScrollingToContentEdge(true);
frameView.setInProgrammaticScroll(oldProgrammaticScroll);
- if (!programmaticScroll && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set) {
+ if (scrollType == ScrollType::User && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set) {
auto scrollingNodeID = frameView.scrollingNodeID();
if (viewportRectStability == ViewportRectStability::Stable)
reconcileViewportConstrainedLayerPositions(scrollingNodeID, frameView.rectForFixedPositionLayout(), scrollingLayerPositionAction);
@@ -403,7 +400,7 @@
FloatPoint positionForFooterLayer = FloatPoint(scrollPositionForFixed.x(),
FrameView::yPositionForFooterLayer(scrollPosition, topContentInset, frameView.totalContentsSize().height(), frameView.footerHeight()));
- if (programmaticScroll || scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) {
+ if (scrollType == ScrollType::Programmatic || scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) {
reconcileScrollPosition(frameView, ScrollingLayerPositionAction::Set);
if (counterScrollingLayer)
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h (243854 => 243855)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-04-04 03:57:22 UTC (rev 243855)
@@ -51,7 +51,7 @@
void scrollingStateTreePropertiesChanged();
- WEBCORE_EXPORT void scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, const Optional<FloatPoint>& layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction);
+ WEBCORE_EXPORT void scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, const Optional<FloatPoint>& layoutViewportOrigin, ScrollingLayerPositionAction);
#if PLATFORM(COCOA)
WEBCORE_EXPORT void setActiveScrollSnapIndices(ScrollingNodeID, unsigned horizontalIndex, unsigned verticalIndex);
@@ -77,7 +77,7 @@
RefPtr<ScrollingTree> releaseScrollingTree() { return WTFMove(m_scrollingTree); }
- void updateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, Optional<FloatPoint> layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction);
+ void updateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, Optional<FloatPoint> layoutViewportOrigin, ScrollType, ScrollingLayerPositionAction);
WEBCORE_EXPORT String scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const override;
WEBCORE_EXPORT void willCommitTree() override;
@@ -118,7 +118,7 @@
WEBCORE_EXPORT void setPositionedNodeGeometry(ScrollingNodeID, const LayoutConstraints&) override;
WEBCORE_EXPORT void setRelatedOverflowScrollingNodes(ScrollingNodeID, Vector<ScrollingNodeID>&&) override;
- WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool programmaticScroll, ViewportRectStability, ScrollingLayerPositionAction) override;
+ WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, ScrollType, ViewportRectStability, ScrollingLayerPositionAction) override;
void reconcileScrollPosition(FrameView&, ScrollingLayerPositionAction);
bool isRubberBandInProgress() const override;
@@ -147,11 +147,10 @@
struct ScheduledScrollUpdate {
ScheduledScrollUpdate() = default;
- ScheduledScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, bool isProgrammatic, ScrollingLayerPositionAction udpateAction)
+ ScheduledScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, ScrollingLayerPositionAction udpateAction)
: nodeID(scrollingNodeID)
, scrollPosition(point)
, layoutViewportOrigin(viewportOrigin)
- , isProgrammaticScroll(isProgrammatic)
, updateLayerPositionAction(udpateAction)
{ }
@@ -158,14 +157,11 @@
ScrollingNodeID nodeID { 0 };
FloatPoint scrollPosition;
Optional<FloatPoint> layoutViewportOrigin;
- bool isProgrammaticScroll { false };
ScrollingLayerPositionAction updateLayerPositionAction { ScrollingLayerPositionAction::Sync };
bool matchesUpdateType(const ScheduledScrollUpdate& other) const
{
- return nodeID == other.nodeID
- && isProgrammaticScroll == other.isProgrammaticScroll
- && updateLayerPositionAction == other.updateLayerPositionAction;
+ return nodeID == other.nodeID && updateLayerPositionAction == other.updateLayerPositionAction;
}
};
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp (243854 => 243855)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp 2019-04-04 03:57:22 UTC (rev 243855)
@@ -509,4 +509,13 @@
return ts;
}
+TextStream& operator<<(TextStream& ts, ScrollType scrollType)
+{
+ switch (scrollType) {
+ case ScrollType::User: ts << "user"; break;
+ case ScrollType::Programmatic: ts << "programmatic"; break;
+ }
+ return ts;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (243854 => 243855)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-04-04 03:57:22 UTC (rev 243855)
@@ -87,7 +87,7 @@
virtual void frameViewLayoutUpdated(FrameView&) { }
using LayoutViewportOriginOrOverrideRect = WTF::Variant<Optional<FloatPoint>, Optional<FloatRect>>;
- virtual void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool /* programmaticScroll */, ViewportRectStability, ScrollingLayerPositionAction) { }
+ virtual void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, ScrollType, ViewportRectStability, ScrollingLayerPositionAction) { }
// Should be called whenever the slow repaint objects counter changes between zero and one.
void frameViewHasSlowRepaintObjectsDidChange(FrameView&);
@@ -214,6 +214,7 @@
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollingNodeType);
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollingLayerPositionAction);
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ViewportRectStability);
+WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollType);
} // namespace WebCore
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (243854 => 243855)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2019-04-04 03:57:22 UTC (rev 243855)
@@ -103,12 +103,15 @@
if (node.isRootNode())
setMainFrameScrollPosition(scrollPosition);
+ if (isHandlingProgrammaticScroll())
+ return;
+
Optional<FloatPoint> layoutViewportOrigin;
if (is<ScrollingTreeFrameScrollingNode>(node))
layoutViewportOrigin = downcast<ScrollingTreeFrameScrollingNode>(node).layoutViewport().location();
- RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, localIsHandlingProgrammaticScroll = isHandlingProgrammaticScroll(), scrollingLayerPositionAction] {
- scrollingCoordinator->scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, localIsHandlingProgrammaticScroll, scrollingLayerPositionAction);
+ RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction] {
+ scrollingCoordinator->scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction);
});
}
Modified: trunk/Source/WebKit/ChangeLog (243854 => 243855)
--- trunk/Source/WebKit/ChangeLog 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebKit/ChangeLog 2019-04-04 03:57:22 UTC (rev 243855)
@@ -1,3 +1,15 @@
+2019-04-03 Simon Fraser <[email protected]>
+
+ Simplify some "programmaticScroll" code paths
+ https://bugs.webkit.org/show_bug.cgi?id=196589
+
+ Reviewed by Zalan Bujtas.
+
+ * WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm: Remove the parameter.
+ (WebKit::RemoteScrollingCoordinator::scrollPositionChangedForNode): Use the enum type.
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::updateVisibleContentRects):
+
2019-04-03 Chris Dumez <[email protected]>
The page's focusedFrame / frameSetLargestFrame do not get cleared on process swap or crash
Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm (243854 => 243855)
--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm 2019-04-04 03:57:22 UTC (rev 243855)
@@ -96,7 +96,7 @@
// Notification from the UI process that we scrolled.
void RemoteScrollingCoordinator::scrollPositionChangedForNode(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, bool syncLayerPosition)
{
- scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, WTF::nullopt, false /* FIXME */, syncLayerPosition ? ScrollingLayerPositionAction::Sync : ScrollingLayerPositionAction::Set);
+ scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, WTF::nullopt, syncLayerPosition ? ScrollingLayerPositionAction::Sync : ScrollingLayerPositionAction::Set);
}
void RemoteScrollingCoordinator::currentSnapPointIndicesChangedForNode(ScrollingNodeID nodeID, unsigned horizontal, unsigned vertical)
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (243854 => 243855)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-04-04 03:57:11 UTC (rev 243854)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-04-04 03:57:22 UTC (rev 243855)
@@ -3247,7 +3247,7 @@
viewportStability = ViewportRectStability::Unstable;
layerAction = ScrollingLayerPositionAction::SetApproximate;
}
- scrollingCoordinator->reconcileScrollingState(frameView, scrollPosition, visibleContentRectUpdateInfo.customFixedPositionRect(), false, viewportStability, layerAction);
+ scrollingCoordinator->reconcileScrollingState(frameView, scrollPosition, visibleContentRectUpdateInfo.customFixedPositionRect(), ScrollType::User, viewportStability, layerAction);
}
}
