[css-grid] Clearing the override height before layout

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

 /*
  * Copyright (C) 2011 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 215 matching lines @@
 
   SubtreeLayoutScope layout_scope(*this);
 
   {
     // LayoutState needs this deliberate scope to pop before updating scroll
     // information (which may trigger relayout).
     LayoutState state(*this);
 
     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();
     has_definite_logical_height_ = HasDefiniteLogicalHeight();
 
     TextAutosizer::LayoutScope text_autosizer_layout_scope(this, &layout_scope);
 
     PlaceItemsOnGrid(grid_, kTrackSizing);
 
     // 1- First, the track sizing algorithm is used to resolve the sizes of the
@@ skipping 1261 matching lines @@
     const LayoutBox& child,
     GridTrackSizingDirection direction) const {
   return !IsOrthogonalChild(child)
              ? direction
              : (direction == kForColumns ? kForRows : kForColumns);
 }
 
 // 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;
   bool allowed_to_stretch_child_block_size =
       block_flow_is_column_axis ? AllowedToStretchChildAlongColumnAxis(child)
                                 : AllowedToStretchChildAlongRowAxis(child);
   if (allowed_to_stretch_child_block_size) {
     LayoutUnit stretched_logical_height =
         AvailableAlignmentSpaceForChildBeforeStretching(
             OverrideContainingBlockContentSizeForChild(child,
@@ skipping 812 matching lines @@
   if (direction == kForRows)
     return grid.NumTracks(kForRows);
 
   return grid.NumTracks(kForRows)
              ? grid.NumTracks(kForColumns)
              : GridPositionsResolver::ExplicitGridColumnCount(
                    StyleRef(), grid.AutoRepeatTracks(kForColumns));
 }
 
 }  // namespace blink