Skip to content

Commit

Permalink
Cherry-pick 270820@main (50aa992). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=259912

    [WPE][Debug] ASSERTION FAILED: inProgrammaticScroll == (options.type == ScrollType::Programmatic)
    https://bugs.webkit.org/show_bug.cgi?id=259912

    Reviewed by Simon Fraser.

    In `AsyncScrollingCoordinator::requestScrollToPosition()` we assert
    that the `scrollableArea`'s `currentScrollType()` matches the
    `ScrollPositionChangeOptions`'s type.

    In some circumstances, this method can be called as a result of
    calling `ScrollableArea::scrollToPositionWithAnimation()`, which
    currently doesn't set its `currentScrollType` from `options` argument.

    `LocalFrameView`, which is a subclass of `ScrollableArea`, adds a method
    whose only job is to set/restore `currentScrollType` and call parent's
    method. We can move this logic to the `ScrollableArea` class. There's
    actually a FIXME about this.

    * Source/WebCore/page/LocalFrameView.cpp:
    (WebCore::LocalFrameView::scrollToPositionWithAnimation): Deleted.
    * Source/WebCore/page/LocalFrameView.h:
    * Source/WebCore/platform/ScrollableArea.cpp:
    (WebCore::ScrollableArea::scrollToPositionWithAnimation):

    Canonical link: https://commits.webkit.org/270820@main
  • Loading branch information
obyknovenius authored and aperezdc committed Jan 26, 2024
1 parent 00ff125 commit 20a9a77
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 17 deletions.
15 changes: 0 additions & 15 deletions Source/WebCore/page/LocalFrameView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4265,21 +4265,6 @@ void LocalFrameView::scrollTo(const ScrollPosition& newPosition)
didChangeScrollOffset();
}

void LocalFrameView::scrollToPositionWithAnimation(const ScrollPosition& position, const ScrollPositionChangeOptions& options)
{
// FIXME: Why isn't all this in ScrollableArea?
auto previousScrollType = currentScrollType();
setCurrentScrollType(options.type);

if (scrollAnimationStatus() == ScrollAnimationStatus::Animating)
scrollAnimator().cancelAnimations();

if (position != scrollPosition())
ScrollableArea::scrollToPositionWithAnimation(position, options);

setCurrentScrollType(previousScrollType);
}

float LocalFrameView::adjustVerticalPageScrollStepForFixedContent(float step)
{
TrackedRendererListHashSet* positionedObjects = nullptr;
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/page/LocalFrameView.h
Original file line number Diff line number Diff line change
Expand Up @@ -718,8 +718,6 @@ class LocalFrameView final : public FrameView {

void renderLayerDidScroll(const RenderLayer&);

void scrollToPositionWithAnimation(const ScrollPosition&, const ScrollPositionChangeOptions& options = ScrollPositionChangeOptions::createProgrammatic());

bool inUpdateEmbeddedObjects() const { return m_inUpdateEmbeddedObjects; }

String debugDescription() const final;
Expand Down
11 changes: 11 additions & 0 deletions Source/WebCore/platform/ScrollableArea.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,23 @@ void ScrollableArea::scrollToPositionWithAnimation(const FloatPoint& position, c
{
LOG_WITH_STREAM(Scrolling, stream << "ScrollableArea " << this << " scrollToPositionWithAnimation " << position);

if (scrollAnimationStatus() == ScrollAnimationStatus::Animating)
scrollAnimator().cancelAnimations();

if (position == scrollPosition())
return;

auto previousScrollType = currentScrollType();
setCurrentScrollType(options.type);

bool startedAnimation = requestScrollToPosition(roundedIntPoint(position), { ScrollType::Programmatic, options.clamping, ScrollIsAnimated::Yes, options.snapPointSelectionMethod, options.originalScrollDelta });
if (!startedAnimation)
startedAnimation = scrollAnimator().scrollToPositionWithAnimation(position, options.clamping);

if (startedAnimation)
setScrollAnimationStatus(ScrollAnimationStatus::Animating);

setCurrentScrollType(previousScrollType);
}

void ScrollableArea::scrollToOffsetWithoutAnimation(const FloatPoint& offset, ScrollClamping clamping)
Expand Down

0 comments on commit 20a9a77

Please sign in to comment.