Fix incorrectly calculated content_size for fragments that create new FC

third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/layout/ng/ng_block_layout_algorithm.h"
 
 #include "core/layout/ng/ng_constraint_space.h"
 #include "core/layout/ng/ng_fragment_builder.h"
 #include "core/layout/ng/ng_fragment.h"
 #include "core/layout/ng/ng_length_utils.h"
@@ skipping 63 matching lines @@
     case kStateChildLayout: {
       if (current_child_) {
         NGFragment* fragment;
         if (!current_child_->Layout(constraint_space_for_children_, &fragment))
           return false;
         NGBoxStrut child_margins = computeMargins(
             *constraint_space_for_children_, *current_child_->Style(),
             constraint_space_for_children_->WritingMode(),
             constraint_space_for_children_->Direction());
 
-        LayoutUnit margin_block_start =
+        const NGBoxStrut margins =
             CollapseMargins(*constraint_space, child_margins, *fragment);
 
         // TODO(layout-ng): Support auto margins
-        builder_->AddChild(fragment,
-                           NGLogicalOffset(border_and_padding_.inline_start +
-                                               child_margins.inline_start,
-                                           content_size_ + margin_block_start));
+        builder_->AddChild(
+            fragment, NGLogicalOffset(border_and_padding_.inline_start +
+                                          child_margins.inline_start,
+                                      content_size_ + margins.block_start));
 
-        content_size_ += fragment->BlockSize() + margin_block_start;
+        content_size_ += fragment->BlockSize() + margins.BlockSum();
         max_inline_size_ =
             std::max(max_inline_size_, fragment->InlineSize() +
                                            child_margins.InlineSum() +
                                            border_and_padding_.InlineSum());
         current_child_ = current_child_->NextSibling();
         if (current_child_)
           return false;
       }
       state_ = kStateFinalize;
       return false;
     }
     case kStateFinalize: {
       content_size_ += border_and_padding_.block_end;
+
       // Recompute the block-axis size now that we know our content size.
       LayoutUnit block_size = computeBlockSizeForFragment(
           *constraint_space, *style_, content_size_);
 
       builder_->SetBlockSize(block_size)
           .SetInlineOverflow(max_inline_size_)
           .SetBlockOverflow(content_size_);
       *out = builder_->ToFragment();
       state_ = kStateInit;
       return true;
     }
   };
   NOTREACHED();
   *out = nullptr;
   return true;
 }
 
-LayoutUnit NGBlockLayoutAlgorithm::CollapseMargins(
+NGBoxStrut NGBlockLayoutAlgorithm::CollapseMargins(
     const NGConstraintSpace& space,
     const NGBoxStrut& margins,
     const NGFragment& fragment) {
-  // Zero-height boxes are ignored and do not participate in margin collapsing.
-  bool is_zero_height_box = !fragment.BlockSize() && margins.IsEmpty();
-  if (is_zero_height_box)
-    return LayoutUnit();
-
-  // Create the current child's margin strut from its children's margin strut.
-  NGMarginStrut curr_margin_strut = fragment.MarginStrut();
+  bool is_zero_height_box = !fragment.BlockSize() && margins.IsEmpty() &&
+                            fragment.MarginStrut().IsEmpty();
+  // Create the current child's margin strut from its children's margin strut or
+  // use margin strut from the the last non-empty child.
+  NGMarginStrut curr_margin_strut =
+      is_zero_height_box ? prev_child_margin_strut_ : fragment.MarginStrut();
 
   // Calculate borders and padding for the current child.
   NGBoxStrut border_and_padding =
       computeBorders(*current_child_->Style()) +
       computePadding(space, *current_child_->Style());
 
   // Collapse BLOCK-START margins if there is no padding or border between
   // parent (current child) and its first in-flow child.
   if (border_and_padding.block_start) {
     curr_margin_strut.SetMarginBlockStart(margins.block_start);
   } else {
     curr_margin_strut.AppendMarginBlockStart(margins.block_start);
   }
 
   // Collapse BLOCK-END margins if
   // 1) there is no padding or border between parent (current child) and its
   //    first/last in-flow child
   // 2) parent's logical height is auto.
   if (current_child_->Style()->logicalHeight().isAuto() &&
       !border_and_padding.block_end) {
     curr_margin_strut.AppendMarginBlockEnd(margins.block_end);
   } else {
     curr_margin_strut.SetMarginBlockEnd(margins.block_end);
   }
 
+  NGBoxStrut result_margins;
+  // Margins of the newly established formatting context do not participate
+  // in Collapsing Margins.
+  bool is_last_child = !current_child_->NextSibling();
+  if (is_last_child && space.IsNewFormattingContext()) {
+    result_margins.block_end = curr_margin_strut.BlockEndSum();
+    builder_->ClearMarginStrut();

[ikilpatrick, 2016/10/04 16:21:35]

isn't this immediately going to be set at UpdateMarginStrut?

Instead of clearing can we just set it when needed?

+  }
+
+  // Zero-height boxes are ignored and do not participate in margin collapsing.
+  if (is_zero_height_box)
+    return result_margins;
+
   // Compute the margin block start for
   // 1) adjoining blocks
   // 2) 1st block in the newly established formatting context.
-  LayoutUnit margin_block_start;
   if (is_fragment_margin_strut_block_start_updated_ ||
       space.IsNewFormattingContext()) {
-    margin_block_start = ComputeCollapsedMarginBlockStart(
+    result_margins.block_start = ComputeCollapsedMarginBlockStart(
         prev_child_margin_strut_, curr_margin_strut);
   }
 
   // Update the parent fragment's margin strut
   UpdateMarginStrut(curr_margin_strut);
 
   prev_child_margin_strut_ = curr_margin_strut;
-  return margin_block_start;
+  return result_margins;
 }
 
 void NGBlockLayoutAlgorithm::UpdateMarginStrut(const NGMarginStrut& from) {
   if (!is_fragment_margin_strut_block_start_updated_) {
     builder_->SetMarginStrutBlockStart(from);
     is_fragment_margin_strut_block_start_updated_ = true;
   }
   builder_->SetMarginStrutBlockEnd(from);
 }
 
 }  // namespace blink