[webkit-reviews] review granted: [Bug 204882] Add support for scroll behavior relies on ScrollAnimation of the Web process : [Attachment 389500] Patch

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has granted cathiechen
<cathiechen at igalia.com>'s request for review:
Bug 204882: Add support for scroll behavior relies on ScrollAnimation of the
Web process
https://bugs.webkit.org/show_bug.cgi?id=204882

Attachment 389500: Patch

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




--- Comment #62 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 389500
  --> https://bugs.webkit.org/attachment.cgi?id=389500
Patch

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

The pattern X && useSmoothScrolling(options.behavior, *X) is very frequent. Do
we always need to check X != nullptr ? Maybe you want to move this into
useSmoothScrolling instead (i.e. make the parameter a pointer) although I think
references are preferred in WebKit. Alternatively, you can maybe follow
https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op to avoid
very long lines.

> Source/WebCore/rendering/RenderLayer.cpp:2828
> +	       bool animated = (box->element() &&
useSmoothScrolling(options.behavior, *box->element()));

parenthesis not needed here