[css-grid] Convert GridRepresentation into an actual class

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 57 matching lines @@
   }
 
   const Vector<LayoutUnit>& rowPositions() const {
     ASSERT(!m_gridIsDirty);
     return m_rowPositions;
   }
 
   typedef Vector<LayoutBox*, 1> GridCell;
   const GridCell& gridCell(int row, int column) const {
     SECURITY_DCHECK(!m_gridIsDirty);
-    return m_grid[row][column];
+    return m_grid.cell(row, column);
   }
 
   const Vector<LayoutBox*>& itemsOverflowingGridArea() const {
     SECURITY_DCHECK(!m_gridIsDirty);
     return m_gridItemsOverflowingGridArea;
   }
 
   int paintIndexForGridItem(const LayoutBox* layoutBox) const {
     SECURITY_DCHECK(!m_gridIsDirty);
     return m_gridItemsIndexesMap.get(layoutBox);
@@ skipping 43 matching lines @@
       Vector<LayoutUnit>& increments,
       LayoutUnit& totalGrowth) 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);
   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,
@@ skipping 185 matching lines @@
 
   bool isOrthogonalChild(const LayoutBox&) const;
   GridTrackSizingDirection flowAwareDirectionForChild(
       const LayoutBox&,
       GridTrackSizingDirection) const;
 
   bool cachedHasDefiniteLogicalHeight() const;
 
   size_t numTracks(GridTrackSizingDirection) const;
 
-  typedef Vector<Vector<GridCell>> GridRepresentation;
-  GridRepresentation m_grid;
+  // TODO(svillar): move into this class once GridIterator is added.
+  typedef Vector<Vector<GridCell>> GridAsVector;

[Manuel Rego, 2016/11/22 10:07:17]

Or GridAsMatrix?

[svillar, 2016/11/22 10:15:18]

If you don't feel strongly about it I'd prefer to keep it as it is, mainly in
order to avoid a lot of refactorings just because this name.

[Manuel Rego, 2016/11/22 10:38:01]

I'd prefer to change it if possible.

+  class Grid final {
+   public:
+    Grid() {}
+
+    size_t numColumns() const { return m_grid.size() ? m_grid[0].size() : 0; }
+    size_t numRows() const { return m_grid.size(); }
+
+    void ensureGridSize(size_t maximumRowSize, size_t maximumColumnSize);
+    void insert(LayoutBox&, const GridArea&);
+
+    const GridCell& cell(size_t row, size_t column) const {
+      return m_grid[row][column];
+    }
+
+    void shrinkToFit() { m_grid.shrinkToFit(); }
+
+    void clear();

[Manuel Rego, 2016/11/22 10:07:17]

Nit: Why this is not defined here?

It seems strange a few methods are written only here
and this one is not (when it's just one line).

[svillar, 2016/11/22 10:15:18]

I don't think it's strange. Don't know if this is still valid in Blink, but in
WebKit is a quite common idiom. The rationale is that oneliners keep the class
definition pretty clean. If you start to add implementations to the class
definition then everything becomes a mess, and the API of the class is not that
easy to identify.

[Manuel Rego, 2016/11/22 10:38:01]

It's strange to me compared to shrinkToFit() for example. :-)
But I don't care it was just a picky comment.

+
+   private:
+    friend class GridIterator;
+    GridAsVector m_grid;

[Manuel Rego, 2016/11/22 10:07:17]

Nit: I think it's kind of confusing to have 2 "m_grid"
even if they're in separated classes.

[svillar, 2016/11/22 10:15:18]

I agree but it's a private method, so there should not be any issue associated
to that, i.e. clients of this class are not aware of the existence of this
attribute.

[Manuel Rego, 2016/11/22 10:38:01]

I know but still, when you see the LayoutGrid code you'll see
2 m_grid in different places. If you're eventually moving this
to a different file, I'm fine with keeping it. Otherwise,
I'd rename it.

+  };
+  Grid m_grid;
+
   bool m_gridIsDirty;
   Vector<LayoutUnit> m_rowPositions;
   Vector<LayoutUnit> m_columnPositions;
   LayoutUnit m_offsetBetweenColumns;
   LayoutUnit m_offsetBetweenRows;
   HashMap<const LayoutBox*, GridArea> m_gridItemArea;
   OrderIterator m_orderIterator;
   Vector<LayoutBox*> m_gridItemsOverflowingGridArea;
   HashMap<const LayoutBox*, size_t> m_gridItemsIndexesMap;
 
@@ skipping 12 matching lines @@
   std::unique_ptr<OrderedTrackIndexSet> m_autoRepeatEmptyRows{nullptr};
 
   Optional<bool> m_hasDefiniteLogicalHeight;
 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutGrid, isLayoutGrid());
 
 }  // namespace blink
 
 #endif  // LayoutGrid_h