[css-grid] Pass Grid as argument to more methods & remove m_gridIsDirty

[css-grid] Pass Grid as argument to more methods & remove m_gridIsDirty

This is the follow up of crrev.com/2536703002, three more methods get the
Grid passed as argument. The remaining methods will access the Grid through
GridSizingData, but that will be another patch.

Apart from that, m_gridIsDirty was removed because it was always too
confusing. It was replaced by Grid's m_needsItemsPlacement which is much
more concise. The dirtyGrid() call was indeed only forcing a replacement of
the grid items.

BUG=627812
Committed: https://crrev.com/b8f9db3a8e2aff0307f677c3b29efc0162a80b5f
Cr-Commit-Position: refs/heads/master@{#436910}

[svillar, 2016-11-30 16:10:35]

svillar@igalia.com changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, jfernandez@igalia.com,
rego@igalia.com

[eae, 2016-12-02 10:09:42]

LGTM

[eae, 2016-12-02 10:09:50]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-12-02 10:10:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Manuel Rego, 2016-12-02 10:21:38]

LGTM too. Just a question for future patches.

https://codereview.chromium.org/2541003003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.h (left):

https://codereview.chromium.org/2541003003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.h:421: Vector<LayoutUnit>
m_columnPositions;
Do you have plans to move these to Grid too?

We talked that we should clear this information too at some point.

[svillar, 2016-12-02 10:56:34]

On 2016/12/02 10:21:38, Manuel Rego wrote:
> LGTM too. Just a question for future patches.
> 
>
https://codereview.chromium.org/2541003003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/LayoutGrid.h (left):
> 
>
https://codereview.chromium.org/2541003003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/LayoutGrid.h:421: Vector<LayoutUnit>
> m_columnPositions;
> Do you have plans to move these to Grid too?
> 
> We talked that we should clear this information too at some point.

Not in the short term because we need to keep them at least for the computed
style.

[commit-bot: I haz the power, 2016-12-02 12:14:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 12:14:09]

Failed to apply patch for third_party/WebKit/Source/core/layout/LayoutGrid.cpp:
While running git apply --index -p1;
  error: patch failed: third_party/WebKit/Source/core/layout/LayoutGrid.cpp:461
  error: third_party/WebKit/Source/core/layout/LayoutGrid.cpp: patch does not
apply

Patch:       third_party/WebKit/Source/core/layout/LayoutGrid.cpp
Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
index
b0722d3929e58847e224d113b2df1dbf21c062e1..1725427dffc0e20da0a3760ca66f83942c30f85b
100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -163,7 +163,14 @@ void LayoutGrid::Grid::setHasAnyOrthogonalGridItem(
   m_hasAnyOrthogonalGridItem = hasAnyOrthogonalGridItem;
 }
 
