[webkit-reviews] review denied: [Bug 59753] Chromium Mac: Use ScrollAnimatorMac.mm for overlay scrollbar support : [Attachment 91733] Fixed change log

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Sailesh Agrawal <sail at chromium.org>'s
request for review:
Bug 59753: Chromium Mac: Use ScrollAnimatorMac.mm for overlay scrollbar support
https://bugs.webkit.org/show_bug.cgi?id=59753

Attachment 91733: Fixed change log
https://bugs.webkit.org/attachment.cgi?id=91733&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91733&action=review

> Source/WebCore/platform/mac/ScrollAnimatorMac.h:29
> +#if ENABLE(SMOOTH_SCROLLING) || (PLATFORM(CHROMIUM) && OS(DARWIN))

Why doesn't Chromium on Mac just enable SMOOTH_SCROLLING if you want to take
this code path.  Adding additional defines defeats the purpose of have fine
grained enabled macros.

> Source/WebCore/platform/mac/ScrollAnimatorMac.mm:54
> +static NSPoint IntPointToNSPoint(WebCore::IntPoint point) {
> +    return NSMakePoint(point.x(), point.y());
> +}
> +
> +static WebCore::IntPoint NSPointToIntPoint(NSPoint point) {
> +    return WebCore::IntPoint(point.x, point.y);
> +}

{ needs to be on the next line. Function can't start with a capital.

> Source/WebCore/platform/mac/ScrollAnimatorMac.mm:221
> +    return
IntPointToNSPoint(_animator->scrollableArea()->currentMousePosition());

Why is this necessary, IntPoint has a conversion to NSPoint.