Changeset 286119 in webkit

Timestamp:

Nov 22, 2021, 1:30:08 PM (4 years ago)

Author:

Simon Fraser

Message:

Clarify the behavior of ScrollAnimator::scroll()
​https://bugs.webkit.org/show_bug.cgi?id=233403

Unreviewed.

Address patch feedback from Chris Lord.

• dom/Element.cpp:

(WebCore::Element::scrollByUnits):

• page/FrameView.cpp:

(WebCore::FrameView::adjustVerticalPageScrollStepForFixedContent):
(WebCore::FrameView::adjustScrollStepForFixedContent): Deleted.

• page/FrameView.h:
• platform/ScrollAnimator.cpp:

(WebCore::ScrollAnimator::singleAxisScroll):

• platform/ScrollTypes.h:

(WebCore::axisFromDirection):
(WebCore::valueForAxis):
(WebCore::setValueForAxis):

• platform/ScrollableArea.cpp:

(WebCore::ScrollableArea::adjustVerticalPageScrollStepForFixedContent):
(WebCore::ScrollableArea::scroll):
(WebCore::ScrollableArea::adjustScrollStepForFixedContent): Deleted.

• platform/ScrollableArea.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• page/FrameView.cpp (modified) (1 diff)
• page/FrameView.h (modified) (1 diff)
• platform/ScrollAnimator.cpp (modified) (1 diff)
• platform/ScrollTypes.h (modified) (5 diffs)
• platform/ScrollableArea.cpp (modified) (2 diffs)
• platform/ScrollableArea.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-22  Simon Fraser  <simon.fraser@apple.com>
+
+        Clarify the behavior of ScrollAnimator::scroll()
+        https://bugs.webkit.org/show_bug.cgi?id=233403
+
+        Unreviewed.
+
+        Address patch feedback from Chris Lord.
+
+        * dom/Element.cpp:
+        (WebCore::Element::scrollByUnits):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::adjustVerticalPageScrollStepForFixedContent):
+        (WebCore::FrameView::adjustScrollStepForFixedContent): Deleted.
+        * page/FrameView.h:
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::singleAxisScroll):
+        * platform/ScrollTypes.h:
+        (WebCore::axisFromDirection):
+        (WebCore::valueForAxis):
+        (WebCore::setValueForAxis):
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::adjustVerticalPageScrollStepForFixedContent):
+        (WebCore::ScrollableArea::scroll):
+        (WebCore::ScrollableArea::adjustScrollStepForFixedContent): Deleted.
+        * platform/ScrollableArea.h:
+
 2021-11-22  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -1119 +1119 @@
         return;
 
-    ScrollDirection direction = ScrollDown;
-    if (units < 0)
-        direction = ScrollUp;
-
+    auto direction = units < 0 ? ScrollUp : ScrollDown;
     auto* stopElement = this;
     downcast<RenderBox>(*renderer).scroll(direction, granularity, std::abs(units), &stopElement);

trunk/Source/WebCore/page/FrameView.cpp

@@ -3835 +3835 @@
 }
 
-float FrameView::adjustScrollStepForFixedContent(float step, ScrollEventAxis axis, ScrollGranularity granularity)
-{
-    if (granularity != ScrollGranularity::Page || axis == ScrollEventAxis::Horizontal)
-        return step;
-
+float FrameView::adjustVerticalPageScrollStepForFixedContent(float step)
+{
     TrackedRendererListHashSet* positionedObjects = nullptr;
     if (RenderView* root = frame().contentRenderer()) {

trunk/Source/WebCore/page/FrameView.h

@@ -662 +662 @@
     void updateScrollingCoordinatorScrollSnapProperties() const;
 
-    float adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity) final;
+    float adjustVerticalPageScrollStepForFixedContent(float step) final;
 
     void didChangeScrollOffset();

trunk/Source/WebCore/platform/ScrollAnimator.cpp

@@ -70 +70 @@
     auto delta = setValueForAxis(FloatSize { }, axis, scrollDelta);
 
-    if (behavior.contains(ScrollBehavior::RespectScrollSnap)) {
-        behavior.remove(ScrollBehavior::RespectScrollSnap);
-        if (m_scrollController.usesScrollSnap()) {
-            auto currentOffset = offsetFromPosition(currentPosition());
-            auto newOffset = currentOffset + delta;
-            auto velocity = copysignf(1.0f, scrollDelta);
-            auto newOffsetOnAxis = m_scrollController.adjustedScrollDestination(axis, newOffset, velocity, valueForAxis(currentOffset, axis));
-            newOffset = setValueForAxis(newOffset, axis, newOffsetOnAxis);
-            delta = newOffset - currentOffset;
-        }
+    if (behavior.contains(ScrollBehavior::RespectScrollSnap) && m_scrollController.usesScrollSnap()) {
+        auto currentOffset = offsetFromPosition(currentPosition());
+        auto newOffset = currentOffset + delta;
+        auto velocity = copysignf(1.0f, scrollDelta);
+        auto newOffsetOnAxis = m_scrollController.adjustedScrollDestination(axis, newOffset, velocity, valueForAxis(currentOffset, axis));
+        newOffset = setValueForAxis(newOffset, axis, newOffsetOnAxis);
+        delta = newOffset - currentOffset;
     }
 

trunk/Source/WebCore/platform/ScrollTypes.h

@@ -180 +180 @@
     case ScrollRight: return ScrollEventAxis::Horizontal;
     }
+    ASSERT_NOT_REACHED();
     return ScrollEventAxis::Vertical;
 }
@@ -189 +190 @@
     case ScrollEventAxis::Vertical: return size.height();
     }
+    ASSERT_NOT_REACHED();
     return 0;
 }
@@ -202 +204 @@
         return size;
     }
+    ASSERT_NOT_REACHED();
     return size;
 }
@@ -211 +214 @@
     case ScrollEventAxis::Vertical: return point.y();
     }
+    ASSERT_NOT_REACHED();
     return 0;
 }
@@ -223 +227 @@
         return point;
     }
+    ASSERT_NOT_REACHED();
     return point;
 }

trunk/Source/WebCore/platform/ScrollableArea.cpp

@@ -92 +92 @@
 }
 
-float ScrollableArea::adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity)
+float ScrollableArea::adjustVerticalPageScrollStepForFixedContent(float step)
 {
     return step;
@@ -120 +120 @@
 
     auto axis = axisFromDirection(direction);
-    step = adjustScrollStepForFixedContent(step, axis, granularity);
+
+    if (granularity == ScrollGranularity::Page && axis == ScrollEventAxis::Vertical)
+        step = adjustVerticalPageScrollStepForFixedContent(step);
+
     auto scrollDelta = step * stepCount;
     

trunk/Source/WebCore/platform/ScrollableArea.h

@@ -379 +379 @@
     void resetScrollOriginChanged() { m_scrollOriginChanged = false; }
 
-    WEBCORE_EXPORT virtual float adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity);
+    WEBCORE_EXPORT virtual float adjustVerticalPageScrollStepForFixedContent(float step);
     virtual void invalidateScrollbarRect(Scrollbar&, const IntRect&) = 0;
     virtual void invalidateScrollCornerRect(const IntRect&) = 0;