Diff
Modified: trunk/Source/WebCore/ChangeLog (251172 => 251173)
--- trunk/Source/WebCore/ChangeLog 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/ChangeLog 2019-10-16 01:27:33 UTC (rev 251173)
@@ -1,5 +1,68 @@
2019-10-15 Simon Fraser <simon.fra...@apple.com>
+ ScrollingTreeScrollingNodeDelegateMac::stretchAmount() should not have side effects
+ https://bugs.webkit.org/show_bug.cgi?id=203009
+
+ Reviewed by Dean Jackson.
+
+ Calling ScrollingTreeScrollingNodeDelegateMac::stretchAmount() had the side effect of calling
+ setMainFrameIsRubberBanding() on the scrolling tree.
+
+ Remove this badness and replace it by modifying updateMainFramePinState() (which is called every time
+ the scroll position changes) to also compute if we're rubber-banding.
+
+ Also make a bunch of methods on ScrollControllerClient const, which makes it clear that
+ they don't have side effects.
+
+ * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
+ * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::commitStateAfterChildren):
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::currentScrollPositionChanged):
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::updateMainFramePinAndRubberbandState):
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::updateMainFramePinState): Deleted.
+ * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h:
+ * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::isAlreadyPinnedInDirectionOfGesture const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::stretchAmount const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::canScrollHorizontally const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::canScrollVertically const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::isAlreadyPinnedInDirectionOfGesture): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::stretchAmount): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::canScrollHorizontally): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::canScrollVertically): Deleted.
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection): Deleted.
+ * platform/cocoa/ScrollController.h:
+ * platform/mac/ScrollAnimatorMac.h:
+ * platform/mac/ScrollAnimatorMac.mm:
+ (WebCore::ScrollAnimatorMac::shouldForwardWheelEventsToParent const):
+ (WebCore::ScrollAnimatorMac::pinnedInDirection const):
+ (WebCore::ScrollAnimatorMac::isAlreadyPinnedInDirectionOfGesture const):
+ (WebCore::ScrollAnimatorMac::allowsVerticalStretching const):
+ (WebCore::ScrollAnimatorMac::allowsHorizontalStretching const):
+ (WebCore::ScrollAnimatorMac::stretchAmount const):
+ (WebCore::ScrollAnimatorMac::canScrollHorizontally const):
+ (WebCore::ScrollAnimatorMac::canScrollVertically const):
+ (WebCore::ScrollAnimatorMac::shouldRubberBandInDirection const):
+ (WebCore::ScrollAnimatorMac::shouldForwardWheelEventsToParent): Deleted.
+ (WebCore::ScrollAnimatorMac::pinnedInDirection): Deleted.
+ (WebCore::ScrollAnimatorMac::isAlreadyPinnedInDirectionOfGesture): Deleted.
+ (WebCore::ScrollAnimatorMac::allowsVerticalStretching): Deleted.
+ (WebCore::ScrollAnimatorMac::allowsHorizontalStretching): Deleted.
+ (WebCore::ScrollAnimatorMac::stretchAmount): Deleted.
+ (WebCore::ScrollAnimatorMac::canScrollHorizontally): Deleted.
+ (WebCore::ScrollAnimatorMac::canScrollVertically): Deleted.
+ (WebCore::ScrollAnimatorMac::shouldRubberBandInDirection): Deleted.
+ * platform/mock/ScrollAnimatorMock.h:
+
+2019-10-15 Simon Fraser <simon.fra...@apple.com>
+
WheelEventTestMonitor doesn't need to be threadsafe
https://bugs.webkit.org/show_bug.cgi?id=203012
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h (251172 => 251173)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h 2019-10-16 01:27:33 UTC (rev 251173)
@@ -56,7 +56,7 @@
FloatPoint minimumScrollPosition() const override;
FloatPoint maximumScrollPosition() const override;
- void updateMainFramePinState();
+ void updateMainFramePinAndRubberbandState();
unsigned exposedUnfilledArea() const;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (251172 => 251173)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2019-10-16 01:27:33 UTC (rev 251173)
@@ -145,7 +145,7 @@
&& (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)
|| scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize)
|| scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollableAreaSize)))
- updateMainFramePinState();
+ updateMainFramePinAndRubberbandState();
}
ScrollingEventResult ScrollingTreeFrameScrollingNodeMac::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
@@ -180,7 +180,7 @@
LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFrameScrollingNodeMac::currentScrollPositionChanged to " << currentScrollPosition() << " min: " << minimumScrollPosition() << " max: " << maximumScrollPosition() << " sync: " << shouldUpdateScrollLayerPositionSynchronously());
if (isRootNode())
- updateMainFramePinState();
+ updateMainFramePinAndRubberbandState();
if (shouldUpdateScrollLayerPositionSynchronously())
scrollingTree().scrollingTreeNodeDidScroll(*this, ScrollingLayerPositionAction::Set);
@@ -256,7 +256,7 @@
return position;
}
-void ScrollingTreeFrameScrollingNodeMac::updateMainFramePinState()
+void ScrollingTreeFrameScrollingNodeMac::updateMainFramePinAndRubberbandState()
{
ASSERT(isRootNode());
@@ -267,6 +267,13 @@
bool pinnedToTheBottom = scrollPosition.y() >= maximumScrollPosition().y();
scrollingTree().setMainFramePinState(pinnedToTheLeft, pinnedToTheRight, pinnedToTheTop, pinnedToTheBottom);
+
+ bool rubberbanding = scrollPosition.x() < minimumScrollPosition().x()
+ || scrollPosition.x() > maximumScrollPosition().x()
+ || scrollPosition.y() < minimumScrollPosition().y()
+ || scrollPosition.y() > maximumScrollPosition().y();
+
+ scrollingTree().setMainFrameIsRubberBanding(rubberbanding);
}
unsigned ScrollingTreeFrameScrollingNodeMac::exposedUnfilledArea() const
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h (251172 => 251173)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h 2019-10-16 01:27:33 UTC (rev 251173)
@@ -64,16 +64,16 @@
void removeWheelEventTestCompletionDeferralForReason(WheelEventTestMonitor::ScrollableAreaIdentifier, WheelEventTestMonitor::DeferReason) const override;
private:
- bool isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent&, ScrollEventAxis);
+ bool isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent&, ScrollEventAxis) const;
// ScrollControllerClient.
- bool allowsHorizontalStretching(const PlatformWheelEvent&) override;
- bool allowsVerticalStretching(const PlatformWheelEvent&) override;
- IntSize stretchAmount() override;
- bool pinnedInDirection(const FloatSize&) override;
- bool canScrollHorizontally() override;
- bool canScrollVertically() override;
- bool shouldRubberBandInDirection(ScrollDirection) override;
+ bool allowsHorizontalStretching(const PlatformWheelEvent&) const override;
+ bool allowsVerticalStretching(const PlatformWheelEvent&) const override;
+ IntSize stretchAmount() const override;
+ bool pinnedInDirection(const FloatSize&) const override;
+ bool canScrollHorizontally() const override;
+ bool canScrollVertically() const override;
+ bool shouldRubberBandInDirection(ScrollDirection) const override;
void immediateScrollBy(const FloatSize&) override;
void immediateScrollByWithoutContentEdgeConstraints(const FloatSize&) override;
void stopSnapRubberbandTimer() override;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm (251172 => 251173)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm 2019-10-16 01:27:33 UTC (rev 251173)
@@ -113,7 +113,7 @@
return wheelEvent.phase() == PlatformWheelEventPhaseMayBegin || wheelEvent.phase() == PlatformWheelEventPhaseBegan;
}
-bool ScrollingTreeScrollingNodeDelegateMac::isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent& wheelEvent, ScrollEventAxis axis)
+bool ScrollingTreeScrollingNodeDelegateMac::isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent& wheelEvent, ScrollEventAxis axis) const
{
auto scrollPosition = currentScrollPosition();
switch (axis) {
@@ -127,7 +127,7 @@
return false;
}
-bool ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching(const PlatformWheelEvent& wheelEvent)
+bool ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching(const PlatformWheelEvent& wheelEvent) const
{
switch (horizontalScrollElasticity()) {
case ScrollElasticityAutomatic: {
@@ -145,7 +145,7 @@
return false;
}
-bool ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching(const PlatformWheelEvent& wheelEvent)
+bool ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching(const PlatformWheelEvent& wheelEvent) const
{
switch (verticalScrollElasticity()) {
case ScrollElasticityAutomatic: {
@@ -163,7 +163,7 @@
return false;
}
-IntSize ScrollingTreeScrollingNodeDelegateMac::stretchAmount()
+IntSize ScrollingTreeScrollingNodeDelegateMac::stretchAmount() const
{
IntSize stretch;
auto scrollPosition = currentScrollPosition();
@@ -178,18 +178,10 @@
else if (scrollPosition.x() > maximumScrollPosition().x())
stretch.setWidth(scrollPosition.x() - maximumScrollPosition().x());
- // FIXME: calling this function should not have these side-effects.
- if (scrollingNode().isRootNode()) {
- if (stretch.isZero())
- scrollingTree().setMainFrameIsRubberBanding(false);
- else
- scrollingTree().setMainFrameIsRubberBanding(true);
- }
-
return stretch;
}
-bool ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection(const FloatSize& delta)
+bool ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection(const FloatSize& delta) const
{
FloatSize limitDelta;
auto scrollPosition = currentScrollPosition();
@@ -218,17 +210,17 @@
return false;
}
-bool ScrollingTreeScrollingNodeDelegateMac::canScrollHorizontally()
+bool ScrollingTreeScrollingNodeDelegateMac::canScrollHorizontally() const
{
return hasEnabledHorizontalScrollbar();
}
-bool ScrollingTreeScrollingNodeDelegateMac::canScrollVertically()
+bool ScrollingTreeScrollingNodeDelegateMac::canScrollVertically() const
{
return hasEnabledVerticalScrollbar();
}
-bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection)
+bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection) const
{
return true;
}
Modified: trunk/Source/WebCore/platform/cocoa/ScrollController.h (251172 => 251173)
--- trunk/Source/WebCore/platform/cocoa/ScrollController.h 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/platform/cocoa/ScrollController.h 2019-10-16 01:27:33 UTC (rev 251173)
@@ -23,8 +23,7 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef ScrollController_h
-#define ScrollController_h
+#pragma once
#if ENABLE(RUBBER_BANDING) || ENABLE(CSS_SCROLL_SNAP)
@@ -53,13 +52,13 @@
public:
#if ENABLE(RUBBER_BANDING)
- virtual bool allowsHorizontalStretching(const PlatformWheelEvent&) = 0;
- virtual bool allowsVerticalStretching(const PlatformWheelEvent&) = 0;
- virtual IntSize stretchAmount() = 0;
- virtual bool pinnedInDirection(const FloatSize&) = 0;
- virtual bool canScrollHorizontally() = 0;
- virtual bool canScrollVertically() = 0;
- virtual bool shouldRubberBandInDirection(ScrollDirection) = 0;
+ virtual bool allowsHorizontalStretching(const PlatformWheelEvent&) const = 0;
+ virtual bool allowsVerticalStretching(const PlatformWheelEvent&) const = 0;
+ virtual IntSize stretchAmount() const = 0;
+ virtual bool pinnedInDirection(const FloatSize&) const = 0;
+ virtual bool canScrollHorizontally() const = 0;
+ virtual bool canScrollVertically() const = 0;
+ virtual bool shouldRubberBandInDirection(ScrollDirection) const = 0;
// FIXME: use ScrollClamping to collapse these to one.
virtual void immediateScrollBy(const FloatSize&) = 0;
@@ -212,5 +211,3 @@
} // namespace WebCore
#endif // ENABLE(RUBBER_BANDING)
-
-#endif // ScrollController_h
Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h (251172 => 251173)
--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2019-10-16 01:27:33 UTC (rev 251173)
@@ -84,7 +84,7 @@
void scrollToOffsetWithoutAnimation(const FloatPoint&, ScrollClamping) override;
#if ENABLE(RUBBER_BANDING)
- bool shouldForwardWheelEventsToParent(const PlatformWheelEvent&);
+ bool shouldForwardWheelEventsToParent(const PlatformWheelEvent&) const;
bool handleWheelEvent(const PlatformWheelEvent&) override;
#endif
@@ -140,18 +140,18 @@
#if ENABLE(RUBBER_BANDING)
/// ScrollControllerClient member functions.
- IntSize stretchAmount() override;
- bool allowsHorizontalStretching(const PlatformWheelEvent&) override;
- bool allowsVerticalStretching(const PlatformWheelEvent&) override;
- bool pinnedInDirection(const FloatSize&) override;
- bool canScrollHorizontally() override;
- bool canScrollVertically() override;
- bool shouldRubberBandInDirection(ScrollDirection) override;
+ IntSize stretchAmount() const override;
+ bool allowsHorizontalStretching(const PlatformWheelEvent&) const override;
+ bool allowsVerticalStretching(const PlatformWheelEvent&) const override;
+ bool pinnedInDirection(const FloatSize&) const override;
+ bool canScrollHorizontally() const override;
+ bool canScrollVertically() const override;
+ bool shouldRubberBandInDirection(ScrollDirection) const override;
void immediateScrollByWithoutContentEdgeConstraints(const FloatSize&) override;
void immediateScrollBy(const FloatSize&) override;
void adjustScrollPositionToBoundsIfNecessary() override;
- bool isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent&, ScrollEventAxis);
+ bool isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent&, ScrollEventAxis) const;
#endif
bool m_haveScrolledSincePageLoad;
Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (251172 => 251173)
--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2019-10-16 01:27:33 UTC (rev 251173)
@@ -1179,7 +1179,7 @@
#if ENABLE(RUBBER_BANDING)
-bool ScrollAnimatorMac::shouldForwardWheelEventsToParent(const PlatformWheelEvent& wheelEvent)
+bool ScrollAnimatorMac::shouldForwardWheelEventsToParent(const PlatformWheelEvent& wheelEvent) const
{
if (std::abs(wheelEvent.deltaY()) >= std::abs(wheelEvent.deltaX()))
return !allowsVerticalStretching(wheelEvent);
@@ -1213,7 +1213,7 @@
return didHandleEvent;
}
-bool ScrollAnimatorMac::pinnedInDirection(const FloatSize& direction)
+bool ScrollAnimatorMac::pinnedInDirection(const FloatSize& direction) const
{
FloatSize limitDelta;
if (fabsf(direction.height()) >= fabsf(direction.width())) {
@@ -1246,7 +1246,7 @@
return wheelEvent.phase() == PlatformWheelEventPhaseMayBegin || wheelEvent.phase() == PlatformWheelEventPhaseBegan;
}
-bool ScrollAnimatorMac::isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent& wheelEvent, ScrollEventAxis axis)
+bool ScrollAnimatorMac::isAlreadyPinnedInDirectionOfGesture(const PlatformWheelEvent& wheelEvent, ScrollEventAxis axis) const
{
switch (axis) {
case ScrollEventAxis::Vertical:
@@ -1272,7 +1272,7 @@
}
#endif
-bool ScrollAnimatorMac::allowsVerticalStretching(const PlatformWheelEvent& wheelEvent)
+bool ScrollAnimatorMac::allowsVerticalStretching(const PlatformWheelEvent& wheelEvent) const
{
switch (m_scrollableArea.verticalScrollElasticity()) {
case ScrollElasticityAutomatic: {
@@ -1296,7 +1296,7 @@
return false;
}
-bool ScrollAnimatorMac::allowsHorizontalStretching(const PlatformWheelEvent& wheelEvent)
+bool ScrollAnimatorMac::allowsHorizontalStretching(const PlatformWheelEvent& wheelEvent) const
{
switch (m_scrollableArea.horizontalScrollElasticity()) {
case ScrollElasticityAutomatic: {
@@ -1320,12 +1320,12 @@
return false;
}
-IntSize ScrollAnimatorMac::stretchAmount()
+IntSize ScrollAnimatorMac::stretchAmount() const
{
return m_scrollableArea.overhangAmount();
}
-bool ScrollAnimatorMac::canScrollHorizontally()
+bool ScrollAnimatorMac::canScrollHorizontally() const
{
Scrollbar* scrollbar = m_scrollableArea.horizontalScrollbar();
if (!scrollbar)
@@ -1333,7 +1333,7 @@
return scrollbar->enabled();
}
-bool ScrollAnimatorMac::canScrollVertically()
+bool ScrollAnimatorMac::canScrollVertically() const
{
Scrollbar* scrollbar = m_scrollableArea.verticalScrollbar();
if (!scrollbar)
@@ -1341,7 +1341,7 @@
return scrollbar->enabled();
}
-bool ScrollAnimatorMac::shouldRubberBandInDirection(ScrollDirection)
+bool ScrollAnimatorMac::shouldRubberBandInDirection(ScrollDirection) const
{
return false;
}
Modified: trunk/Source/WebCore/platform/mock/ScrollAnimatorMock.h (251172 => 251173)
--- trunk/Source/WebCore/platform/mock/ScrollAnimatorMock.h 2019-10-16 01:04:41 UTC (rev 251172)
+++ trunk/Source/WebCore/platform/mock/ScrollAnimatorMock.h 2019-10-16 01:27:33 UTC (rev 251173)
@@ -45,13 +45,13 @@
private:
#if ENABLE(RUBBER_BANDING)
- bool allowsHorizontalStretching(const PlatformWheelEvent&) override { return false; }
- bool allowsVerticalStretching(const PlatformWheelEvent&) override { return false; }
- IntSize stretchAmount() override { return IntSize(); }
- bool pinnedInDirection(const FloatSize&) override { return false; }
- bool canScrollHorizontally() override { return false; }
- bool canScrollVertically() override { return false; }
- bool shouldRubberBandInDirection(ScrollDirection) override { return false; }
+ bool allowsHorizontalStretching(const PlatformWheelEvent&) const override { return false; }
+ bool allowsVerticalStretching(const PlatformWheelEvent&) const override { return false; }
+ IntSize stretchAmount() const override { return IntSize(); }
+ bool pinnedInDirection(const FloatSize&) const override { return false; }
+ bool canScrollHorizontally() const override { return false; }
+ bool canScrollVertically() const override { return false; }
+ bool shouldRubberBandInDirection(ScrollDirection) const override { return false; }
void immediateScrollBy(const FloatSize&) override { }
void immediateScrollByWithoutContentEdgeConstraints(const FloatSize&) override { }
void adjustScrollPositionToBoundsIfNecessary() override { }
