Collapse bottom margins of a last child and its parent if parent's height=auto

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

Index: third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
index 674d88a69c6cf7724e851525b08fadf90e2a75f8..1ee17643bc8a4a8446df00f3731c023880741317 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
@@ -18,6 +18,15 @@ class NGBlockLayoutAlgorithmTest : public ::testing::Test {
  protected:
   void SetUp() override { style_ = ComputedStyle::create(); }
 
+  NGPhysicalFragment* RunBlockLayoutAlgorithm(const NGConstraintSpace* space,
+                                              NGBox* first_child) {
+    NGBlockLayoutAlgorithm algorithm(style_, first_child);
+    NGPhysicalFragment* frag;
+    while (!algorithm.Layout(space, &frag))
+      ;

[cbiesinger, 2016/09/13 18:59:48]

I recently learned that the style guide actually doesn't allow this:
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

Mind fixing that while you're touching this?

[Gleb Lanbin, 2016/09/13 19:33:44]

Done.

+    return frag;
+  }
+
   RefPtr<ComputedStyle> style_;
 };
 
@@ -45,9 +54,6 @@ TEST_F(NGBlockLayoutAlgorithmTest, LayoutBlockChildren) {
   const int kMarginBottom = 20;
   style_->setWidth(Length(kWidth, Fixed));
 
-  NGConstraintSpace* space = new NGConstraintSpace(
-      HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
-
   RefPtr<ComputedStyle> first_style = ComputedStyle::create();
   first_style->setHeight(Length(kHeight1, Fixed));
   NGBox* first_child = new NGBox(first_style.get());
@@ -60,10 +66,10 @@ TEST_F(NGBlockLayoutAlgorithmTest, LayoutBlockChildren) {
 
   first_child->SetNextSibling(second_child);
 
-  NGBlockLayoutAlgorithm algorithm(style_, first_child);
-  NGPhysicalFragment* frag;
-  while (!algorithm.Layout(space, &frag))
-    ;
+  auto space = new NGConstraintSpace(

[cbiesinger, 2016/09/13 18:59:49]

auto* please, for all of these -- I think that was the consensus on previous
discussions

[ikilpatrick, 2016/09/13 19:08:39]

auto*

[Gleb Lanbin, 2016/09/13 19:33:44]

Done.

[Gleb Lanbin, 2016/09/13 19:33:44]

Done.

+      HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
+  NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, first_child);
+
   EXPECT_EQ(frag->Width(), LayoutUnit(kWidth));
   EXPECT_EQ(frag->Height(), LayoutUnit(kHeight1 + kHeight2 + kMarginTop));
   EXPECT_EQ(frag->Type(), NGPhysicalFragmentBase::FragmentBox);
@@ -106,17 +112,15 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase1) {
 
   div1->SetFirstChild(div2);
 
-  NGConstraintSpace* space = new NGConstraintSpace(
+  auto* space = new NGConstraintSpace(
       HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
-  NGBlockLayoutAlgorithm algorithm(style_, div1);
-  NGPhysicalFragment* frag;
-  while (!algorithm.Layout(space, &frag))
-    ;
+  NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, div1);
 
-  EXPECT_EQ(frag->MarginStrut().margin_block_start, kDiv1MarginTop);
+  EXPECT_EQ(frag->MarginStrut(), NGMarginStrut({LayoutUnit(kDiv1MarginTop)}));
   ASSERT_EQ(frag->Children().size(), 1UL);
   const NGPhysicalFragmentBase* div2_fragment = frag->Children()[0];
-  EXPECT_EQ(div2_fragment->MarginStrut().margin_block_start, kDiv2MarginTop);
+  EXPECT_EQ(div2_fragment->MarginStrut(),
+            NGMarginStrut({LayoutUnit(kDiv2MarginTop)}));
 }
 
 // Verifies the collapsing margins case for the next pair:
@@ -168,12 +172,9 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase2) {
   div3->SetFirstChild(div4);
   div1->SetNextSibling(div3);
 
-  NGConstraintSpace* space = new NGConstraintSpace(
+  auto space = new NGConstraintSpace(

[ikilpatrick, 2016/09/13 19:08:39]

auto*

[Gleb Lanbin, 2016/09/13 19:33:44]

Done.

       HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
-  NGBlockLayoutAlgorithm algorithm(style_, div1);
-  NGPhysicalFragment* frag;
-  while (!algorithm.Layout(space, &frag))
-    ;
+  NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, div1);
 
   ASSERT_EQ(frag->Children().size(), 2UL);
 
@@ -186,6 +187,52 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase2) {
   EXPECT_EQ(child->TopOffset(), kHeight + kExpectedCollapsedMargin);
 }
 
+// Verifies the collapsing margins case for the next pair:
+// - bottom margin of a last in-flow child and bottom margin of its parent if
+//   the parent has 'auto' computed height
+//
+// Test case's HTML representation:
+// <div style="margin-bottom: 20px; height: 50px;">            <!-- DIV1 -->
+//   <div style="margin-bottom: 200px; height: 50px;"/>        <!-- DIV2 -->
+// </div>
+//
+// Expected:
+//   1) Margins are collapsed with the result = std::max(20, 200)
+//      if DIV1.height == auto
+//   2) Margins are NOT collapsed if DIV1.height != auto
+TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase3) {
+  const int kHeight = 50;
+  const int kDiv1MarginBottom = 20;
+  const int kDiv2MarginBottom = 200;
+
+  // DIV1
+  RefPtr<ComputedStyle> div1_style = ComputedStyle::create();
+  div1_style->setMarginBottom(Length(kDiv1MarginBottom, Fixed));
+  NGBox* div1 = new NGBox(div1_style.get());
+
+  // DIV2
+  RefPtr<ComputedStyle> div2_style = ComputedStyle::create();
+  div2_style->setHeight(Length(kHeight, Fixed));
+  div2_style->setMarginBottom(Length(kDiv2MarginBottom, Fixed));
+  NGBox* div2 = new NGBox(div2_style.get());
+
+  div1->SetFirstChild(div2);
+
+  auto space = new NGConstraintSpace(
+      HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
+  NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, div1);
+
+  // Verify that margins are collapsed.
+  EXPECT_EQ(frag->MarginStrut(),
+            NGMarginStrut({LayoutUnit(0), LayoutUnit(kDiv2MarginBottom)}));
+
+  // Verify that margins are NOT collapsed.
+  div1_style->setHeight(Length(kHeight, Fixed));
+  frag = RunBlockLayoutAlgorithm(space, div1);
+  EXPECT_EQ(frag->MarginStrut(),
+            NGMarginStrut({LayoutUnit(0), LayoutUnit(kDiv1MarginBottom)}));
+}
+
 // Verifies that a box's size includes its borders and padding, and that
 // children are positioned inside the content box.
 //
@@ -234,12 +281,9 @@ TEST_F(NGBlockLayoutAlgorithmTest, BorderAndPadding) {
 
   div1->SetFirstChild(div2);
 
-  NGConstraintSpace* space = new NGConstraintSpace(
+  auto space = new NGConstraintSpace(
       HorizontalTopBottom, NGLogicalSize(LayoutUnit(1000), NGSizeIndefinite));
-  NGBlockLayoutAlgorithm algorithm(style_, div1);
-  NGPhysicalFragment* frag;
-  while (!algorithm.Layout(space, &frag))
-    ;
+  NGPhysicalFragment* frag = RunBlockLayoutAlgorithm(space, div1);
 
   ASSERT_EQ(frag->Children().size(), 1UL);