[webkit-reviews] review granted: [Bug 61414] Replace RenderLayer::x/y/width/height with location/size : [Attachment 94737] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Emil A Eklund <eae at chromium.org>'s
request for review:
Bug 61414: Replace RenderLayer::x/y/width/height with location/size
https://bugs.webkit.org/show_bug.cgi?id=61414

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

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

> Source/WebCore/rendering/RenderLayer.cpp:655
>	   inlineBoundingBoxOffset = IntSize(lineBox.x(), lineBox.y());

I assume lineBox has a location() function by now.

> Source/WebCore/rendering/RenderLayer.cpp:2007
> +    IntPoint bottomRight = toPoint(size());

Kinda a funny assumption that location() is always 0, 0.  Seems this should be
asking for m_frameRect.bottomRight() or whatever location() + size() is.  Or
maybe we should be ASSERTing that location() is 0, 0?  Or maybe its that the
internal and external size of the layer are considered the same, and that the
offset this returns is in local coordinates?  Confused.

> Source/WebCore/rendering/RenderLayer.cpp:3454
>      convertToLayerCoords(rootLayer, x, y);
> -    layerBounds = IntRect(x, y, width(), height());
> +    layerBounds = IntRect(IntPoint(x, y), size());

Seems odd that we wouldn't just map the whole rect.

> Source/WebCore/rendering/RenderTreeAsText.cpp:583
> +		       writeLayers(ts, l, l, IntRect(l->location(), l->size()),
indent + 1, behavior);

Why not just ask for l->rect()?

> Source/WebCore/rendering/RenderTreeAsText.cpp:778
> +	   writeLayers(ts, l, l, IntRect(l->location(), l->size()), 0,
behavior);

l->rect() here?