[webkit-reviews] review granted: [Bug 218477] Scroll position can get reset after programmatic scroll : [Attachment 413005] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 3 02:52:37 PST 2020
Antti Koivisto <koivisto at iki.fi> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 218477: Scroll position can get reset after programmatic scroll
https://bugs.webkit.org/show_bug.cgi?id=218477
Attachment 413005: Patch
https://bugs.webkit.org/attachment.cgi?id=413005&action=review
--- Comment #4 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 413005
--> https://bugs.webkit.org/attachment.cgi?id=413005
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 { }.
More information about the webkit-reviews
mailing list