[css-grid] Convert GridRepresentation into an actual class

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 4e72a4839fee9556a87fcac4d55a97dd80e31c06..f08c5a57aac9b0b9017c5b5b5cb9329b566fbae3 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.h
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.h
@@ -75,7 +75,7 @@ class LayoutGrid final : public LayoutBlock {
   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 {
@@ -139,9 +139,6 @@ class LayoutGrid final : public LayoutBlock {
   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;
@@ -347,8 +344,32 @@ class LayoutGrid final : public LayoutBlock {
 
   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;