[webkit-reviews] review granted: [Bug 107062] [CSS Grid Layout] Updating -webkit-grid-rows or -webkit-grid-columns doesn't work as expected : [Attachment 183071] Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method.

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 107062: [CSS Grid Layout] Updating -webkit-grid-rows or
-webkit-grid-columns doesn't work as expected
https://bugs.webkit.org/show_bug.cgi?id=107062

Attachment 183071: Proposed fix 1: Force a relayout if the grid area size
changed and fixed a boolean logic in the style diff method.
https://bugs.webkit.org/attachment.cgi?id=183071&action=review

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


> Source/WebCore/rendering/RenderGrid.cpp:236
> +	       LayoutUnit oldOverrideContainingBlockContentLogicalWidth =
child->hasOverrideContainingBlockLogicalWidth() ?
child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
> +	       LayoutUnit oldOverrideContainingBlockContentLogicalHeight =
child->hasOverrideContainingBlockLogicalHeight() ?
child->overrideContainingBlockContentLogicalHeight() : LayoutUnit();
> +	       LayoutUnit newOverrideContainingBlockContentLogicalWidth =
columnTracks[columnTrack].m_usedBreadth;
> +	       LayoutUnit newOverrideContainingBlockContentLogicalHeight =
rowTracks[rowTrack].m_usedBreadth;

This is pretty hard to read.  How about:
LayoutUnit oldOverrideContainingBlockContentLogicalWidth =
child->hasOverrideContainingBlockLogicalWidth() ?
child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
LayoutUnit oldOverrideContainingBlockContentLogicalHeight =
child->hasOverrideContainingBlockLogicalHeight() ?
child->overrideContainingBlockContentLogicalHeight() : LayoutUnit();

bool widthOrHeightChanged = oldOverrideContainingBlockContentLogicalWidth !=
columnTracks[columnTrack].m_usedBreadth ||
oldOverrideContainingBlockContentLogicalHeight !=
rowTracks[rowTrack].m_usedBreadth;
if (widthOrHeightChanged)
    child->setNeedsLayout(true, MarkOnlyThis);

child->setOverride...
child->setOverride...