[css-grid] Refactor GridSpan to avoid pointers

third_party/WebKit/Source/core/style/GridCoordinate.h

Index: third_party/WebKit/Source/core/style/GridCoordinate.h
diff --git a/third_party/WebKit/Source/core/style/GridCoordinate.h b/third_party/WebKit/Source/core/style/GridCoordinate.h
index 3a1e5b62cf4d986fe31f295da21fc936c0034400..1bf698258b48dbb97b1a0e97d85d84d18350ac0b 100644
--- a/third_party/WebKit/Source/core/style/GridCoordinate.h
+++ b/third_party/WebKit/Source/core/style/GridCoordinate.h
@@ -49,37 +49,45 @@ const size_t kGridMaxTracks = 1000000;
 struct GridSpan {
     USING_FAST_MALLOC(GridSpan);
 public:
-    static PassOwnPtr<GridSpan> create(const GridResolvedPosition& resolvedInitialPosition, const GridResolvedPosition& resolvedFinalPosition)
+
+    static GridSpan definiteGridSpan(const GridResolvedPosition& resolvedInitialPosition, const GridResolvedPosition& resolvedFinalPosition)
+    {
+        return GridSpan(resolvedInitialPosition, resolvedFinalPosition);
+    }
+
+    static GridSpan indefiniteGridSpan()
     {
-        return adoptPtr(new GridSpan(resolvedInitialPosition, resolvedFinalPosition));
+        GridSpan span = GridSpan(0, 1);
+        span.m_definite = false;
+        return span;
     }
 
-    static PassOwnPtr<GridSpan> createWithSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, GridPositionSide side)
+    static GridSpan definiteGridSpanWithSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, GridPositionSide side)
     {
         size_t positionOffset = position.spanPosition();
         if (side == ColumnStartSide || side == RowStartSide) {
             if (resolvedOppositePosition == 0)
-                return GridSpan::create(resolvedOppositePosition, resolvedOppositePosition.next());
+                return GridSpan::definiteGridSpan(resolvedOppositePosition, resolvedOppositePosition.next());
 
             GridResolvedPosition initialResolvedPosition = GridResolvedPosition(std::max<int>(0, resolvedOppositePosition.toInt() - positionOffset));
-            return GridSpan::create(initialResolvedPosition, resolvedOppositePosition);
+            return GridSpan::definiteGridSpan(initialResolvedPosition, resolvedOppositePosition);
         }
 
-        return GridSpan::create(resolvedOppositePosition, GridResolvedPosition(resolvedOppositePosition.toInt() + positionOffset));
+        return GridSpan::definiteGridSpan(resolvedOppositePosition, GridResolvedPosition(resolvedOppositePosition.toInt() + positionOffset));
     }
 
-    static PassOwnPtr<GridSpan> createWithNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, GridPositionSide side, const Vector<size_t>& gridLines)
+    static GridSpan definiteGridSpanWithNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, GridPositionSide side, const Vector<size_t>& gridLines)
     {
         if (side == RowStartSide || side == ColumnStartSide)
-            return createWithInitialNamedSpanAgainstOpposite(resolvedOppositePosition, position, gridLines);
+            return definiteGridSpanWithInitialNamedSpanAgainstOpposite(resolvedOppositePosition, position, gridLines);
 
-        return createWithFinalNamedSpanAgainstOpposite(resolvedOppositePosition, position, gridLines);
+        return definiteGridSpanWithFinalNamedSpanAgainstOpposite(resolvedOppositePosition, position, gridLines);
     }
 
-    static PassOwnPtr<GridSpan> createWithInitialNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, const Vector<size_t>& gridLines)
+    static GridSpan definiteGridSpanWithInitialNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, const Vector<size_t>& gridLines)
     {
         if (resolvedOppositePosition == 0)
-            return GridSpan::create(resolvedOppositePosition, resolvedOppositePosition.next());
+            return GridSpan::definiteGridSpan(resolvedOppositePosition, resolvedOppositePosition.next());
 
         size_t firstLineBeforeOppositePositionIndex = 0;
         const size_t* firstLineBeforeOppositePosition = std::lower_bound(gridLines.begin(), gridLines.end(), resolvedOppositePosition.toInt());
@@ -89,10 +97,10 @@ public:
         GridResolvedPosition resolvedGridLinePosition = GridResolvedPosition(gridLines[gridLineIndex]);
         if (resolvedGridLinePosition >= resolvedOppositePosition)
             resolvedGridLinePosition = resolvedOppositePosition.prev();
-        return GridSpan::create(resolvedGridLinePosition, resolvedOppositePosition);
+        return GridSpan::definiteGridSpan(resolvedGridLinePosition, resolvedOppositePosition);
     }
 
