Diff
Modified: trunk/Source/WebCore/ChangeLog (283715 => 283716)
--- trunk/Source/WebCore/ChangeLog 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/ChangeLog 2021-10-07 16:20:36 UTC (rev 283716)
@@ -1,3 +1,61 @@
+2021-10-06 Simon Fraser <[email protected]>
+
+ Clean up state maintenance around animated scrolls
+ https://bugs.webkit.org/show_bug.cgi?id=231347
+
+ Reviewed by Martin Robinson.
+
+ ScrollBehaviorStatus tracked whether an animated scroll is in progress. Rename it
+ for clarity, and remove the "non-native" term since this will never track native
+ vs. non-native animations.
+
+ Since ScrollBehaviorStatus is specifically about programmatic smooth scroll animations
+ triggered from script, it should be stored on ScrollableArea and is not relevant to the
+ scrolling tree. Remove it from the ScrollingTreeScrollingNodeDelegates.
+
+ The state becomes ScrollAnimationStatus::Animating immediately when JS triggers animations,
+ but the animation completion signal may come back in future from the scrolling thread.
+ Currently it comes via ScrollingEffectsController::scrollAnimationDidEnd().
+
+ We never need to call setScrollAnimationStatus(ScrollAnimationStatus::NotAnimating) other
+ than from scrollAnimationDidEnd(), because canceling an ongoing animation will always
+ call that.
+
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::scrollTo const):
+ * page/FrameView.cpp:
+ (WebCore::FrameView::scrollToPositionWithAnimation):
+ * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h:
+ * page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.h:
+ * platform/ScrollAnimator.cpp:
+ (WebCore::ScrollAnimator::scrollToPositionWithAnimation):
+ (WebCore::ScrollAnimator::retargetRunningAnimation):
+ (WebCore::ScrollAnimator::willStartAnimatedScroll):
+ (WebCore::ScrollAnimator::didStopAnimatedScroll):
+ (WebCore::ScrollAnimator::setScrollBehaviorStatus): Deleted.
+ (WebCore::ScrollAnimator::scrollBehaviorStatus const): Deleted.
+ * platform/ScrollAnimator.h:
+ * platform/ScrollTypes.h:
+ * platform/ScrollView.cpp:
+ (WebCore::ScrollView::setScrollPosition):
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::scrollToPositionWithAnimation):
+ (WebCore::ScrollableArea::resnapAfterLayout):
+ * platform/ScrollableArea.h:
+ (WebCore::ScrollableArea::scrollAnimationStatus):
+ (WebCore::ScrollableArea::setScrollAnimationStatus):
+ (WebCore::ScrollableArea::currentScrollBehaviorStatus): Deleted.
+ (WebCore::ScrollableArea::setScrollBehaviorStatus): Deleted.
+ * platform/ScrollingEffectsController.cpp:
+ (WebCore::ScrollingEffectsController::scrollAnimationDidEnd):
+ * platform/ScrollingEffectsController.h:
+ (WebCore::ScrollingEffectsControllerClient::willStartAnimatedScroll):
+ (WebCore::ScrollingEffectsControllerClient::didStopAnimatedScroll):
+ * rendering/RenderLayerScrollableArea.cpp:
+ (WebCore::RenderLayerScrollableArea::scrollToOffset):
+ (WebCore::RenderLayerScrollableArea::scrollTo):
+ (WebCore::RenderLayerScrollableArea::updateScrollPosition):
+
2021-10-07 Alan Bujtas <[email protected]>
[LFC][IFC] InlineDisplay::Box should be able to tell if it's the first or the last box within the associated Layout::Box
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (283715 => 283716)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -1712,7 +1712,7 @@
// This is an optimization for the common case of scrolling to (0, 0) when the scroller is already at the origin.
// If an animated scroll is in progress, this optimization is skipped to ensure that the animated scroll is really stopped.
- if (view->currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation && !scrollToOptions.left.value() && !scrollToOptions.top.value() && view->contentsScrollPosition() == IntPoint(0, 0))
+ if (view->scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating && !scrollToOptions.left.value() && !scrollToOptions.top.value() && view->contentsScrollPosition().isZero())
return;
document()->updateLayoutIgnorePendingStylesheets();
Modified: trunk/Source/WebCore/page/FrameView.cpp (283715 => 283716)
--- trunk/Source/WebCore/page/FrameView.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/page/FrameView.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -3768,9 +3768,10 @@
auto previousScrollType = currentScrollType();
setCurrentScrollType(scrollType);
- if (currentScrollBehaviorStatus() == ScrollBehaviorStatus::InNonNativeAnimation)
+ if (scrollAnimationStatus() == ScrollAnimationStatus::Animating)
scrollAnimator().cancelAnimations();
- if (position != this->scrollPosition())
+
+ if (position != scrollPosition())
ScrollableArea::scrollToPositionWithAnimation(position);
setCurrentScrollType(previousScrollType);
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h (283715 => 283716)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -81,8 +81,6 @@
RectEdges<bool> edgePinnedState() const final;
bool allowsHorizontalScrolling() const final;
bool allowsVerticalScrolling() const final;
- void setScrollBehaviorStatus(ScrollBehaviorStatus status) final { m_scrollBehaviorStatus = status; }
- ScrollBehaviorStatus scrollBehaviorStatus() const final { return m_scrollBehaviorStatus; }
bool shouldRubberBandOnSide(BoxSide) const final;
void immediateScrollBy(const FloatSize&, ScrollClamping = ScrollClamping::Clamped) final;
@@ -108,7 +106,6 @@
std::unique_ptr<RunLoop::Timer<ScrollingTreeScrollingNodeDelegateMac>> m_scrollControllerAnimationTimer;
- ScrollBehaviorStatus m_scrollBehaviorStatus { ScrollBehaviorStatus::NotInAnimation };
bool m_inMomentumPhase { false };
};
Modified: trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.h (283715 => 283716)
--- trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -63,9 +63,6 @@
bool allowsHorizontalScrolling() const final;
bool allowsVerticalScrolling() const final;
- void setScrollBehaviorStatus(ScrollBehaviorStatus status) final { m_scrollBehaviorStatus = status; }
- ScrollBehaviorStatus scrollBehaviorStatus() const final { return m_scrollBehaviorStatus; }
-
void immediateScrollBy(const FloatSize&, ScrollClamping = ScrollClamping::Clamped) final;
void adjustScrollPositionToBoundsIfNecessary() final;
@@ -80,7 +77,6 @@
ScrollingEffectsController m_scrollController;
std::unique_ptr<RunLoop::Timer<ScrollingTreeScrollingNodeDelegateNicosia>> m_animationTimer;
- ScrollBehaviorStatus m_scrollBehaviorStatus { ScrollBehaviorStatus::NotInAnimation };
bool m_scrollAnimatorEnabled { false };
};
Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -119,14 +119,12 @@
if (!positionChanged && !scrollableArea().scrollOriginChanged())
return false;
- m_scrollController.startAnimatedScrollToDestination(offsetFromPosition(m_currentPosition), offsetFromPosition(newPosition));
- setScrollBehaviorStatus(ScrollBehaviorStatus::InNonNativeAnimation);
- return true;
+ return m_scrollController.startAnimatedScrollToDestination(offsetFromPosition(m_currentPosition), offsetFromPosition(newPosition));
}
void ScrollAnimator::retargetRunningAnimation(const FloatPoint& newPosition)
{
- ASSERT(scrollableArea().currentScrollBehaviorStatus() == ScrollBehaviorStatus::InNonNativeAnimation);
+ ASSERT(scrollableArea().scrollAnimationStatus() == ScrollAnimationStatus::Animating);
m_scrollController.retargetAnimatedScroll(offsetFromPosition(newPosition));
}
@@ -292,14 +290,14 @@
return m_scrollableArea.allowsVerticalScrolling();
}
-void ScrollAnimator::setScrollBehaviorStatus(ScrollBehaviorStatus status)
+void ScrollAnimator::willStartAnimatedScroll()
{
- m_scrollableArea.setScrollBehaviorStatus(status);
+ m_scrollableArea.setScrollAnimationStatus(ScrollAnimationStatus::Animating);
}
-ScrollBehaviorStatus ScrollAnimator::scrollBehaviorStatus() const
+void ScrollAnimator::didStopAnimatedScroll()
{
- return m_scrollableArea.currentScrollBehaviorStatus();
+ m_scrollableArea.setScrollAnimationStatus(ScrollAnimationStatus::NotAnimating);
}
#if HAVE(RUBBER_BANDING)
Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollAnimator.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -145,8 +145,8 @@
bool allowsHorizontalScrolling() const final;
bool allowsVerticalScrolling() const final;
- void setScrollBehaviorStatus(ScrollBehaviorStatus) final;
- ScrollBehaviorStatus scrollBehaviorStatus() const final;
+ void willStartAnimatedScroll() final;
+ void didStopAnimatedScroll() final;
void immediateScrollBy(const FloatSize&, ScrollClamping = ScrollClamping::Clamped) final;
void adjustScrollPositionToBoundsIfNecessary() final;
Modified: trunk/Source/WebCore/platform/ScrollTypes.h (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollTypes.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollTypes.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -67,11 +67,9 @@
ScrollInlineDirectionForward
};
-// FIXME: Add another status InNativeAnimation to indicate native scrolling is in progress.
-// See: https://bugs.webkit.org/show_bug.cgi?id=204936
-enum class ScrollBehaviorStatus : uint8_t {
- NotInAnimation,
- InNonNativeAnimation,
+enum class ScrollAnimationStatus : uint8_t {
+ NotAnimating,
+ Animating,
};
enum class ScrollIsAnimated : uint8_t {
Modified: trunk/Source/WebCore/platform/ScrollView.cpp (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollView.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollView.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -523,17 +523,15 @@
return;
}
- if (currentScrollBehaviorStatus() == ScrollBehaviorStatus::InNonNativeAnimation)
+ if (scrollAnimationStatus() == ScrollAnimationStatus::Animating)
scrollAnimator().cancelAnimations();
ScrollPosition newScrollPosition = (!delegatesScrolling() && options.clamping == ScrollClamping::Clamped) ? adjustScrollPositionWithinRange(scrollPosition) : scrollPosition;
- if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation && newScrollPosition == this->scrollPosition())
+ if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && newScrollPosition == this->scrollPosition())
return;
if (!requestScrollPositionUpdate(newScrollPosition, currentScrollType(), options.clamping))
updateScrollbars(newScrollPosition);
-
- setScrollBehaviorStatus(ScrollBehaviorStatus::NotInAnimation);
}
bool ScrollView::scroll(ScrollDirection direction, ScrollGranularity granularity)
Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -143,7 +143,8 @@
void ScrollableArea::scrollToPositionWithAnimation(const FloatPoint& position, ScrollClamping)
{
LOG_WITH_STREAM(Scrolling, stream << "ScrollableArea " << this << " scrollToPositionWithAnimation " << position);
- scrollAnimator().scrollToPositionWithAnimation(position);
+ if (scrollAnimator().scrollToPositionWithAnimation(position))
+ setScrollAnimationStatus(ScrollAnimationStatus::Animating);
}
void ScrollableArea::scrollToOffsetWithoutAnimation(const FloatPoint& offset, ScrollClamping clamping)
@@ -539,7 +540,7 @@
if (correctedOffset != currentOffset) {
LOG_WITH_STREAM(ScrollSnap, stream << " adjusting offset from " << currentOffset << " to " << correctedOffset);
auto position = scrollPositionFromOffset(correctedOffset);
- if (currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation)
+ if (scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating)
scrollToOffsetWithoutAnimation(correctedOffset);
else
scrollAnimator->retargetRunningAnimation(position);
Modified: trunk/Source/WebCore/platform/ScrollableArea.h (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollableArea.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollableArea.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -68,9 +68,6 @@
virtual bool isListBox() const { return false; }
virtual bool isPDFPlugin() const { return false; }
- ScrollBehaviorStatus currentScrollBehaviorStatus() { return m_currentScrollBehaviorStatus; }
- void setScrollBehaviorStatus(ScrollBehaviorStatus status) { m_currentScrollBehaviorStatus = status; }
-
WEBCORE_EXPORT bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1);
WEBCORE_EXPORT void scrollToPositionWithAnimation(const FloatPoint&, ScrollClamping = ScrollClamping::Clamped);
WEBCORE_EXPORT void scrollToPositionWithoutAnimation(const FloatPoint&, ScrollClamping = ScrollClamping::Clamped);
@@ -252,6 +249,10 @@
ScrollType currentScrollType() const { return m_currentScrollType; }
void setCurrentScrollType(ScrollType scrollType) { m_currentScrollType = scrollType; }
+ // This reflects animated scrolls triggered by CSS OM View "smooth" scrolls.
+ ScrollAnimationStatus scrollAnimationStatus() { return m_scrollAnimationStatus; }
+ void setScrollAnimationStatus(ScrollAnimationStatus status) { m_scrollAnimationStatus = status; }
+
bool scrollShouldClearLatchedState() const { return m_scrollShouldClearLatchedState; }
void setScrollShouldClearLatchedState(bool shouldClear) { m_scrollShouldClearLatchedState = shouldClear; }
@@ -403,7 +404,7 @@
ScrollbarOverlayStyle m_scrollbarOverlayStyle { ScrollbarOverlayStyle::ScrollbarOverlayStyleDefault };
ScrollType m_currentScrollType { ScrollType::User };
- ScrollBehaviorStatus m_currentScrollBehaviorStatus { ScrollBehaviorStatus::NotInAnimation };
+ ScrollAnimationStatus m_scrollAnimationStatus { ScrollAnimationStatus::NotAnimating };
bool m_inLiveResize { false };
bool m_scrollOriginChanged { false };
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.cpp (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -456,7 +456,8 @@
stopScrollSnapAnimation();
}
- m_client.setScrollBehaviorStatus(ScrollBehaviorStatus::NotInAnimation);
+ // FIXME: Need to track state better and only call this when the running animation is for CSS smooth scrolling. Calling should be harmless, though.
+ m_client.didStopAnimatedScroll();
startOrStopAnimationCallbacks();
}
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.h (283715 => 283716)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-07 16:20:36 UTC (rev 283716)
@@ -82,10 +82,6 @@
virtual bool allowsHorizontalScrolling() const = 0;
virtual bool allowsVerticalScrolling() const = 0;
- // FIXME: Maybe ScrollBehaviorStatus should be stored on ScrollingEffectsController.
- virtual void setScrollBehaviorStatus(ScrollBehaviorStatus) = 0;
- virtual ScrollBehaviorStatus scrollBehaviorStatus() const = 0;
-
virtual void immediateScrollBy(const FloatSize&, ScrollClamping = ScrollClamping::Clamped) = 0;
// If the current scroll position is within the overhang area, this function will cause
@@ -113,8 +109,13 @@
virtual void removeWheelEventTestCompletionDeferralForReason(WheelEventTestMonitor::ScrollableAreaIdentifier, WheelEventTestMonitor::DeferReason) const { /* Do nothing */ }
virtual FloatPoint scrollOffset() const = 0;
+
+ virtual void willStartAnimatedScroll() { }
+ virtual void didStopAnimatedScroll() { }
+
virtual void willStartScrollSnapAnimation() { }
virtual void didStopScrollSnapAnimation() { }
+
virtual float pageScaleFactor() const = 0;
virtual ScrollExtents scrollExtents() const = 0;
virtual bool scrollAnimationEnabled() const { return true; }
Modified: trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp (283715 => 283716)
--- trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2021-10-07 15:40:41 UTC (rev 283715)
+++ trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2021-10-07 16:20:36 UTC (rev 283716)
@@ -256,7 +256,7 @@
ScrollOffset RenderLayerScrollableArea::scrollToOffset(const ScrollOffset& scrollOffset, const ScrollPositionChangeOptions& options)
{
- if (currentScrollBehaviorStatus() == ScrollBehaviorStatus::InNonNativeAnimation)
+ if (scrollAnimationStatus() == ScrollAnimationStatus::Animating)
scrollAnimator().cancelAnimations();
ScrollOffset clampedScrollOffset = options.clamping == ScrollClamping::Clamped ? clampScrollOffset(scrollOffset) : scrollOffset;
@@ -270,11 +270,8 @@
auto snappedPosition = scrollPositionFromOffset(snappedOffset);
if (options.animated == ScrollIsAnimated::Yes)
ScrollableArea::scrollToPositionWithAnimation(snappedPosition);
- else {
- if (!requestScrollPositionUpdate(snappedPosition, options.type, options.clamping))
- scrollToPositionWithoutAnimation(snappedPosition, options.clamping);
- setScrollBehaviorStatus(ScrollBehaviorStatus::NotInAnimation);
- }
+ else if (!requestScrollPositionUpdate(snappedPosition, options.type, options.clamping))
+ scrollToPositionWithoutAnimation(snappedPosition, options.clamping);
setCurrentScrollType(previousScrollType);
return snappedOffset;
@@ -312,7 +309,7 @@
#endif
}
- if (m_scrollPosition == newPosition && currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation) {
+ if (m_scrollPosition == newPosition && scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating) {
// FIXME: Nothing guarantees we get a scrollTo() with an unchanged position at the end of a user gesture.
// The ScrollingCoordinator probably needs to message the main thread when a gesture ends.
if (requiresScrollPositionReconciliation()) {
@@ -1759,7 +1756,7 @@
ASSERT(box);
ScrollOffset clampedScrollOffset = clampScrollOffset(scrollOffset() + toIntSize(roundedIntRect(revealRect).location()));
- if (clampedScrollOffset != scrollOffset() || currentScrollBehaviorStatus() != ScrollBehaviorStatus::NotInAnimation) {
+ if (clampedScrollOffset != scrollOffset() || scrollAnimationStatus() != ScrollAnimationStatus::NotAnimating) {
ScrollOffset oldScrollOffset = scrollOffset();
ScrollOffset realScrollOffset = scrollToOffset(clampedScrollOffset, options);
