[CSS Grid Layout] Store m_grid across layout

Source/core/rendering/RenderGrid.cpp

Index: Source/core/rendering/RenderGrid.cpp
diff --git a/Source/core/rendering/RenderGrid.cpp b/Source/core/rendering/RenderGrid.cpp
index 837e189c51a5550da3f8eae5af4e09a4b2f776da..70913b9ac54d70e4c06eafc477380235f050e1d4 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -160,6 +160,69 @@ RenderGrid::~RenderGrid()
 {
 }
 
+void RenderGrid::addChild(RenderObject* newChild, RenderObject* beforeChild)
+{
+    RenderBlock::addChild(newChild, beforeChild);
+
+    if (gridIsDirty() || !newChild->isBox())

[ojan, 2013/07/30 23:18:57]

Don't we force the children to be boxes? Can the second bit here be replaced
with an ASSERT(newChild->isBox())?

[Julien - ping for review, 2013/07/30 23:57:42]

Good catch, it seems to be the case.

+        return;
+
+    RenderBox* newChildBox = toRenderBox(newChild);
+    OwnPtr<GridSpan> rowPositions = resolveGridPositionsFromStyle(newChildBox, ForRows);
+    OwnPtr<GridSpan> columnPositions = resolveGridPositionsFromStyle(newChildBox, ForColumns);
+    if (!rowPositions || !columnPositions) {
+        // The new child requires the auto-placement algorithm to run so we need to recompute the grid fully.
+        dirtyGrid();
+    } else {
+        if (gridRowCount() <= rowPositions->finalPositionIndex || gridColumnCount() <= columnPositions->finalPositionIndex) {
+            // FIXME: We could just insert the new child provided we had a primitive to arbitrarily grow the grid.
+            dirtyGrid();
+        } else {
+            insertItemIntoGrid(newChildBox, GridCoordinate(*rowPositions, *columnPositions));
+        }
+    }
+}
+
+void RenderGrid::removeChild(RenderObject* child)
+{
+    RenderBlock::removeChild(child);
+
+    if (gridIsDirty() || !child->isBox())

[ojan, 2013/07/30 23:18:57]

Ditto.

+        return;
+
+    // FIXME: We could avoid dirtying the grid in some cases (e.g. if it's an explicitly positioned element).
+    dirtyGrid();
+}
+
+void RenderGrid::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
+{
+    RenderBlock::styleDidChange(diff, oldStyle);
+    if (!oldStyle)
+        return;
+
+    // FIXME: The following checks could be narrowed down if we kept track of which type of grid items we have:
+    // - explicit grid size changes impact negative explicitely positioned and auto-placed grid items.
+    // - named grid lines only impact grid items with named grid lines.
+    // - auto-flow changes only impacts auto-placed children.
+
+    if (explicitGridDidResize(oldStyle)
+        || namedGridLinesDefinitionDidChange(oldStyle)
+        || oldStyle->gridAutoFlow() != style()->gridAutoFlow())
+        dirtyGrid();
+}
+
+bool RenderGrid::explicitGridDidResize(const RenderStyle* oldStyle) const
+{
+    return oldStyle->gridDefinitionColumns().size() != style()->gridDefinitionColumns().size()
+        || oldStyle->gridDefinitionRows().size() != style()->gridDefinitionRows().size();
+}
+
+bool RenderGrid::namedGridLinesDefinitionDidChange(const RenderStyle* oldStyle) const
+{
+    return oldStyle->namedGridRowLines() != style()->namedGridRowLines()
+        || oldStyle->namedGridColumnLines() != style()->namedGridColumnLines();
+}
+
 void RenderGrid::layoutBlock(bool relayoutChildren, LayoutUnit)
 {
     ASSERT(needsLayout());
@@ -229,8 +292,6 @@ void RenderGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo
 
         // FIXME: This should add in the scrollbarWidth (e.g. see RenderFlexibleBox).
     }
-
-    const_cast<RenderGrid*>(this)->clearGrid();
 }
 
 void RenderGrid::computePreferredLogicalWidths()
@@ -621,7 +682,9 @@ void RenderGrid::insertItemIntoGrid(RenderBox* child, size_t rowTrack, size_t co
 
 void RenderGrid::placeItemsOnGrid()
 {
-    ASSERT(!gridWasPopulated());
+    if (!gridIsDirty())
+        return;
+
     ASSERT(m_gridItemCoordinate.isEmpty());
 
     populateExplicitGridAndOrderIterator();
@@ -753,8 +816,10 @@ RenderGrid::TrackSizingDirection RenderGrid::autoPlacementMinorAxisDirection() c
     return (flow == AutoFlowColumn) ? ForRows : ForColumns;
 }
 
-void RenderGrid::clearGrid()
+void RenderGrid::dirtyGrid()
 {
+    // An empty grid is a dirty grid as we always one item (so that gridColumnCount() can return an answer) but we
+    // could hold onto the memory to avoid massive free / malloc cycles if we stored the information differently.

[ojan, 2013/07/30 23:18:57]

A simple way to do this would be to resize(0) here and shrinkToFit at the end of
placeItemsOnGrid...which we should probably do anyways.

[Julien - ping for review, 2013/07/30 23:57:42]

Yup, I didn't want to piggyback on this change - especially with no data
supporting whether this is an issue - but it's probably not going to hurt us
anyway. Done!

[ojan, 2013/07/31 00:13:18]

Normally I'm violently against premature optimization, but in this case, given
that this situation is almost identical (performance-wise) to the order iterator
stuff and we know it mattered there, it's a pretty safe guess it'll matter here.

     m_grid.clear();
     m_gridItemCoordinate.clear();
 }
@@ -811,7 +876,6 @@ void RenderGrid::layoutGridItems()
     // FIXME: We should handle min / max logical height.
 
     setLogicalHeight(logicalHeight() + borderAndPaddingLogicalHeight());
-    clearGrid();
 }
 
 GridCoordinate RenderGrid::cachedGridCoordinate(const RenderBox* gridItem) const