-    static PassOwnPtr<GridSpan> createWithFinalNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, const Vector<size_t>& gridLines)
+    static GridSpan definiteGridSpanWithFinalNamedSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, const Vector<size_t>& gridLines)
     {
         size_t firstLineAfterOppositePositionIndex = gridLines.size() - 1;
         const size_t* firstLineAfterOppositePosition = std::upper_bound(gridLines.begin(), gridLines.end(), resolvedOppositePosition.toInt());
@@ -103,40 +111,65 @@ public:
         if (resolvedGridLinePosition <= resolvedOppositePosition)
             resolvedGridLinePosition = resolvedOppositePosition.next();
 
-        return GridSpan::create(resolvedOppositePosition, resolvedGridLinePosition);
+        return GridSpan::definiteGridSpan(resolvedOppositePosition, resolvedGridLinePosition);
     }
 
-    GridSpan(const GridResolvedPosition& resolvedInitialPosition, const GridResolvedPosition& resolvedFinalPosition)
-        : resolvedInitialPosition(std::min(resolvedInitialPosition.toInt(), kGridMaxTracks - 1))
-        , resolvedFinalPosition(std::min(resolvedFinalPosition.toInt(), kGridMaxTracks))
+    bool operator==(const GridSpan& o) const
     {
-        ASSERT(resolvedInitialPosition < resolvedFinalPosition);
+        ASSERT(isDefinite());

[cbiesinger, 2015/11/19 22:22:08]

Seems slightly weird that you can't compare a definite span against an
indefinite one (returning false), why?

[Manuel Rego, 2015/11/20 08:53:45]

We don't need it right now, but I agree that it might be nice to allow it
(it might be needed in the future).
So I've modified the method accordingly.

+        return m_resolvedInitialPosition == o.m_resolvedInitialPosition && m_resolvedFinalPosition == o.m_resolvedFinalPosition;
     }
 
-    bool operator==(const GridSpan& o) const
+    size_t integerSpan() const
     {
-        return resolvedInitialPosition == o.resolvedInitialPosition && resolvedFinalPosition == o.resolvedFinalPosition;
+        ASSERT(isDefinite());
+        return m_resolvedFinalPosition.toInt() - m_resolvedInitialPosition.toInt();
     }
 
-    size_t integerSpan() const
+    const GridResolvedPosition& resolvedInitialPosition() const
     {
-        return resolvedFinalPosition.toInt() - resolvedInitialPosition.toInt();
+        ASSERT(isDefinite());
+        return m_resolvedInitialPosition;
     }
 
-    GridResolvedPosition resolvedInitialPosition;
-    GridResolvedPosition resolvedFinalPosition;
+    const GridResolvedPosition& resolvedFinalPosition() const
+    {
+        ASSERT(isDefinite());
+        return m_resolvedFinalPosition;
+    }
 
     typedef GridResolvedPosition iterator;
 
     iterator begin() const
     {
-        return resolvedInitialPosition;
+        ASSERT(isDefinite());
+        return m_resolvedInitialPosition;
     }
 
     iterator end() const
     {
-        return resolvedFinalPosition;
+        ASSERT(isDefinite());
+        return m_resolvedFinalPosition;
+    }
+
+    bool isDefinite() const
+    {
+        return m_definite;
     }
+
+private:
+
+    GridSpan(const GridResolvedPosition& resolvedInitialPosition, const GridResolvedPosition& resolvedFinalPosition)
+        : m_resolvedInitialPosition(std::min(resolvedInitialPosition.toInt(), kGridMaxTracks - 1))
+        , m_resolvedFinalPosition(std::min(resolvedFinalPosition.toInt(), kGridMaxTracks))

[cbiesinger, 2015/11/19 22:22:08]

Why didn't you also initialize m_definite in the constructor list here?

[Manuel Rego, 2015/11/20 08:53:45]

Ups, I forgot to do it there. Fixed now. :-)

+    {
+        ASSERT(resolvedInitialPosition < resolvedFinalPosition);
+        m_definite = true;
+    }
+
+    GridResolvedPosition m_resolvedInitialPosition;
+    GridResolvedPosition m_resolvedFinalPosition;
+    bool m_definite;
 };
 
 // This represents a grid area that spans in both rows' and columns' direction.
@@ -145,8 +178,8 @@ struct GridCoordinate {
 public:
     // HashMap requires a default constuctor.
     GridCoordinate()
-        : columns(0, 1)
-        , rows(0, 1)
+        : columns(GridSpan::indefiniteGridSpan())
+        , rows(GridSpan::indefiniteGridSpan())
     {
     }
 
@@ -170,13 +203,13 @@ public:
     {
         switch (side) {
         case ColumnStartSide:
-            return columns.resolvedInitialPosition;
+            return columns.resolvedInitialPosition();
         case ColumnEndSide:
-            return columns.resolvedFinalPosition;
+            return columns.resolvedFinalPosition();
         case RowStartSide:
-            return rows.resolvedInitialPosition;
+            return rows.resolvedInitialPosition();
         case RowEndSide:
-            return rows.resolvedFinalPosition;
+            return rows.resolvedFinalPosition();
         }
         ASSERT_NOT_REACHED();
         return 0;