[webkit-reviews] review denied: [Bug 61893] Switch ContainerNode to use IntPoint : [Attachment 95690] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Emil A Eklund <eae at chromium.org>'s
request for review:
Bug 61893: Switch ContainerNode to use IntPoint
https://bugs.webkit.org/show_bug.cgi?id=61893

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

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

I think we should consider renaming the move() calls to moveBy() or something
to make it clear what move(Point) is doing.  (I know rect and IntPoint use the
same naming, so maybe that should be a separate change?)

> Source/WebCore/dom/ContainerNode.cpp:881
> +		   point.move(box->location());

Bleh.  I'm liking generic move(IntPoint) less and less.  it's not clear if it
sets or moves.

> Source/WebCore/dom/ContainerNode.cpp:907
> +	   point.move(box->size());

move(Size) is OK, but we still might want to rename both of these to moveBy.

> Source/WebCore/platform/graphics/FloatPoint.h:107
> +	   return IntPoint(m_x > other.m_x ? m_x : other.m_x,
> +	       m_y > other.m_y ? m_y : other.m_y);

I don't think the wrapping his helpful here.