[FlexNG] Use previous constraint space for layout roots.

Devtools had an issue where elements were sized at 0px height.
This was caused by the relayout boundary logic not working correctly.

In Devtools these elements had layout and size containment triggering
ObjectIsRelayoutBoundary() to return true.

The call to NGConstraintSpace::CreateFromLayoutObject for this flex-item
was incorrect as it wasn't using the stretched size (as we don't
set HasOverrideLogicalWidth(), etc).

This refactors the various callers of
NGConstraintSpace::CreateFromLayoutObject and create a new method
UpdateInFlowBlockLayout which contains the previously (duplicated)
logic.

This new method will try and use the previous constraint space (if
it is a layout root), instead of creating it.

Bug: 845235
Change-Id: I52ddb9d58ae010305b980657bb434a30d5b7fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010983
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734420}

third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc

@@ -257,8 +257,8 @@ TEST_F(NGInlineLayoutAlgorithmTest, ContainerBorderPadding) {
   auto* block_flow =
       To<LayoutBlockFlow>(GetLayoutObjectByElementId("container"));
   NGBlockNode block_node(block_flow);
-  NGConstraintSpace space = NGConstraintSpace::CreateFromLayoutObject(
-      *block_flow, false /* is_layout_root */);
+  NGConstraintSpace space =
+      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
   scoped_refptr<const NGLayoutResult> layout_result = block_node.Layout(space);
 
   EXPECT_TRUE(layout_result->BfcBlockOffset().has_value());
@@ -291,8 +291,8 @@ TEST_F(NGInlineLayoutAlgorithmTest, MAYBE_VerticalAlignBottomReplaced) {
       To<LayoutBlockFlow>(GetLayoutObjectByElementId("container"));
   NGInlineNode inline_node(block_flow);
   NGInlineChildLayoutContext context;
-  NGConstraintSpace space = NGConstraintSpace::CreateFromLayoutObject(
-      *block_flow, false /* is_layout_root */);
+  NGConstraintSpace space =
+      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
   scoped_refptr<const NGLayoutResult> layout_result =
       inline_node.Layout(space, nullptr, &context);
 

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

@@ -223,7 +223,7 @@ bool LayoutNGBlockFlowMixin<Base>::NodeAtPoint(
     const PhysicalOffset& accumulated_offset,
     HitTestAction action) {
   if (const NGPaintFragment* paint_fragment = PaintFragment()) {
-    if (!this->IsEffectiveRootScroller()) {
+    if (!Base::IsEffectiveRootScroller()) {
       // Check if we need to do anything at all.
       // If we have clipping, then we can't have any spillout.
       PhysicalRect overflow_box = Base::HasOverflowClip()
@@ -311,26 +311,16 @@ void LayoutNGBlockFlowMixin<Base>::UpdateNGBlockLayout() {
   LayoutAnalyzer::BlockScope analyzer(*this);
 
   if (Base::IsOutOfFlowPositioned()) {
-    this->UpdateOutOfFlowBlockLayout();
+    LayoutNGMixin<Base>::UpdateOutOfFlowBlockLayout();
     return;
   }
 
-  NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(
-          *this, !Base::View()->GetLayoutState()->Next() /* is_layout_root */);
-
-  scoped_refptr<const NGLayoutResult> result =
-      NGBlockNode(this).Layout(constraint_space);
-
-  for (const auto& descendant :
-       result->PhysicalFragment().OutOfFlowPositionedDescendants())
-    descendant.node.UseLegacyOutOfFlowPositioning();
-  this->UpdateMargins(constraint_space);
+  LayoutNGMixin<Base>::UpdateInFlowBlockLayout();
+  UpdateMargins();
 }
 
 template <typename Base>
-void LayoutNGBlockFlowMixin<Base>::UpdateMargins(
-    const NGConstraintSpace& space) {
+void LayoutNGBlockFlowMixin<Base>::UpdateMargins() {
   const LayoutBlock* containing_block = Base::ContainingBlock();
   if (!containing_block || !containing_block->IsLayoutBlockFlow())
     return;
@@ -343,13 +333,13 @@ void LayoutNGBlockFlowMixin<Base>::UpdateMargins(
   const ComputedStyle& cb_style = containing_block->StyleRef();
   const auto writing_mode = cb_style.GetWritingMode();
   const auto direction = cb_style.Direction();
-  LayoutUnit percentage_resolution_size =
-      space.PercentageResolutionInlineSizeForParentWritingMode();
-  NGBoxStrut margins = ComputePhysicalMargins(style, percentage_resolution_size)
+  LayoutUnit available_logical_width =
+      LayoutBoxUtils::AvailableLogicalWidth(*this, containing_block);
+  NGBoxStrut margins = ComputePhysicalMargins(style, available_logical_width)
                            .ConvertToLogical(writing_mode, direction);
-  ResolveInlineMargins(style, cb_style, space.AvailableSize().inline_size,
+  ResolveInlineMargins(style, cb_style, available_logical_width,
                        Base::LogicalWidth(), &margins);
-  this->SetMargin(margins.ConvertToPhysical(writing_mode, direction));
+  Base::SetMargin(margins.ConvertToPhysical(writing_mode, direction));
 }
 
 template class CORE_TEMPLATE_EXPORT LayoutNGBlockFlowMixin<LayoutBlockFlow>;

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

@@ -89,7 +89,7 @@ class LayoutNGBlockFlowMixin : public LayoutNGMixin<Base> {
 
  private:
   void AddScrollingOverflowFromChildren();
-  void UpdateMargins(const NGConstraintSpace& space);
+  void UpdateMargins();
 };
 
 // If you edit these export templates, also update templates in

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

@@ -40,16 +40,7 @@ void LayoutNGFlexibleBox::UpdateBlockLayout(bool relayout_children) {
     return;
   }
 
-  NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(
-          *this, !View()->GetLayoutState()->Next() /* is_layout_root */);
-
-  scoped_refptr<const NGLayoutResult> result =
-      NGBlockNode(this).Layout(constraint_space);
-
-  for (const auto& descendant :
-       result->PhysicalFragment().OutOfFlowPositionedDescendants())
-    descendant.node.UseLegacyOutOfFlowPositioning();
+  UpdateInFlowBlockLayout();
 }
 
 namespace {

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

@@ -108,8 +108,7 @@ void LayoutNGMixin<Base>::UpdateOutOfFlowBlockLayout() {
                                                 : Base::ContainingBlock();
   const ComputedStyle* container_style = container->Style();
   NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(*this,
-                                                false /* is_layout_root */);
+      NGConstraintSpace::CreateFromLayoutObject(*this);
 
   // As this is part of the Legacy->NG bridge, the container_builder is used
   // for indicating the resolved size of the OOF-positioned containing-block
@@ -219,6 +218,29 @@ void LayoutNGMixin<Base>::UpdateOutOfFlowBlockLayout() {
   Base::SetIsLegacyInitiatedOutOfFlowLayout(true);
 }
 
+template <typename Base>
+scoped_refptr<const NGLayoutResult>
+LayoutNGMixin<Base>::UpdateInFlowBlockLayout() {
+  const auto* previous_result = Base::GetCachedLayoutResult();
+  bool is_layout_root = !Base::View()->GetLayoutState()->Next();
+
+  // If we are a layout root, use the previous space if available. This will
+  // include any stretched sizes if applicable.
+  NGConstraintSpace constraint_space =
+      is_layout_root && previous_result
+          ? previous_result->GetConstraintSpaceForCaching()
+          : NGConstraintSpace::CreateFromLayoutObject(*this);
+
+  scoped_refptr<const NGLayoutResult> result =
+      NGBlockNode(this).Layout(constraint_space);
+
+  for (const auto& descendant :
+       result->PhysicalFragment().OutOfFlowPositionedDescendants())
+    descendant.node.UseLegacyOutOfFlowPositioning();
+
+  return result;
+}
+
 template class CORE_TEMPLATE_EXPORT LayoutNGMixin<LayoutBlock>;
 template class CORE_TEMPLATE_EXPORT LayoutNGMixin<LayoutBlockFlow>;
 template class CORE_TEMPLATE_EXPORT LayoutNGMixin<LayoutProgress>;

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

@@ -37,6 +37,7 @@ class LayoutNGMixin : public Base {
       LayoutUnit& max_logical_width) const override;
 
   void UpdateOutOfFlowBlockLayout();
+  scoped_refptr<const NGLayoutResult> UpdateInFlowBlockLayout();
 };
 
 extern template class CORE_EXTERN_TEMPLATE_EXPORT LayoutNGMixin<LayoutBlock>;

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

@@ -57,23 +57,9 @@ void LayoutNGTableCaption::UpdateBlockLayout(bool relayout_children) {
 
   DCHECK(!IsOutOfFlowPositioned()) << "Out of flow captions are blockified.";
 
-  NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(
-          *this, !View()->GetLayoutState()->Next() /* is_layout_root */);
-
-  scoped_refptr<const NGLayoutResult> result =
-      NGBlockNode(this).Layout(constraint_space);
-
-  CalculateAndSetMargins(constraint_space, result->PhysicalFragment());
-
-  // Tell legacy layout there were abspos descendents we couldn't place. We know
-  // we have to pass up to legacy here because this method is legacy's entry
-  // point to LayoutNG. If our parent were LayoutNG, it wouldn't have called
-  // UpdateBlockLayout, it would have packaged this LayoutObject into
-  // NGBlockNode and called Layout on that.
-  for (const auto& descendant :
-       result->PhysicalFragment().OutOfFlowPositionedDescendants())
-    descendant.node.UseLegacyOutOfFlowPositioning();
+  scoped_refptr<const NGLayoutResult> result = UpdateInFlowBlockLayout();
+  CalculateAndSetMargins(result->GetConstraintSpaceForCaching(),
+                         result->PhysicalFragment());
 
   // The parent table sometimes changes the caption's position after laying it
   // out. So there's no point in setting the fragment's offset here;

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

@@ -22,17 +22,7 @@ void LayoutNGTableCell::UpdateBlockLayout(bool relayout_children) {
   LayoutAnalyzer::BlockScope analyzer(*this);
 
   SetOverrideLogicalWidth(LogicalWidth());
-
-  NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(
-          *this, !View()->GetLayoutState()->Next() /* is_layout_root */);
-
-  scoped_refptr<const NGLayoutResult> result =
-      NGBlockNode(this).Layout(constraint_space);
-
-  for (const auto& descendant :
-       result->PhysicalFragment().OutOfFlowPositionedDescendants())
-    descendant.node.UseLegacyOutOfFlowPositioning();
+  UpdateInFlowBlockLayout();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/mathml/layout_ng_mathml_block.cc

@@ -27,16 +27,7 @@ void LayoutNGMathMLBlock::UpdateBlockLayout(bool relayout_children) {
     return;
   }
 
-  NGConstraintSpace constraint_space =
-      NGConstraintSpace::CreateFromLayoutObject(
-          *this, !View()->GetLayoutState()->Next() /* is_layout_root */);
-
-  scoped_refptr<const NGLayoutResult> result =
-      NGBlockNode(this).Layout(constraint_space);
-
-  for (const auto& descendant :
-       result->PhysicalFragment().OutOfFlowPositionedDescendants())
-    descendant.node.UseLegacyOutOfFlowPositioning();
+  UpdateInFlowBlockLayout();
 }
 
 bool LayoutNGMathMLBlock::IsOfType(LayoutObjectType type) const {

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

@@ -49,8 +49,8 @@ std::pair<scoped_refptr<const NGPhysicalBoxFragment>, NGConstraintSpace>
 NGBaseLayoutAlgorithmTest::RunBlockLayoutAlgorithmForElement(Element* element) {
   auto* block_flow = To<LayoutBlockFlow>(element->GetLayoutObject());
   NGBlockNode node(block_flow);
-  NGConstraintSpace space = NGConstraintSpace::CreateFromLayoutObject(
-      *block_flow, false /* is_layout_root */);
+  NGConstraintSpace space =
+      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
   NGFragmentGeometry fragment_geometry =
       CalculateInitialFragmentGeometry(space, node);
 

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

@@ -35,8 +35,7 @@ static_assert(sizeof(NGConstraintSpace) == sizeof(SameSizeAsNGConstraintSpace),
 }  // namespace
 
 NGConstraintSpace NGConstraintSpace::CreateFromLayoutObject(
-    const LayoutBlock& block,
-    bool is_layout_root) {
+    const LayoutBlock& block) {
   // We should only ever create a constraint space from legacy layout if the
   // object is a new formatting context.
   DCHECK(block.CreatesNewFormattingContext());
@@ -77,20 +76,7 @@ NGConstraintSpace NGConstraintSpace::CreateFromLayoutObject(
                                    /* is_new_fc */ true,
                                    !parallel_containing_block);
 
-  auto* previous_result = block.GetCachedLayoutResult();
-  if (is_layout_root && previous_result) {
-    // Due to layout-roots (starting layout at an arbirary node, instead of the
-    // |LayoutView|), we can end up with a situation where we'll miss our cache
-    // due to baseline-requests not matching.
-    //
-    // For the case where we start at a layout-root, the baselines don't
-    // particularly matter, so we just request exactly the same as the previous
-    // layout.
-    const NGConstraintSpace& previous_space =
-        previous_result->GetConstraintSpaceForCaching();
-    builder.SetNeedsBaseline(previous_space.NeedsBaseline());
-    builder.SetBaselineAlgorithmType(previous_space.BaselineAlgorithmType());
-  } else if (!block.IsWritingModeRoot() || block.IsGridItem()) {
+  if (!block.IsWritingModeRoot() || block.IsGridItem()) {
     // We don't know if the parent layout will require our baseline, so always
     // request it.
     builder.SetNeedsBaseline(true);

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

@@ -146,8 +146,7 @@ class CORE_EXPORT NGConstraintSpace final {
   // Creates NGConstraintSpace representing LayoutObject's containing block.
   // This should live on NGBlockNode or another layout bridge and probably take
   // a root NGConstraintSpace.
-  static NGConstraintSpace CreateFromLayoutObject(const LayoutBlock&,
-                                                  bool is_layout_root);
+  static NGConstraintSpace CreateFromLayoutObject(const LayoutBlock&);
 
   const NGExclusionSpace& ExclusionSpace() const { return exclusion_space_; }
 

third_party/blink/web_tests/external/wpt/css/css-flexbox/dynamic-change-simplified-layout-002.html

+<!DOCTYPE html>
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht" />
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/" />
+<meta name="assert" content="Tests that certain dynamic changes don't lead to a flex item being sized as zero, instead of its stretched size." />
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="display: flex; width: 100px; height: 100px; background: red;">
+  <div style="contain: layout size; height: 100px; flex: 1; background: green;">
+    <div id="target"></div>
+  </div>
+</div>
+<script>
+document.body.offsetTop;
+document.getElementById('target').style.width = '1px';
+</script>