[webkit-reviews] review denied: [Bug 65396] Switch RenderBlock to to new layout types : [Attachment 102403] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Emil A Eklund <eae at chromium.org>'s
request for review:
Bug 65396: Switch RenderBlock to to new layout types
https://bugs.webkit.org/show_bug.cgi?id=65396

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102403&action=review


No EWS because the patch doesn’t apply.

I suggest putting in a patch that only adds the new typedef and is entirely
mechanical, and leaving the part that introduces new functions as a separate
far smaller patch.

> Source/WebCore/platform/graphics/IntRect.cpp:142
> +IntRect enclosingIntRect(const IntRect& rect)
> +{
> +    return IntRect(rect);
> +}

This must be inlined in the header. We do not want to pay a function call to
convert an IntRect to an IntRect.

> Source/WebCore/rendering/LayoutTypes.h:89
> +inline bool layoutUnitEquals(LayoutUnit a, LayoutUnit b)

If the idea here is to use a definition that involves epsilon, then I think you
need a different name. After all, == does not use epsilon, so the name "equal"
is already taken.