[webkit-reviews] review granted: [Bug 61414] Replace RenderLayer::x/y/width/height with location/size : [Attachment 94875] 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 94875: Patch
https://bugs.webkit.org/attachment.cgi?id=94875&action=review

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

I don't need to see this again, but you should look at fixing some of the below
mentioned nits before landing (with teh cq or otherwise).

> Source/WebCore/dom/MouseRelatedEvent.cpp:209
> +	       m_layerX -= location.x();
> +	       m_layerY -= location.y();

An obvious next candidate? :)

> Source/WebCore/rendering/RenderInline.cpp:1083
> +	   IntRect boxRect(IntPoint(), containerBox->layer()->size());
>	   rect = intersection(repaintRect, boxRect);

What is similar about this and the previous function?  Should IntRect
rect(IntPoint(), containingBlock->layer()->size()); 
rect.intercect(repaintRect, boxRect); be some helper fucntion?

> Source/WebCore/rendering/RenderLayer.cpp:655
> +	   inlineBoundingBoxOffset = toSize(lineBox.location());

I take it we youse inlineBoundingBoxOffset again later, or should this just get
merged into the localPoint += line below?

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

I feel like this needs a FIXME to explain why we assume the location is 0, 0.

> Source/WebCore/rendering/RenderLayer.cpp:3453
>      convertToLayerCoords(rootLayer, x, y);

An obvious next target.

> Source/WebCore/rendering/RenderLayer.h:220
> +    IntRect rect() const { return IntRect(location(), size()); }

Seems we should just move m_topLeft and m_layerSize into a rect soon. :) 
Unless the location and size don't actually make sense paired as a rect?
(except for DRT dumping).

> Source/WebCore/rendering/RenderView.cpp:331
> +    quads.append(FloatRect(IntPoint(), m_layer->size()));

I think you meant FloatPoint() here.