[webkit-reviews] review granted: [Bug 60490] Create LayerOffset class : [Attachment 92889] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Levi Weintraub <leviw at chromium.org>'s
request for review:
Bug 60490: Create LayerOffset class
https://bugs.webkit.org/show_bug.cgi?id=60490

Attachment 92889: Patch
https://bugs.webkit.org/attachment.cgi?id=92889&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92889&action=review

I think this is good.  I think we can land this as-is.	But I think you should
be sure to wave this in front of maciej's nose at some point (that can be after
landing), just to see what his thoughts are on introducing this point/size
in-between.

> Source/WebCore/platform/graphics/LayerOffset.h:34
> +// LayerOffset represents the off

you trailed off...

> Source/WebCore/platform/graphics/LayerOffset.h:42
> +    LayerOffset(const IntSize& size) : m_x(size.width()), m_y(size.height())
{ }
> +    LayerOffset(const IntPoint& point) : m_x(point.x()), m_y(point.y()) { }

I wonder if it's a good idea to have implicit constructors here.

> Source/WebCore/rendering/RenderScrollbar.cpp:278
> +    partRenderer->paintIntoRect(graphicsContext, pos(), rect);

seems pos() needs to be renamed in some later patch. :)

> Source/WebCore/rendering/RenderScrollbarPart.cpp:165
> +    setLocation(rect.location() - layerOffset.toSize());

I wonder if there is a moveLocation or something that takes a size.