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

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

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 40 matching lines @@
 class LayoutGrid final : public LayoutBlock {
 public:
     explicit LayoutGrid(Element*);
     ~LayoutGrid() override;
 
     static LayoutGrid* createAnonymous(Document*);
     const char* name() const override { return "LayoutGrid"; }
 
     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;
 
     const Vector<LayoutUnit>& columnPositions() const
     {
         ASSERT(!m_gridIsDirty);
         return m_columnPositions;
     }
 
     const Vector<LayoutUnit>& rowPositions() const
@@ skipping 46 matching lines @@
     struct GridSizingData;
     enum SizingOperation { TrackSizing, IntrinsicSizeComputation };
     void computeUsedBreadthOfGridTracks(GridTrackSizingDirection, GridSizingData&, LayoutUnit& baseSizesWithoutMaximization, LayoutUnit& growthLimitsWithoutMaximization) const;
     LayoutUnit computeUsedBreadthOfMinLength(const GridTrackSize&, LayoutUnit maxBreadth) const;
     LayoutUnit computeUsedBreadthOfMaxLength(const GridTrackSize&, LayoutUnit usedBreadth, LayoutUnit maxBreadth) const;
     void resolveContentBasedTrackSizingFunctions(GridTrackSizingDirection, GridSizingData&) const;
 
     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;
     std::unique_ptr<OrderedTrackIndexSet> computeEmptyTracksForAutoRepeat(GridTrackSizingDirection) const;
 
     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*>&);
     void placeAutoMajorAxisItemsOnGrid(const Vector<LayoutBox*>&);
     void placeAutoMajorAxisItemOnGrid(LayoutBox&, std::pair<size_t, size_t>& autoPlacementCursor);
     GridTrackSizingDirection autoPlacementMajorAxisDirection() const;
     GridTrackSizingDirection autoPlacementMinorAxisDirection() const;
 
     void computeTrackSizesForIndefiniteSize(GridTrackSizingDirection, GridSizingData&, LayoutUnit& minIntrinsicSize, LayoutUnit& maxIntrinsicSize) const;
     LayoutUnit computeTrackBasedLogicalHeight(const GridSizingData&) const;
@@ skipping 70 matching lines @@
 
     size_t gridColumnCount() const;
     size_t gridRowCount() const;
 
     bool isOrthogonalChild(const LayoutBox&) const;
     GridTrackSizingDirection flowAwareDirectionForChild(const LayoutBox&, GridTrackSizingDirection) const;
 
     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 };
 
     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;
 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutGrid, isLayoutGrid());
 
 } // namespace blink
 
 #endif // LayoutGrid_h