Bug 218477 – Scroll position can get reset after programmatic scroll

• Status: RESOLVED
• Resolution: FIXED
• Component: Scrolling

[Simon Fraser (smfr)]

Scroll position can get reset after programmatic scroll

[Simon Fraser (smfr)]

Created attachment 413001 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/70978102>

[Simon Fraser (smfr)]

Created attachment 413005 [details]
Patch

[Antti Koivisto]

Comment on attachment 413005 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413005&action=review

> Source/WebCore/page/scrolling/ScrollingTree.cpp:405
> +//    LOG(Scrolling, "\nScrollingTree %p applyLayerPositions (main thread %d)", this, isMainThread());

Stray //

> Source/WebCore/page/scrolling/ScrollingTree.cpp:582
> +    LockHolder locker(m_pendingScrollUpdatesLock);

I prefer

auto locker = holdLock(m_pendingScrollUpdatesLock);

> Source/WebCore/page/scrolling/ScrollingTree.h:221
> +        ScrollUpdate() = default;
> +        ScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, ScrollingLayerPositionAction udpateAction)
> +            : nodeID(scrollingNodeID)
> +            , scrollPosition(point)
> +            , layoutViewportOrigin(viewportOrigin)
> +            , updateLayerPositionAction(udpateAction)
> +        { }

You can probably delete these explicit constructors and the code will still compile as-is.

> Source/WebCore/page/scrolling/ScrollingTree.h:225
> +        FloatPoint scrollPosition;
> +        Optional<FloatPoint> layoutViewportOrigin;

...except these may need { }.

[Antti Koivisto]

Comment on attachment 413005 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413005&action=review

> Source/WebCore/page/scrolling/ScrollingTree.h:216
> +        ScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, ScrollingLayerPositionAction udpateAction)

Spelling udpateAction

[Simon Fraser (smfr)]

https://trac.webkit.org/r269312