[webkit-reviews] review granted: [Bug 231395] Minor refactoring to use ScrollUpdate in more places : [Attachment 440544] Patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 231395: Minor refactoring to use ScrollUpdate in more places
https://bugs.webkit.org/show_bug.cgi?id=231395

Attachment 440544: Patch

https://bugs.webkit.org/attachment.cgi?id=440544&action=review




--- Comment #2 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 440544
  --> https://bugs.webkit.org/attachment.cgi?id=440544
Patch

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

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:263
> +	   auto scrollUpdate = ScrollUpdate { scrollingNodeID, scrollPosition,
{ }, ScrollingLayerPositionAction::Set };

Why not ScrollUpdate scrollUpdate(scrollingNodeID, scrollPosition, { },
ScrollingLayerPositionAction::Set);?

Or even applyScrollUpdate(ScrollUpdate(scrollingNodeID, scrollPosition, { },
ScrollingLayerPositionAction::Set), ScrollType::Programmatic);?

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:309
> +	   applyScrollPositionUpdate(WTFMove(update), ScrollType::User);

Cool.

> Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h:105
> +struct ScrollUpdate {

If you're going to move this, why not move it into its own file?