[css-grid] Refactor GridSpan to avoid pointers

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

 /*
  * Copyright (C) 2013 Google 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 31 matching lines @@
 
 // Recommended maximum size for both explicit and implicit grids.
 const size_t kGridMaxTracks = 1000000;
 
 // A span in a single direction (either rows or columns). Note that |resolvedInitialPosition|
 // and |resolvedFinalPosition| are grid lines' indexes.
 // Iterating over the span shouldn't include |resolvedFinalPosition| to be correct.
 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 adoptPtr(new GridSpan(resolvedInitialPosition, resolvedFinalPosition));
+        return GridSpan(resolvedInitialPosition, resolvedFinalPosition);
     }
 
-    static PassOwnPtr<GridSpan> createWithSpanAgainstOpposite(const GridResolvedPosition& resolvedOppositePosition, const GridPosition& position, GridPositionSide side)
+    static GridSpan indefiniteGridSpan()
+    {
+        GridSpan span = GridSpan(0, 1);
+        span.m_definite = false;
+        return span;
+    }
+
+    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());
         if (firstLineBeforeOppositePosition != gridLines.end())
             firstLineBeforeOppositePositionIndex = firstLineBeforeOppositePosition - gridLines.begin();
         size_t gridLineIndex = std::max<int>(0, firstLineBeforeOppositePositionIndex - position.spanPosition());
         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());
         if (firstLineAfterOppositePosition != gridLines.end())
             firstLineAfterOppositePositionIndex = firstLineAfterOppositePosition - gridLines.begin();
         size_t gridLineIndex = std::min(gridLines.size() - 1, firstLineAfterOppositePositionIndex + position.spanPosition() - 1);
         GridResolvedPosition resolvedGridLinePosition = gridLines[gridLineIndex];
         if (resolvedGridLinePosition <= resolvedOppositePosition)
             resolvedGridLinePosition = resolvedOppositePosition.next();
 
-        return GridSpan::create(resolvedOppositePosition, resolvedGridLinePosition);
-    }
-
-    GridSpan(const GridResolvedPosition& resolvedInitialPosition, const GridResolvedPosition& resolvedFinalPosition)
-        : resolvedInitialPosition(std::min(resolvedInitialPosition.toInt(), kGridMaxTracks - 1))
-        , resolvedFinalPosition(std::min(resolvedFinalPosition.toInt(), kGridMaxTracks))
-    {
-        ASSERT(resolvedInitialPosition < resolvedFinalPosition);
+        return GridSpan::definiteGridSpan(resolvedOppositePosition, resolvedGridLinePosition);
     }
 
     bool operator==(const GridSpan& o) const
     {
-        return resolvedInitialPosition == o.resolvedInitialPosition && resolvedFinalPosition == o.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;
     }
 
     size_t integerSpan() const
     {
-        return resolvedFinalPosition.toInt() - resolvedInitialPosition.toInt();
+        ASSERT(isDefinite());
+        return m_resolvedFinalPosition.toInt() - m_resolvedInitialPosition.toInt();
     }
 
-    GridResolvedPosition resolvedInitialPosition;
-    GridResolvedPosition resolvedFinalPosition;
+    const GridResolvedPosition& resolvedInitialPosition() const
+    {
+        ASSERT(isDefinite());
+        return m_resolvedInitialPosition;
+    }
+
+    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.
 struct GridCoordinate {
     USING_FAST_MALLOC(GridCoordinate);
 public:
     // HashMap requires a default constuctor.
     GridCoordinate()
-        : columns(0, 1)
-        , rows(0, 1)
+        : columns(GridSpan::indefiniteGridSpan())
+        , rows(GridSpan::indefiniteGridSpan())
     {
     }
 
     GridCoordinate(const GridSpan& r, const GridSpan& c)
         : columns(c)
         , rows(r)
     {
     }
 
     bool operator==(const GridCoordinate& o) const
     {
         return columns == o.columns && rows == o.rows;
     }
 
     bool operator!=(const GridCoordinate& o) const
     {
         return !(*this == o);
     }
 
     GridResolvedPosition positionForSide(GridPositionSide side) const
     {
         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;
     }
 
     GridSpan columns;
     GridSpan rows;
 };
 
 typedef HashMap<String, GridCoordinate> NamedGridAreaMap;
 
 } // namespace blink
 
 #endif // GridCoordinate_h