[webkit-reviews] review denied: [Bug 60739] Switch paintCollapsedBorder to use IntRect : [Attachment 93386] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Levi Weintraub <leviw at chromium.org>'s
request for review:
Bug 60739: Switch paintCollapsedBorder to use IntRect
https://bugs.webkit.org/show_bug.cgi?id=60739

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

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

I don't think the adjustedPaintRect change is a ncessarily a win.

> Source/WebCore/rendering/RenderTableCell.cpp:931
> +    IntRect adjustedPaintRect = IntRect(paintRect.x() - leftWidth / 2,

You don't have to do an equal here, you can just construct directly.  Webkit
doesn't normally do cascading indents like this though.

> Source/WebCore/rendering/RenderTableCell.cpp:952
> +    borders.addBorder(topVal, BSTop, renderTop, adjustedPaintRect.x(),
adjustedPaintRect.y(), adjustedPaintRect.maxX(), adjustedPaintRect.y() +
topWidth, topStyle);
> +    borders.addBorder(bottomVal, BSBottom, renderBottom,
adjustedPaintRect.x(), adjustedPaintRect.maxY() - bottomWidth,
adjustedPaintRect.maxX(), adjustedPaintRect.maxY(), bottomStyle);
> +    borders.addBorder(leftVal, BSLeft, renderLeft, adjustedPaintRect.x(),
adjustedPaintRect.y(), adjustedPaintRect.x() + leftWidth,
adjustedPaintRect.maxY(), leftStyle);
> +    borders.addBorder(rightVal, BSRight, renderRight,
adjustedPaintRect.maxX() - rightWidth, adjustedPaintRect.y(),
adjustedPaintRect.maxX(), adjustedPaintRect.maxY(), rightStyle);

Seems like making the locals above might make this more clear?	Unleass you
plan to actually use the adjustedPaintRect as an object somewhere?