[webkit-reviews] review granted: [Bug 83147] Update LayoutUnit usage in RenderView : [Attachment 135565] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Levi Weintraub
<leviw at chromium.org>'s request for review:
Bug 83147: Update LayoutUnit usage in RenderView
https://bugs.webkit.org/show_bug.cgi?id=83147

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=135565&action=review


> Source/WebCore/rendering/RenderView.cpp:221
> +    // RenderViews should never be called to paint with an offset not on
device pixels.
> +    ASSERT(LayoutPoint(IntPoint(paintOffset.x(), paintOffset.y())) ==
paintOffset);

I feel this will be used at several places to ensure that we properly converted
/ rounded. Maybe it would be worth having a convenience function to check that
(like bool isPixelAlignedLayoutPoint)?

> Source/WebCore/rendering/RenderView.cpp:-329
> -	   // FIXME: Hardcoded offsets here are not good.

I think this comment still applies. I read this comment as: the offset is
hardcoded to the start of the object padding box (which it still is). I wonder
if it really does add something though.

> Source/WebCore/rendering/RenderView.h:72
> +    virtual void computeRectForRepaint(RenderBoxModelObject*
repaintContainer, LayoutRect&, bool fixed = false) const;
> +    virtual void repaintViewRectangle(const LayoutRect&, bool immediate =
false);

please decorate the virtual overridden functions as you go with OVERRIDE (not
repeated).