[css-grid] Remove a duplicated auto repeat computation for intrinsic sizes

third_party/WebKit/Source/core/layout/LayoutGrid.h

Index: third_party/WebKit/Source/core/layout/LayoutGrid.h
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.h b/third_party/WebKit/Source/core/layout/LayoutGrid.h
index 90a906943bc7771e8ee9cf0b3c659f27cb7605d5..e8c0f2f37a3faa03c8994fd26d26515224997a01 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.h
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.h
@@ -58,7 +58,7 @@ public:
 
     void layoutBlock(bool relayoutChildren) override;
 
-    void dirtyGrid();
+    void dirtyGrid() const;

[Manuel Rego, 2016/10/06 12:31:30]

I still don't get why we want to make this method const.
It seems really confusing to me, as dirtyGrid() is actually changing the grid.

Why insertItemIntoGrid() is not const then?
It only modifies m_grid which is mutable now.

[svillar, 2016/10/06 13:14:33]

Because we want to use it in the intrinsic size computation method which is
const
It is not semantically. It only touches cached data, i.e. attributes that are
not mandatory for the layout object but that ease the implementation/make it
faster.

The problem is perhaps the name. I wanted to rename it to clearCachedData() but
I left it for another patch.

 
     Vector<LayoutUnit> trackSizesForComputedStyle(GridTrackSizingDirection) const;
 
@@ -125,6 +125,7 @@ private:
     void ensureGridSize(size_t maximumRowSize, size_t maximumColumnSize);
     void insertItemIntoGrid(LayoutBox&, const GridArea&);
 
+    void updateAutoRepeatTracksAndSetDirtyIfNeeded(SizingOperation) const;
     size_t computeAutoRepeatTracksCount(GridTrackSizingDirection, SizingOperation) const;
 
     typedef ListHashSet<size_t> OrderedTrackIndexSet;
@@ -133,7 +134,7 @@ private:
     bool hasAutoRepeatEmptyTracks(GridTrackSizingDirection) const;
     bool isEmptyAutoRepeatTrack(GridTrackSizingDirection, size_t lineNumber) const;
 
-    void placeItemsOnGrid(SizingOperation);
+    void placeItemsOnGrid();
     void populateExplicitGridAndOrderIterator();
     std::unique_ptr<GridArea> createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(const LayoutBox&, GridTrackSizingDirection, const GridSpan& specifiedPositions) const;
     void placeSpecifiedMajorAxisItemsOnGrid(const Vector<LayoutBox*>&);
@@ -224,16 +225,16 @@ private:
     bool cachedHasDefiniteLogicalHeight() const;
 
     typedef Vector<Vector<GridCell>> GridRepresentation;
-    GridRepresentation m_grid;
-    bool m_gridIsDirty;
+    mutable GridRepresentation m_grid;
+    mutable bool m_gridIsDirty;
     Vector<LayoutUnit> m_rowPositions;

[Manuel Rego, 2016/10/06 12:31:30]

Why some attributes are mutables and other don't?
What's the reasoning behind?

For example m_rowPositions is also a value calculated during layout,
like the rest of attributes in the class.
Why do we just mark a few as "mutable" and not all?

[svillar, 2016/10/06 13:14:33]

Why do you want to make them all mutable or none? What's the rationale of that?
 
This totally depends on what you consider is part of the state of the object and
what is not. For me there are some attributes that are really cached data (like
m_gridItemsOverflowingGridArea), i.e. attributes that are not mandatory but that
help. Some others are subject to further discussion I agree.

This is the minimal subset that allows us to have a const dirtyGrid().

     Vector<LayoutUnit> m_columnPositions;
     LayoutUnit m_offsetBetweenColumns;
     LayoutUnit m_offsetBetweenRows;
-    HashMap<const LayoutBox*, GridArea> m_gridItemArea;
+    mutable HashMap<const LayoutBox*, GridArea> m_gridItemArea;
     OrderIterator m_orderIterator;
-    Vector<LayoutBox*> m_gridItemsOverflowingGridArea;
-    HashMap<const LayoutBox*, size_t> m_gridItemsIndexesMap;
+    mutable Vector<LayoutBox*> m_gridItemsOverflowingGridArea;
+    mutable HashMap<const LayoutBox*, size_t> m_gridItemsIndexesMap;
 
     LayoutUnit m_minContentHeight { -1 };
     LayoutUnit m_maxContentHeight { -1 };
@@ -241,13 +242,13 @@ private:
     int m_smallestRowStart;
     int m_smallestColumnStart;
 
-    size_t m_autoRepeatColumns { 0 };
-    size_t m_autoRepeatRows { 0 };
+    mutable size_t m_autoRepeatColumns { 0 };
+    mutable size_t m_autoRepeatRows { 0 };
 
     bool m_hasAnyOrthogonalChild;
 
-    std::unique_ptr<OrderedTrackIndexSet> m_autoRepeatEmptyColumns { nullptr };
-    std::unique_ptr<OrderedTrackIndexSet> m_autoRepeatEmptyRows { nullptr };
+    mutable std::unique_ptr<OrderedTrackIndexSet> m_autoRepeatEmptyColumns { nullptr };
+    mutable std::unique_ptr<OrderedTrackIndexSet> m_autoRepeatEmptyRows { nullptr };
 
     Optional<bool> m_hasDefiniteLogicalHeight;
 };