[LayoutNG] Only create a constraint space for margins when necessary.

Before creating the constraint space for child layout, we need to
calculate the line-left offset for the child. This requires us to
calculate the inline margins. If there are no auto inline margins and no
special alignment involved, we have everything we need, and can defer
fully resolving the margins until right after child layout (when we
actually do know the inline size of the child).

Only create a temporary constraint space for margin resolution if we
need to calculate the inline size of the fragment before layout to
calculate the line-left offset.

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: If84e4d6360c537a8c3a662c2617bef9dece7a644
Reviewed-on: https://chromium-review.googlesource.com/c/1286425Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601868}

third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc

@@ -1317,8 +1317,20 @@ bool NGBlockLayoutAlgorithm::HandleInflow(
   if (child.IsBlock())
     container_builder_.PropagateBreak(*layout_result);
 
-  if (child.IsBlock())
+  if (child.IsBlock()) {
+    // We haven't yet resolved margins wrt. overconstrainedness, unless that was
+    // also required to calculate line-left offset (due to block alignment)
+    // before layout. Do so now, so that we store the correct values (which is
+    // required by e.g. getComputedStyle()).
+    if (!child_data.margins_fully_resolved) {
+      ResolveInlineMargins(child.Style(), Style(),
+                           child_available_size_.inline_size,
+                           fragment.InlineSize(), &child_data.margins);
+      child_data.margins_fully_resolved = true;
+    }
+
     ToNGBlockNode(child).StoreMargins(ConstraintSpace(), child_data.margins);
+  }
 
   *previous_inflow_position = ComputeInflowPosition(
       *previous_inflow_position, child, child_data, child_bfc_block_offset,
@@ -1341,7 +1353,9 @@ NGInflowChildData NGBlockLayoutAlgorithm::ComputeChildData(
   DCHECK(!child.IsFloating());
 
   // Calculate margins in parent's writing mode.
-  NGBoxStrut margins = CalculateMargins(child, child_break_token);
+  bool margins_fully_resolved;
+  NGBoxStrut margins =
+      CalculateMargins(child, child_break_token, &margins_fully_resolved);
 
   // Append the current margin strut with child's block start margin.
   // Non empty border/padding, and new FC use cases are handled inside of the
@@ -1367,7 +1381,8 @@ NGInflowChildData NGBlockLayoutAlgorithm::ComputeChildData(
           margins.LineLeft(ConstraintSpace().Direction()),
       BfcBlockOffset() + logical_block_offset};
 
-  return {child_bfc_offset, margin_strut, margins, force_clearance};
+  return {child_bfc_offset, margin_strut, margins, margins_fully_resolved,
+          force_clearance};
 }
 
 NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition(
@@ -1798,26 +1813,45 @@ NGBlockLayoutAlgorithm::BreakType NGBlockLayoutAlgorithm::BreakTypeBeforeChild(
 
 NGBoxStrut NGBlockLayoutAlgorithm::CalculateMargins(
     NGLayoutInputNode child,
-    const NGBreakToken* child_break_token) {
+    const NGBreakToken* child_break_token,
+    bool* margins_fully_resolved) {
+  // We need to at least partially resolve margins before creating a constraint
+  // space for layout. Layout needs to know the line-left offset before
+  // starting. If the line-left offset cannot be calculated without fully
+  // resolving the margins (because of block alignment), we have to create a
+  // temporary constraint space now to figure out the inline size first. In all
+  // other cases we'll postpone full resolution until after child layout, when
+  // we actually have a child constraint space to use (and know the inline
+  // size).
+  *margins_fully_resolved = false;
+
   DCHECK(child);
   if (child.IsInline())
     return {};
   const ComputedStyle& child_style = child.Style();
+  bool needs_inline_size =
+      NeedsInlineSizeToResolveLineLeft(child_style, Style());
+  if (!needs_inline_size && !child_style.HasMargin())
+    return {};
 
-  NGConstraintSpace space =
-      NGConstraintSpaceBuilder(ConstraintSpace())
-          .SetAvailableSize(child_available_size_)
-          .SetPercentageResolutionSize(child_percentage_size_)
-          .ToConstraintSpace(child_style.GetWritingMode());
-
-  NGBoxStrut margins = ComputeMarginsFor(space, child_style, ConstraintSpace());
+  NGBoxStrut margins = ComputeMarginsFor(
+      child_style, child_percentage_size_.inline_size,
+      ConstraintSpace().GetWritingMode(), ConstraintSpace().Direction());
   if (ShouldIgnoreBlockStartMargin(ConstraintSpace(), child, child_break_token))
     margins.block_start = LayoutUnit();
 
   // As long as the child isn't establishing a new formatting context, we need
-  // to resolve auto margins before layout, to be able to position child floats
-  // correctly.
-  if (!child.CreatesNewFormattingContext()) {
+  // to know its line-left offset before layout, to be able to position child
+  // floats correctly. If we need to resolve auto margins or other alignment
+  // properties to calculate the line-left offset, we also need to calculate its
+  // inline size first.
+  if (!child.CreatesNewFormattingContext() && needs_inline_size) {
+    NGConstraintSpace space =
+        NGConstraintSpaceBuilder(ConstraintSpace())
+            .SetAvailableSize(child_available_size_)
+            .SetPercentageResolutionSize(child_percentage_size_)
+            .ToConstraintSpace(child_style.GetWritingMode());
+
     NGBoxStrut child_border_padding =
         ComputeBorders(space, child) + ComputePadding(space, child.Style());
     LayoutUnit child_inline_size =
@@ -1826,6 +1860,7 @@ NGBoxStrut NGBlockLayoutAlgorithm::CalculateMargins(
     ResolveInlineMargins(child_style, Style(),
                          space.AvailableSize().inline_size, child_inline_size,
                          &margins);
+    *margins_fully_resolved = true;
   }
   return margins;
 }

third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.h

@@ -39,6 +39,7 @@ struct NGInflowChildData {
   NGBfcOffset bfc_offset_estimate;
   NGMarginStrut margin_strut;
   NGBoxStrut margins;
+  bool margins_fully_resolved;
   bool force_clearance;
 };
 
@@ -86,7 +87,8 @@ class CORE_EXPORT NGBlockLayoutAlgorithm
   }
 
   NGBoxStrut CalculateMargins(NGLayoutInputNode child,
-                              const NGBreakToken* child_break_token);
+                              const NGBreakToken* child_break_token,
+                              bool* margins_fully_resolved);
 
   // Creates a new constraint space for the current child.
   NGConstraintSpace CreateConstraintSpaceForChild(

third_party/blink/renderer/core/layout/ng/ng_length_utils.cc

@@ -18,6 +18,41 @@
 
 namespace blink {
 
+namespace {
+
+enum class EBlockAlignment { kStart, kCenter, kEnd };
+
+inline EBlockAlignment BlockAlignment(const ComputedStyle& style,
+                                      const ComputedStyle& container_style) {
+  bool start_auto = style.MarginStartUsing(container_style).IsAuto();
+  bool end_auto = style.MarginEndUsing(container_style).IsAuto();
+  if (start_auto || end_auto) {
+    if (start_auto)
+      return end_auto ? EBlockAlignment::kCenter : EBlockAlignment::kEnd;
+    return EBlockAlignment::kStart;
+  }
+
+  // If none of the inline margins are auto, look for -webkit- text-align
+  // values (which are really about block alignment). These are typically
+  // mapped from the legacy "align" HTML attribute.
+  switch (container_style.GetTextAlign()) {
+    case ETextAlign::kWebkitLeft:
+      if (container_style.IsLeftToRightDirection())
+        return EBlockAlignment::kStart;
+      return EBlockAlignment::kEnd;
+    case ETextAlign::kWebkitRight:
+      if (container_style.IsLeftToRightDirection())
+        return EBlockAlignment::kEnd;
+      return EBlockAlignment::kStart;
+    case ETextAlign::kWebkitCenter:
+      return EBlockAlignment::kCenter;
+    default:
+      return EBlockAlignment::kStart;
+  }
+}
+
+}  // anonymous namespace
+
 bool NeedMinMaxSize(const NGConstraintSpace& constraint_space,
                     const ComputedStyle& style) {
   // This check is technically too broad (fill-available does not need intrinsic
@@ -264,7 +299,14 @@ LayoutUnit ResolveBlockLength(
 
 LayoutUnit ResolveMarginPaddingLength(const NGConstraintSpace& constraint_space,
                                       const Length& length) {
-  DCHECK_GE(constraint_space.AvailableSize().inline_size, LayoutUnit());
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ResolveMarginPaddingLength(percentage_resolution_size, length);
+}
+
+LayoutUnit ResolveMarginPaddingLength(LayoutUnit percentage_resolution_size,
+                                      const Length& length) {
+  DCHECK_GE(percentage_resolution_size, LayoutUnit());
 
   // Margins and padding always get computed relative to the inline size:
   // https://www.w3.org/TR/CSS2/box.html#value-def-margin-width
@@ -274,11 +316,8 @@ LayoutUnit ResolveMarginPaddingLength(const NGConstraintSpace& constraint_space,
       return LayoutUnit();
     case kPercent:
     case kFixed:
-    case kCalculated: {
-      LayoutUnit percentage_resolution_size =
-          constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+    case kCalculated:
       return ValueForLength(length, percentage_resolution_size);
-    }
     case kMinContent:
     case kMaxContent:
     case kFillAvailable:
@@ -663,51 +702,70 @@ LayoutUnit ResolveUsedColumnGap(LayoutUnit available_size,
 NGPhysicalBoxStrut ComputePhysicalMargins(
     const NGConstraintSpace& constraint_space,
     const ComputedStyle& style) {
-  if (style.MarginLeft().IsZero() && style.MarginRight().IsZero() &&
-      style.MarginTop().IsZero() && style.MarginBottom().IsZero()) {
-    return NGPhysicalBoxStrut();
-  }
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ComputePhysicalMargins(style, percentage_resolution_size);
+}
 
-  if (constraint_space.IsAnonymous())
+NGPhysicalBoxStrut ComputePhysicalMargins(
+    const ComputedStyle& style,
+    LayoutUnit percentage_resolution_size) {
+  if (!style.HasMargin())
     return NGPhysicalBoxStrut();
 
   NGPhysicalBoxStrut physical_dim;
-  physical_dim.left =
-      ResolveMarginPaddingLength(constraint_space, style.MarginLeft());
-  physical_dim.right =
-      ResolveMarginPaddingLength(constraint_space, style.MarginRight());
+  physical_dim.left = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                 style.MarginLeft());
+  physical_dim.right = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                  style.MarginRight());
   physical_dim.top =
-      ResolveMarginPaddingLength(constraint_space, style.MarginTop());
-  physical_dim.bottom =
-      ResolveMarginPaddingLength(constraint_space, style.MarginBottom());
+      ResolveMarginPaddingLength(percentage_resolution_size, style.MarginTop());
+  physical_dim.bottom = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                   style.MarginBottom());
   return physical_dim;
 }
 
 NGBoxStrut ComputeMarginsFor(const NGConstraintSpace& constraint_space,
                              const ComputedStyle& style,
                              const NGConstraintSpace& compute_for) {
-  return ComputePhysicalMargins(constraint_space, style)
+  if (constraint_space.IsAnonymous())
+    return NGBoxStrut();
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ComputePhysicalMargins(style, percentage_resolution_size)
       .ConvertToLogical(compute_for.GetWritingMode(), compute_for.Direction());
 }
 
 NGLineBoxStrut ComputeLineMarginsForVisualContainer(
     const NGConstraintSpace& constraint_space,
     const ComputedStyle& style) {
-  return ComputePhysicalMargins(constraint_space, style)
+  if (constraint_space.IsAnonymous())
+    return NGLineBoxStrut();
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ComputePhysicalMargins(style, percentage_resolution_size)
       .ConvertToLineLogical(constraint_space.GetWritingMode(),
                             TextDirection::kLtr);
 }
 
 NGBoxStrut ComputeMarginsForSelf(const NGConstraintSpace& constraint_space,
                                  const ComputedStyle& style) {
-  return ComputePhysicalMargins(constraint_space, style)
+  if (constraint_space.IsAnonymous())
+    return NGBoxStrut();
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ComputePhysicalMargins(style, percentage_resolution_size)
       .ConvertToLogical(style.GetWritingMode(), style.Direction());
 }
 
 NGLineBoxStrut ComputeLineMarginsForSelf(
     const NGConstraintSpace& constraint_space,
     const ComputedStyle& style) {
-  return ComputePhysicalMargins(constraint_space, style)
+  if (constraint_space.IsAnonymous())
+    return NGLineBoxStrut();
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
+  return ComputePhysicalMargins(style, percentage_resolution_size)
       .ConvertToLineLogical(style.GetWritingMode(), style.Direction());
 }
 
@@ -792,15 +850,17 @@ NGBoxStrut ComputePadding(const NGConstraintSpace& constraint_space,
   if (constraint_space.IsAnonymous())
     return NGBoxStrut();
 
+  LayoutUnit percentage_resolution_size =
+      constraint_space.PercentageResolutionInlineSizeForParentWritingMode();
   NGBoxStrut padding;
-  padding.inline_start =
-      ResolveMarginPaddingLength(constraint_space, style.PaddingStart());
-  padding.inline_end =
-      ResolveMarginPaddingLength(constraint_space, style.PaddingEnd());
-  padding.block_start =
-      ResolveMarginPaddingLength(constraint_space, style.PaddingBefore());
-  padding.block_end =
-      ResolveMarginPaddingLength(constraint_space, style.PaddingAfter());
+  padding.inline_start = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                    style.PaddingStart());
+  padding.inline_end = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                  style.PaddingEnd());
+  padding.block_start = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                   style.PaddingBefore());
+  padding.block_end = ResolveMarginPaddingLength(percentage_resolution_size,
+                                                 style.PaddingAfter());
   return padding;
 }
 
@@ -810,8 +870,18 @@ NGLineBoxStrut ComputeLinePadding(const NGConstraintSpace& constraint_space,
                         style.IsFlippedLinesWritingMode());
 }
 
+bool NeedsInlineSizeToResolveLineLeft(const ComputedStyle& style,
+                                      const ComputedStyle& container_style) {
+  // In RTL, there's no block alignment where we can guarantee that line-left
+  // doesn't depend on the inline size of a fragment.
+  if (IsRtl(container_style.Direction()))
+    return true;
+
+  return BlockAlignment(style, container_style) != EBlockAlignment::kStart;
+}
+
 void ResolveInlineMargins(const ComputedStyle& style,
-                          const ComputedStyle& containing_block_style,
+                          const ComputedStyle& container_style,
                           LayoutUnit available_inline_size,
                           LayoutUnit inline_size,
                           NGBoxStrut* margins) {
@@ -819,36 +889,10 @@ void ResolveInlineMargins(const ComputedStyle& style,
   const LayoutUnit used_space = inline_size + margins->InlineSum();
   const LayoutUnit available_space = available_inline_size - used_space;
   if (available_space > LayoutUnit()) {
-    bool start_auto = style.MarginStartUsing(containing_block_style).IsAuto();
-    bool end_auto = style.MarginEndUsing(containing_block_style).IsAuto();
-    enum EBlockAlignment { kStart, kCenter, kEnd };
-    EBlockAlignment alignment;
-    if (start_auto || end_auto) {
-      alignment = start_auto ? (end_auto ? kCenter : kEnd) : kStart;
-    } else {
-      // If none of the inline margins are auto, look for -webkit- text-align
-      // values (which are really about block alignment). These are typically
-      // mapped from the legacy "align" HTML attribute.
-      switch (containing_block_style.GetTextAlign()) {
-        case ETextAlign::kWebkitLeft:
-          alignment =
-              containing_block_style.IsLeftToRightDirection() ? kStart : kEnd;
-          break;
-        case ETextAlign::kWebkitRight:
-          alignment =
-              containing_block_style.IsLeftToRightDirection() ? kEnd : kStart;
-          break;
-        case ETextAlign::kWebkitCenter:
-          alignment = kCenter;
-          break;
-        default:
-          alignment = kStart;
-          break;
-      }
-    }
-    if (alignment == kCenter)
+    EBlockAlignment alignment = BlockAlignment(style, container_style);
+    if (alignment == EBlockAlignment::kCenter)
       margins->inline_start += available_space / 2;
-    else if (alignment == kEnd)
+    else if (alignment == EBlockAlignment::kEnd)
       margins->inline_start += available_space;
   }
   margins->inline_end =

third_party/blink/renderer/core/layout/ng/ng_length_utils.h

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/core/layout/min_max_size.h"
 #include "third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h"
 #include "third_party/blink/renderer/core/layout/ng/geometry/ng_logical_size.h"
+#include "third_party/blink/renderer/core/layout/ng/geometry/ng_physical_size.h"
 #include "third_party/blink/renderer/core/style/computed_style_constants.h"
 #include "third_party/blink/renderer/platform/text/text_direction.h"
 #include "third_party/blink/renderer/platform/text/writing_mode.h"
@@ -81,6 +82,8 @@ CORE_EXPORT LayoutUnit ResolveBlockLength(
 // given constraint space.
 CORE_EXPORT LayoutUnit ResolveMarginPaddingLength(const NGConstraintSpace&,
                                                   const Length&);
+LayoutUnit ResolveMarginPaddingLength(LayoutUnit percentage_resolution_size,
+                                      const Length&);
 
 // For the given style and min/max content sizes, computes the min and max
 // content contribution (https://drafts.csswg.org/css-sizing/#contributions).
@@ -161,12 +164,23 @@ CORE_EXPORT LayoutUnit ResolveUsedColumnGap(LayoutUnit available_size,
 // Compute physical margins.
 CORE_EXPORT NGPhysicalBoxStrut ComputePhysicalMargins(const NGConstraintSpace&,
                                                       const ComputedStyle&);
+CORE_EXPORT NGPhysicalBoxStrut
+ComputePhysicalMargins(const ComputedStyle&,
+                       LayoutUnit percentage_resolution_size);
 
 // Compute margins for the specified NGConstraintSpace.
 CORE_EXPORT NGBoxStrut ComputeMarginsFor(const NGConstraintSpace&,
                                          const ComputedStyle&,
                                          const NGConstraintSpace& compute_for);
 
+inline NGBoxStrut ComputeMarginsFor(const ComputedStyle& child_style,
+                                    LayoutUnit percentage_resolution_size,
+                                    WritingMode container_writing_mode,
+                                    TextDirection container_direction) {
+  return ComputePhysicalMargins(child_style, percentage_resolution_size)
+      .ConvertToLogical(container_writing_mode, container_direction);
+}
+
 // Compute margins for the style owner.
 CORE_EXPORT NGBoxStrut ComputeMarginsForSelf(const NGConstraintSpace&,
                                              const ComputedStyle&);
@@ -206,6 +220,14 @@ CORE_EXPORT NGBoxStrut ComputePadding(const NGConstraintSpace&,
 CORE_EXPORT NGLineBoxStrut ComputeLinePadding(const NGConstraintSpace&,
                                               const ComputedStyle&);
 
+// Return true if we need to know the inline size of the fragment in order to
+// calculate its line-left offset. This is the case when we have auto margins,
+// or when block alignment isn't line-left (e.g. with align!=left, and always in
+// RTL mode).
+bool NeedsInlineSizeToResolveLineLeft(
+    const ComputedStyle& style,
+    const ComputedStyle& containing_block_style);
+
 // Convert inline margins from computed to used values. This will resolve 'auto'
 // values and over-constrainedness. This uses the available size from the
 // constraint space and inline size to compute the margins that are auto, if