Reviewers: cbiesinger, Manuel Rego
CL:
https://codereview.chromium.org/2263213002/Message:
Adding reviewers who checked the original patch
Description:
[css-grid] Do not recursively call layout during auto repeat computation
The computation of auto repeat tracks was incorrectly recursively triggering
a layout in order to compute the available size. That was happening whenever
the width was indefinite. In such cases we should treat the width always as
indefinite without having to run any extra code. During the layout phase
we'll have the actual width available.
This partially reverts commit 49bc5ba13c492f09971882e2cb842e3514d19922 which
was a quick fix for a security issue but that was not actually fixing the
real problem behind.
BUG=633474
Base URL:
https://chromium.googlesource.com/chromium/src.git@masterAffected files (+30, -22 lines):
M third_party/WebKit/Source/core/layout/LayoutGrid.h
M 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 784ab711f8449cd0e337ac60d7f8090aba6c38b0..2f0dccccb881997d688f2c3479d3455045263d40 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -441,10 +441,9 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
// TODO(svillar): we won't need to do this once the intrinsic width computation is isolated
// from the LayoutGrid object state (it should not touch any attribute) (see
crbug.com/627812)
- size_t autoRepeatColumnsCount = computeAutoRepeatTracksCount(ForColumns);
- if (m_autoRepeatColumns && m_autoRepeatColumns != autoRepeatColumnsCount)
+ if (m_autoRepeatColumns && m_autoRepeatColumns != computeAutoRepeatTracksCount(ForColumns, TrackSizing))
dirtyGrid();
- placeItemsOnGrid(autoRepeatColumnsCount);
+ placeItemsOnGrid(TrackSizing);
GridSizingData sizingData(gridColumnCount(), gridRowCount());
@@ -575,7 +574,7 @@ LayoutUnit LayoutGrid::guttersSize(GridTrackSizingDirection direction, size_t st
void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
{
- const_cast<LayoutGrid*>(this)->placeItemsOnGrid(styleRef().gridAutoRepeatColumns().size());
+ const_cast<LayoutGrid*>(this)->placeItemsOnGrid(IntrinsicSizeComputation);
GridSizingData sizingData(gridColumnCount(), gridRowCount());
sizingData.freeSpaceForDirection(ForColumns) = LayoutUnit();
@@ -1384,7 +1383,7 @@ void LayoutGrid::insertItemIntoGrid(LayoutBox& child, const GridArea& area)
}
}
-size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direction) const
+size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direction, SizingOperation sizingOperation) const
{
bool isRowAxis = direction == ForColumns;
const auto& autoRepeatTracks = isRowAxis ? styleRef().gridAutoRepeatColumns() : styleRef().gridAutoRepeatRows();
@@ -1393,18 +1392,23 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
if (!autoRepeatTrackListLength)
return 0;
- LayoutUnit availableSize = isRowAxis ? availableLogicalWidth() : computeContentLogicalHeight(MainOrPreferredSize, styleRef().logicalHeight(), LayoutUnit(-1));
- if (availableSize == -1) {
- const Length& maxLength = isRowAxis ? styleRef().logicalMaxWidth() : styleRef().logicalMaxHeight();
- if (!maxLength.isMaxSizeNone()) {
- availableSize = isRowAxis
- ? computeLogicalWidthUsing(MaxSize, maxLength, containingBlockLogicalWidthForContent(), containingBlock())
- : computeContentLogicalHeight(MaxSize, maxLength, LayoutUnit(-1));
+ LayoutUnit availableSize(-1);
+ // Widths are always definite except during intrinsic size computation. For heights we need to
+ // check whether the computed logical height is -1 or not to determine it.
+ if (sizingOperation != IntrinsicSizeComputation || !isRowAxis) {
+ if (isRowAxis) {
+ DCHECK_NE(sizingOperation, IntrinsicSizeComputation);
+ availableSize = availableLogicalWidth();
+ } else {
+ availableSize = computeContentLogicalHeight(MainOrPreferredSize, styleRef().logicalHeight(), LayoutUnit(-1));
+ if (availableSize == -1) {
+ const Length& maxLength = styleRef().logicalMaxHeight();
+ if (!maxLength.isMaxSizeNone())
+ availableSize = computeContentLogicalHeight(MaxSize, maxLength, LayoutUnit(-1));
+ } else {
+ availableSize = constrainLogicalHeightByMinMax(availableSize, LayoutUnit(-1));
+ }
}
- } else {
- availableSize = isRowAxis
- ? constrainLogicalWidthByMinMax(availableSize, availableLogicalWidth(), containingBlock())
- : constrainLogicalHeightByMinMax(availableSize, LayoutUnit(-1));
}
bool needsToFulfillMinimumSize = false;
@@ -1491,15 +1495,19 @@ std::unique_ptr<LayoutGrid::OrderedTrackIndexSet> LayoutGrid::computeEmptyTracks
return emptyTrackIndexes;
}
-void LayoutGrid::placeItemsOnGrid(size_t autoRepeatColumnsCount)
+void LayoutGrid::placeItemsOnGrid(SizingOperation sizingOperation)
{
if (!m_gridIsDirty)
return;
- ASSERT(m_gridItemArea.isEmpty());
+ DCHECK(m_gridItemArea.isEmpty());
+ DCHECK(m_gridItemsIndexesMap.isEmpty());
- m_autoRepeatColumns = autoRepeatColumnsCount;
- m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows);
+ if (sizingOperation == IntrinsicSizeComputation)
+ m_autoRepeatColumns = styleRef().gridAutoRepeatColumns().size();
+ else
+ m_autoRepeatColumns = computeAutoRepeatTracksCount(ForColumns, sizingOperation);
+ m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows, sizingOperation);
populateExplicitGridAndOrderIterator();
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 90ebefe8bdd5a178db2ee9dd9e602e58e430ffc2..8588ff69fb9280733ec1e662c407ea85b416edb4 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.h
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.h
@@ -124,7 +124,7 @@ private:
void ensureGridSize(size_t maximumRowSize, size_t maximumColumnSize);
void insertItemIntoGrid(LayoutBox&, const GridArea&);
- size_t computeAutoRepeatTracksCount(GridTrackSizingDirection) const;
+ size_t computeAutoRepeatTracksCount(GridTrackSizingDirection, SizingOperation) const;
typedef ListHashSet<size_t> OrderedTrackIndexSet;
std::unique_ptr<OrderedTrackIndexSet> computeEmptyTracksForAutoRepeat(GridTrackSizingDirection) const;
@@ -132,7 +132,7 @@ private:
bool hasAutoRepeatEmptyTracks(GridTrackSizingDirection) const;
bool isEmptyAutoRepeatTrack(GridTrackSizingDirection, size_t lineNumber) const;
- void placeItemsOnGrid(size_t autoRepeatColumnsCount);
+ void placeItemsOnGrid(SizingOperation);
void populateExplicitGridAndOrderIterator();
std::unique_ptr<GridArea> createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(const LayoutBox&, GridTrackSizingDirection, const GridSpan& specifiedPositions) const;
void placeSpecifiedMajorAxisItemsOnGrid(const Vector<LayoutBox*>&);