Fix incorrectly calculated content_size for fragments that create new FC

This patch adds MarginStrut.BlockEndSum to the fragment's content size
if the fragment establishes a new formatting context.

The change is tested with layout_ng virtual test suite located in fast/block/basic.

BUG=635619

Review-Url: https://codereview.chromium.org/2389823003
Cr-Commit-Position: refs/heads/master@{#422878}

third_party/WebKit/LayoutTests/TestExpectations

@@ -127,18 +127,6 @@ crbug.com/635619 virtual/layout_ng/fast/block/basic/quirk-percent-height-table-c
 crbug.com/635619 virtual/layout_ng/fast/block/basic/text-indent-rtl.html [ Failure ]
 crbug.com/635619 virtual/layout_ng/fast/block/basic/truncation-rtl.html [ Failure ]
 crbug.com/635619 virtual/layout_ng/fast/block/basic/white-space-pre-wraps.html [ Failure ]
-# TODO(glebl) Update TestExpecations deleting the lines below once http://crrev.com/2389823003 is submitted
-crbug.com/635619 virtual/layout_ng/fast/block/basic/004.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/005.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/006.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/007.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/008.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/009.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/010.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/015.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/019.html [ Failure ]
-crbug.com/635619 virtual/layout_ng/fast/block/basic/021.html [ Failure ]
-
 # ====== LayoutNG-only failures until here ======
 
 # ====== Paint team owned tests to here ======

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

@@ -81,16 +81,16 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
             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() +
@@ -104,6 +104,7 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
     }
     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_);
@@ -121,17 +122,16 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
   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 =
@@ -157,13 +157,28 @@ LayoutUnit NGBlockLayoutAlgorithm::CollapseMargins(
     curr_margin_strut.SetMarginBlockEnd(margins.block_end);
   }
 
-  // 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(
+  NGBoxStrut result_margins;
+  // Margins of the newly established formatting context do not participate
+  // in Collapsing Margins:
+  // - Compute margins block start for adjoining blocks *including* 1st block.
+  // - Compute margins block end for the last block.
+  // - Do not set the computed margins to the parent fragment.
+  if (space.IsNewFormattingContext()) {
+    result_margins.block_start = ComputeCollapsedMarginBlockStart(
+        prev_child_margin_strut_, curr_margin_strut);
+    bool is_last_child = !current_child_->NextSibling();
+    if (is_last_child)
+      result_margins.block_end = curr_margin_strut.BlockEndSum();
+    return result_margins;
+  }
+
+  // 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 adjoining blocks *excluding* 1st block
+  if (is_fragment_margin_strut_block_start_updated_) {
+    result_margins.block_start = ComputeCollapsedMarginBlockStart(
         prev_child_margin_strut_, curr_margin_strut);
   }
 
@@ -171,7 +186,7 @@ LayoutUnit NGBlockLayoutAlgorithm::CollapseMargins(
   UpdateMarginStrut(curr_margin_strut);
 
   prev_child_margin_strut_ = curr_margin_strut;
-  return margin_block_start;
+  return result_margins;
 }
 
 void NGBlockLayoutAlgorithm::UpdateMarginStrut(const NGMarginStrut& from) {

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

@@ -53,8 +53,8 @@ class CORE_EXPORT NGBlockLayoutAlgorithm : public NGLayoutAlgorithm {
   // @param space Constraint space for the block.
   // @param child_margins Margins information for the current child.
   // @param fragment Current child's fragment.
-  // @return Margin block start based on collapsed margins result.
-  LayoutUnit CollapseMargins(const NGConstraintSpace& space,
+  // @return NGBoxStrut with margins block start/end.
+  NGBoxStrut CollapseMargins(const NGConstraintSpace& space,
                              const NGBoxStrut& child_margins,
                              const NGFragment& fragment);
 

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

@@ -134,6 +134,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, LayoutBlockChildrenWithWritingMode) {
 // </div>
 //
 // Expected:
+// - Empty margin strut of the fragment that establishes new formatting context
 // - Margins are collapsed resulting a single margin 20px = max(20px, 10px)
 // - The top offset of DIV2 == 20px
 TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase1) {
@@ -160,7 +161,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase1) {
   space->SetIsNewFormattingContext(true);
   NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, div1);
 
-  EXPECT_EQ(NGMarginStrut({LayoutUnit(kDiv1MarginTop)}), frag->MarginStrut());
+  EXPECT_TRUE(frag->MarginStrut().IsEmpty());
   ASSERT_EQ(frag->Children().size(), 1UL);
   const NGPhysicalFragmentBase* div2_fragment = frag->Children()[0];
   EXPECT_EQ(NGMarginStrut({LayoutUnit(kDiv2MarginTop)}),

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

@@ -71,6 +71,10 @@ bool NGBoxStrut::operator==(const NGBoxStrut& other) const {
          std::tie(inline_start, inline_end, block_start, block_end);
 }
 
+LayoutUnit NGMarginStrut::BlockEndSum() const {
+  return margin_block_end + negative_margin_block_end;
+}
+
 void NGMarginStrut::AppendMarginBlockStart(const LayoutUnit& value) {
   if (value < 0) {
     negative_margin_block_start =
@@ -112,6 +116,10 @@ String NGMarginStrut::ToString() const {
                         negative_margin_block_end.toInt());
 }
 
+bool NGMarginStrut::IsEmpty() const {
+  return *this == NGMarginStrut();
+}
+
 bool NGMarginStrut::operator==(const NGMarginStrut& other) const {
   return std::tie(other.margin_block_start, other.margin_block_end,
                   other.negative_margin_block_start,

third_party/WebKit/Source/core/layout/ng/ng_units.h

@@ -139,11 +139,15 @@ struct CORE_EXPORT NGMarginStrut {
   LayoutUnit negative_margin_block_start;
   LayoutUnit negative_margin_block_end;
 
+  LayoutUnit BlockEndSum() const;
+
   void AppendMarginBlockStart(const LayoutUnit& value);
   void AppendMarginBlockEnd(const LayoutUnit& value);
   void SetMarginBlockStart(const LayoutUnit& value);
   void SetMarginBlockEnd(const LayoutUnit& value);
 
+  bool IsEmpty() const;
+
   String ToString() const;
 
   bool operator==(const NGMarginStrut& other) const;