-void LayoutGrid::Grid::clear() {
+void LayoutGrid::Grid::setNeedsItemsPlacement(bool needsItemsPlacement) {
+  m_needsItemsPlacement = needsItemsPlacement;
+
+  if (!needsItemsPlacement) {
+    m_grid.shrinkToFit();
+    return;
+  }
+
   m_grid.resize(0);
   m_gridItemArea.clear();
   m_gridItemsIndexesMap.clear();
@@ -461,8 +468,7 @@ struct GridItemsSpanGroupRange {
   Vector<GridItemWithSpan>::iterator rangeEnd;
 };
 
-LayoutGrid::LayoutGrid(Element* element)
-    : LayoutBlock(element), m_grid(this), m_gridIsDirty(true) {
+LayoutGrid::LayoutGrid(Element* element) : LayoutBlock(element), m_grid(this) {
   ASSERT(!childrenInline());
 }
 
@@ -591,7 +597,8 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) {
 
   // We cannot perform a simplifiedLayout() on a dirty grid that
   // has positioned items to be laid out.
-  if (!relayoutChildren && (!m_gridIsDirty || !posChildNeedsLayout()) &&
+  if (!relayoutChildren &&
+      (!m_grid.needsItemsPlacement() || !posChildNeedsLayout()) &&
       simplifiedLayout())
     return;
 
@@ -634,7 +641,8 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) {
       dirtyGrid();
     placeItemsOnGrid(m_grid, TrackSizing);
 
-    GridSizingData sizingData(numTracks(ForColumns), numTracks(ForRows));
+    GridSizingData sizingData(numTracks(ForColumns, m_grid),
+                              numTracks(ForRows, m_grid));
 
     // 1- First, the track sizing algorithm is used to resolve the sizes of the
     // grid columns.
@@ -793,7 +801,8 @@ void LayoutGrid::computeIntrinsicLogicalWidths(
   const_cast<LayoutGrid*>(this)->placeItemsOnGrid(const_cast<Grid&>(m_grid),
                                                   IntrinsicSizeComputation);
 
-  GridSizingData sizingData(numTracks(ForColumns), numTracks(ForRows));
+  GridSizingData sizingData(numTracks(ForColumns, m_grid),
+                            numTracks(ForRows, m_grid));
   computeTrackSizesForIndefiniteSize(ForColumns, sizingData, minLogicalWidth,
                                      maxLogicalWidth);
 
@@ -2010,6 +2019,7 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(
 
 std::unique_ptr<LayoutGrid::OrderedTrackIndexSet>
 LayoutGrid::computeEmptyTracksForAutoRepeat(
+    Grid& grid,
     GridTrackSizingDirection direction) const {
   bool isRowAxis = direction == ForColumns;
   if ((isRowAxis && styleRef().gridAutoRepeatColumnsType() != AutoFit) ||
@@ -2021,9 +2031,9 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
                               ?
styleRef().gridAutoRepeatColumnsInsertionPoint()
                               : styleRef().gridAutoRepeatRowsInsertionPoint();
   size_t firstAutoRepeatTrack =
-      insertionPoint + std::abs(m_grid.smallestTrackStart(direction));
+      insertionPoint + std::abs(grid.smallestTrackStart(direction));
   size_t lastAutoRepeatTrack =
-      firstAutoRepeatTrack + autoRepeatCountForDirection(direction);
+      firstAutoRepeatTrack + grid.autoRepeatTracks(direction);
 
   if (!m_grid.hasGridItems()) {
     emptyTrackIndexes = wrapUnique(new OrderedTrackIndexSet);
@@ -2033,7 +2043,7 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
   } else {
     for (size_t trackIndex = firstAutoRepeatTrack;
          trackIndex < lastAutoRepeatTrack; ++trackIndex) {
-      GridIterator iterator(m_grid, direction, trackIndex);
+      GridIterator iterator(grid, direction, trackIndex);
       if (!iterator.nextGridItem()) {
         if (!emptyTrackIndexes)
           emptyTrackIndexes = wrapUnique(new OrderedTrackIndexSet);
@@ -2046,7 +2056,7 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
 
 void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid& grid,
                                   SizingOperation sizingOperation) {
-  if (!m_gridIsDirty)
+  if (!grid.needsItemsPlacement())
     return;
 
   DCHECK(!grid.hasGridItems());
@@ -2064,16 +2074,13 @@ void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid&
grid,
 
   populateExplicitGridAndOrderIterator(grid);
 
-  // We clear the dirty bit here as the grid sizes have been updated.
-  m_gridIsDirty = false;
-  bool hasAnyOrthogonalGridItem = false;
-
   Vector<LayoutBox*> autoMajorAxisAutoGridItems;
   Vector<LayoutBox*> specifiedMajorAxisAutoGridItems;
 #if ENABLE(ASSERT)
   DCHECK(!grid.hasAnyGridItemPaintOrder());
 #endif
   DCHECK(!grid.hasAnyOrthogonalGridItem());
+  bool hasAnyOrthogonalGridItem = false;
   size_t childIndex = 0;
   for (LayoutBox* child = grid.orderIterator().first(); child;
        child = grid.orderIterator().next()) {
@@ -2103,7 +2110,7 @@ void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid& grid,
     }
     grid.insert(*child, area);
   }
-  m_grid.setHasAnyOrthogonalGridItem(hasAnyOrthogonalGridItem);
+  grid.setHasAnyOrthogonalGridItem(hasAnyOrthogonalGridItem);
 
 #if ENABLE(ASSERT)
   if (grid.hasGridItems()) {
@@ -2119,11 +2126,12 @@ void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid&
grid,
   placeSpecifiedMajorAxisItemsOnGrid(grid, specifiedMajorAxisAutoGridItems);
   placeAutoMajorAxisItemsOnGrid(grid, autoMajorAxisAutoGridItems);
 
-  grid.shrinkToFit();
-
   // Compute collapsable tracks for auto-fit.
-  grid.setAutoRepeatEmptyColumns(computeEmptyTracksForAutoRepeat(ForColumns));
-  grid.setAutoRepeatEmptyRows(computeEmptyTracksForAutoRepeat(ForRows));
+  grid.setAutoRepeatEmptyColumns(
+      computeEmptyTracksForAutoRepeat(grid, ForColumns));
+  grid.setAutoRepeatEmptyRows(computeEmptyTracksForAutoRepeat(grid, ForRows));
+
+  grid.setNeedsItemsPlacement(false);
 
 #if ENABLE(ASSERT)
   for (LayoutBox* child = grid.orderIterator().first(); child;
@@ -2199,12 +2207,13 @@ void
LayoutGrid::populateExplicitGridAndOrderIterator(Grid& grid) const {
 
 std::unique_ptr<GridArea>
 LayoutGrid::createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(
+    const Grid& grid,
     const LayoutBox& gridItem,
     GridTrackSizingDirection specifiedDirection,
     const GridSpan& specifiedPositions) const {
   GridTrackSizingDirection crossDirection =
       specifiedDirection == ForColumns ? ForRows : ForColumns;
-  const size_t endOfCrossDirection = m_grid.numTracks(crossDirection);
+  const size_t endOfCrossDirection = grid.numTracks(crossDirection);
   size_t crossDirectionSpanSize =
       GridPositionsResolver::spanSizeForAutoPlacedItem(*style(), gridItem,
                                                        crossDirection);
@@ -2247,9 +2256,11 @@ void LayoutGrid::placeSpecifiedMajorAxisItemsOnGrid(
                             : minorAxisCursors.get(majorAxisInitialPosition));
     std::unique_ptr<GridArea> emptyGridArea = iterator.nextEmptyGridArea(
         majorAxisPositions.integerSpan(), minorAxisSpanSize);
-    if (!emptyGridArea)
+    if (!emptyGridArea) {
       emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(
-          *autoGridItem, autoPlacementMajorAxisDirection(),
majorAxisPositions);
+          grid, *autoGridItem, autoPlacementMajorAxisDirection(),
+          majorAxisPositions);
+    }
 
     grid.insert(*autoGridItem, *emptyGridArea);
 
@@ -2314,9 +2325,11 @@ void LayoutGrid::placeAutoMajorAxisItemOnGrid(
           minorAxisPositions.integerSpan(), majorAxisSpanSize);
     }
 
-    if (!emptyGridArea)
+    if (!emptyGridArea) {
       emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(
-          gridItem, autoPlacementMinorAxisDirection(), minorAxisPositions);
+          grid, gridItem, autoPlacementMinorAxisDirection(),
+          minorAxisPositions);
+    }
   } else {
     size_t minorAxisSpanSize =
GridPositionsResolver::spanSizeForAutoPlacedItem(
         *style(), gridItem, autoPlacementMinorAxisDirection());
@@ -2354,7 +2367,7 @@ void LayoutGrid::placeAutoMajorAxisItemOnGrid(
 
     if (!emptyGridArea)
       emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(
-          gridItem, autoPlacementMinorAxisDirection(),
+          grid, gridItem, autoPlacementMinorAxisDirection(),
           GridSpan::translatedDefiniteGridSpan(0, minorAxisSpanSize));
   }
 
@@ -2373,12 +2386,11 @@ GridTrackSizingDirection
LayoutGrid::autoPlacementMinorAxisDirection() const {
 }
 
 void LayoutGrid::dirtyGrid() {
-  if (m_gridIsDirty)
+  if (m_grid.needsItemsPlacement())
     return;
 
-  m_grid.clear();
+  m_grid.setNeedsItemsPlacement(true);
   m_gridItemsOverflowingGridArea.resize(0);
-  m_gridIsDirty = true;
 }
 
 Vector<LayoutUnit> LayoutGrid::trackSizesForComputedStyle(
@@ -2620,7 +2632,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(
                                             : child.style()->gridRowStart();
   GridPosition endPosition = isForColumns ? child.style()->gridColumnEnd()
                                           : child.style()->gridRowEnd();
-  int lastLine = numTracks(direction);
+  int lastLine = numTracks(direction, m_grid);
 
   bool startIsAuto =
       startPosition.isAuto() ||
@@ -3565,7 +3577,8 @@ bool LayoutGrid::cachedHasDefiniteLogicalHeight() const {
   return m_hasDefiniteLogicalHeight.value();
 }
 
-size_t LayoutGrid::numTracks(GridTrackSizingDirection direction) const {
+size_t LayoutGrid::numTracks(GridTrackSizingDirection direc…
(message too large)

[svillar, 2016-12-07 09:24:32]

The CQ bit was checked by svillar@igalia.com

[svillar, 2016-12-07 09:24:32]

The patchset sent to the CQ was uploaded after l-g-t-m from rego@igalia.com,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2541003003/#ps20001
(title: "Patch for landing")

[commit-bot: I haz the power, 2016-12-07 09:24:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 10:47:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481102672312990,
"parent_rev": "e24e754ca49b19e2396c01d2d9476deb64d9f45f", "commit_rev":
"456b0fdcc08f5b67a3d5ab1380004e1e6a5b016b"}

[commit-bot: I haz the power, 2016-12-07 10:48:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-07 10:51:37]

Description was changed from

==========
[css-grid] Pass Grid as argument to more methods & remove m_gridIsDirty

This is the follow up of crrev.com/2536703002, three more methods get the
Grid passed as argument. The remaining methods will access the Grid through
GridSizingData, but that will be another patch.

Apart from that, m_gridIsDirty was removed because it was always too
confusing. It was replaced by Grid's m_needsItemsPlacement which is much
more concise. The dirtyGrid() call was indeed only forcing a replacement of
the grid items.

BUG=627812
==========

to

==========
[css-grid] Pass Grid as argument to more methods & remove m_gridIsDirty

This is the follow up of crrev.com/2536703002, three more methods get the
Grid passed as argument. The remaining methods will access the Grid through
GridSizingData, but that will be another patch.

Apart from that, m_gridIsDirty was removed because it was always too
confusing. It was replaced by Grid's m_needsItemsPlacement which is much
more concise. The dirtyGrid() call was indeed only forcing a replacement of
the grid items.

BUG=627812

Committed: https://crrev.com/b8f9db3a8e2aff0307f677c3b29efc0162a80b5f
Cr-Commit-Position: refs/heads/master@{#436910}
==========

[commit-bot: I haz the power, 2016-12-07 10:51:38]

Patchset 2 (id:??) landed as
https://crrev.com/b8f9db3a8e2aff0307f677c3b29efc0162a80b5f
Cr-Commit-Position: refs/heads/master@{#436910}