[css-grid] Clearing the override height before layout

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 957ecad4da47dc281d667cbd7d469519da0556cd..c146a760a92ae593ed8980d18df56f30d8b2446f 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -233,19 +233,22 @@ void LayoutGrid::UpdateBlockLayout(bool relayout_children) {
 
     LayoutSize previous_size = Size();
 
-    // We need to clear both own and containingBlock override sizes to
-    // ensure we get the same result when grid's intrinsic size is
-    // computed again in the updateLogicalWidth call bellow.
-    if (SizesLogicalWidthToFitContent(StyleRef().LogicalWidth()) ||
-        StyleRef().LogicalWidth().IsIntrinsicOrAuto()) {
-      for (auto* child = FirstInFlowChildBox(); child;
-           child = child->NextInFlowSiblingBox()) {
-        if (!IsOrthogonalChild(*child))
-          continue;
-        child->ClearOverrideSize();
-        child->ClearContainingBlockOverrideSize();
-        child->ForceLayout();
-      }
+    // Grid's layout logic controls the grid item's override size, so
+    // we need to clear any override size set previously, so it doesn't
+    // interfere in current layout execution.

[Manuel Rego, 2017/04/28 09:51:16]

Thanks for the new comment!

Nit: the two "so" sound weird to me.

+    for (auto* child = FirstInFlowChildBox(); child;
+         child = child->NextInFlowSiblingBox()) {
+      child->ClearOverrideSize();
+      if (!IsOrthogonalChild(*child) ||
+          (!SizesLogicalWidthToFitContent(StyleRef().LogicalWidth()) &&
+           !StyleRef().LogicalWidth().IsIntrinsicOrAuto()))
+        continue;
+      // Additionally, we may need to clear containingBlock override sizes and
+      // force a layout of the grid items to ensure we get the same result when
+      // grid's intrinsic size is computed again in the updateLogicalWidth call
+      // bellow.
+      child->ClearContainingBlockOverrideSize();
+      child->ForceLayout();

[svillar, 2017/04/28 15:33:31]

I know this forced layout was already there in order to mimic the pre-layout
done in FrameView. The thing is that we no longer to that for grid items, so
perhaps it'd be enough to call SetNeedsLayout() but we can do it in a separate
patch.

     }
 
     UpdateLogicalWidth();
@@ -1527,11 +1530,6 @@ GridTrackSizingDirection LayoutGrid::FlowAwareDirectionForChild(
 // FIXME: This logic is shared by LayoutFlexibleBox, so it should be moved to
 // LayoutBox.
 void LayoutGrid::ApplyStretchAlignmentToChildIfNeeded(LayoutBox& child) {
-  // We clear height override values because we will decide now whether it's
-  // allowed or not, evaluating the conditions which might have changed since
-  // the old values were set.
-  child.ClearOverrideLogicalContentHeight();
-
   GridTrackSizingDirection child_block_direction =
       FlowAwareDirectionForChild(child, kForRows);
   bool block_flow_is_column_axis = child_block_direction == kForRows;