[css-grid] Check if baseline alignment affects grid areas sizing

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 58d131096b4f51b0708bb3f642378d9f7d7f7b91..50108410973a11870a834e0fd01493f1914de574 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -219,7 +219,8 @@ void LayoutGrid::RepeatTracksSizingIfNeeded(
   ComputeTrackSizesForDefiniteSize(kForColumns, available_space_for_columns);
   ComputeTrackSizesForDefiniteSize(kForRows, available_space_for_rows);
 
-  if (baseline_affect_intrinsic_height) {
+  if (baseline_affect_intrinsic_height &&
+      StyleRef().LogicalHeight().IsIntrinsicOrAuto()) {
     SetLogicalHeight(ComputeTrackBasedLogicalHeight() +
                      BorderAndPaddingLogicalHeight() +
                      ScrollbarLogicalHeight());
@@ -1862,24 +1863,36 @@ bool LayoutGrid::IsBaselineContextComputed(GridAxis baseline_axis) const {
              : !col_axis_alignment_context_.IsEmpty();
 }
 
+// TODO (lajava): Maybe refactoring these methods based on direction.

[svillar, 2017/05/25 07:26:27]

I think it's better to do the refactoring in this patch mainly because you have
said that the meaning of the functions have changed. I'd think about a new name
and refactor both functions.

[jfernandez, 2017/05/30 10:39:30]

Acknowledged.

 bool LayoutGrid::BaselineMayAffectIntrinsicWidth() const {
-  if (!StyleRef().LogicalWidth().IsIntrinsicOrAuto())
-    return false;
   for (const auto& context : col_axis_alignment_context_) {
+    auto track_size =
+        track_sizing_algorithm_.GetGridTrackSize(kForColumns, context.key);
+    if (!track_size.IsContentSized())
+      continue;
     for (const auto& group : context.value->SharedGroups()) {
-      if (group.size() > 1)
+      auto track_size =

[svillar, 2017/05/25 07:26:27]

Having two "auto track_size" in the same method is very confusing, we need
different names for these variables.

[jfernandez, 2017/05/30 10:39:30]

Acknowledged.

+          track_sizing_algorithm_.Tracks(kForColumns)[context.key].BaseSize();
+      if ((group.size() > 1) &&

[svillar, 2017/05/25 07:26:27]

Perhaps is a dumb question and the code was already there, but what does this
check mean?

[jfernandez, 2017/05/30 10:39:30]

group is the baseline-sharing group [1] which contains items compatible to be
baseline aligned together. It requires more than 1 item to make sense. We can
think of a possible optimization of removing groups of size < 1, but
definitively, not as part of this patch,

[1] https://drafts.csswg.org/css-align-3/#baseline-terms

+          (group.MaxAscent() + group.MaxDescent() > track_size))
         return true;
     }
   }
   return false;
 }
 
+// TODO (lajava): Maybe refactoring these methods based on direction.
 bool LayoutGrid::BaselineMayAffectIntrinsicHeight() const {
-  if (!StyleRef().LogicalHeight().IsIntrinsicOrAuto())
-    return false;
   for (const auto& context : row_axis_alignment_context_) {
+    auto track_size =
+        track_sizing_algorithm_.GetGridTrackSize(kForRows, context.key);
+    if (!track_size.IsContentSized())
+      continue;
     for (const auto& group : context.value->SharedGroups()) {
-      if (group.size() > 1)
+      auto track_size =
+          track_sizing_algorithm_.Tracks(kForRows)[context.key].BaseSize();
+      if ((group.size() > 1) &&
+          (group.MaxAscent() + group.MaxDescent() > track_size))
         return true;
     }
   }