[webkit-reviews] review granted: [Bug 111885] [CSS Grid Layout] Resolve grid-{end|after} integer against the end|after edge : [Attachment 192282] Proposed change 1: Propagate PositionSide information from CSS to do the right resolution in RenderGrid.

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 111885: [CSS Grid Layout] Resolve grid-{end|after} integer against the
end|after edge
https://bugs.webkit.org/show_bug.cgi?id=111885

Attachment 192282: Proposed change 1: Propagate PositionSide information from
CSS to do the right resolution in RenderGrid.
https://bugs.webkit.org/attachment.cgi?id=192282&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=192282&action=review


> Source/WebCore/rendering/RenderGrid.cpp:341
> +	   const GridPosition& initialPosition = (direction == ForColumns) ?
child->style()->gridItemStart() : child->style()->gridItemBefore();
> +	   const GridPosition& finalPosition = (direction == ForColumns) ?
child->style()->gridItemEnd() : child->style()->gridItemAfter();

I think it's a bit weird that we add the side information to GridPosition
because when we read the values here, it's clear what side we're talking about.
 

Another way to do this is to make another class (e.g., GridPositionWithSide)
and construct it here to pass around.  This would avoid using the extra memory
in RenderStyle and having to construct the grid position with side information.


Anyway, the current code is OK, but it seems redundant to store the direction
information in 2 different ways and trying to make sure it stays in sync.

> Source/WebCore/rendering/style/GridPosition.h:51
> +    GridPosition(GridPositionSide side)

explicit