Initial implementation of Collapsing Margins computational logic for LayoutNG

This patches introduces the logic that computes Collapsing Margins for the
following pairs:
- top margin of a box and top margin of its first in-flow child
- bottom margin of box and top margin of its next in-flow following sibling

This implementation doesn't take into account any Margins Collapsing style exceptions, e.g. padding, borders etc.

Design document: https://docs.google.com/document/d/1uxbDh4uONFQOiGuiumlJBLGgO4KDWB8ZEkp7Rd47fw4/edit#bookmark=id.gk14n6z7jt8a

BUG=635619

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

third_party/WebKit/Source/core/layout/BUILD.gn

@@ -298,7 +298,6 @@ blink_core_sources("layout") {
     "ng/ng_layout_input_text.h",
     "ng/ng_length_utils.cc",
     "ng/ng_length_utils.h",
-    "ng/ng_margin_strut.h",
     "ng/ng_physical_constraint_space.cc",
     "ng/ng_physical_constraint_space.h",
     "ng/ng_physical_fragment.h",

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

@@ -14,6 +14,18 @@
 #include "platform/LengthFunctions.h"
 
 namespace blink {
+namespace {
+
+LayoutUnit ComputeCollapsedMarginBlockStart(
+    const NGMarginStrut& prev_margin_strut,
+    const NGMarginStrut& curr_margin_strut) {
+  return std::max(prev_margin_strut.margin_block_end,
+                  curr_margin_strut.margin_block_start) -
+         std::max(prev_margin_strut.negative_margin_block_end.abs(),
+                  curr_margin_strut.negative_margin_block_start.abs());
+}
+
+}  // namespace
 
 NGBlockLayoutAlgorithm::NGBlockLayoutAlgorithm(
     PassRefPtr<const ComputedStyle> style,
@@ -50,13 +62,16 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
           return false;
         NGBoxStrut child_margins = computeMargins(
             *constraint_space_for_children_, *current_child_->Style());
+
+        LayoutUnit margin_block_start =
+            CollapseMargins(child_margins, fragment->MarginStrut());
+
         // TODO(layout-ng): Support auto margins
-        builder_->AddChild(
-            fragment,
-            NGLogicalOffset(child_margins.inline_start,
-                            content_size_ + child_margins.block_start));
+        builder_->AddChild(fragment,
+                           NGLogicalOffset(child_margins.inline_start,
+                                           content_size_ + margin_block_start));
 
-        content_size_ += fragment->BlockSize() + child_margins.BlockSum();
+        content_size_ += fragment->BlockSize() + margin_block_start;
         max_inline_size_ =
             std::max(max_inline_size_,
                      fragment->InlineSize() + child_margins.InlineSum());
@@ -64,7 +79,6 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
         if (current_child_)
           return false;
       }
-
       state_ = kStateFinalize;
       return false;
     }
@@ -86,4 +100,31 @@ bool NGBlockLayoutAlgorithm::Layout(const NGConstraintSpace* constraint_space,
   return true;
 }
 
+LayoutUnit NGBlockLayoutAlgorithm::CollapseMargins(
+    const NGBoxStrut& margins,
+    const NGMarginStrut& children_margin_strut) {
+  // Calculate margin strut for the current child.
+  NGMarginStrut curr_margin_strut = children_margin_strut;
+  curr_margin_strut.AppendMarginBlockStart(margins.block_start);
+  curr_margin_strut.AppendMarginBlockEnd(margins.block_end);
+
+  // Set the margin strut for the resultant fragment if this is the first or
+  // last child fragment.
+  if (current_child_ == first_child_)
+    builder_->SetMarginStrutBlockStart(curr_margin_strut);
+  if (!current_child_->NextSibling())
+    builder_->SetMarginStrutBlockEnd(curr_margin_strut);
+
+  // Compute the margin block start for adjoining blocks.
+  LayoutUnit margin_block_start;
+  if (current_child_ != first_child_)
+    margin_block_start = ComputeCollapsedMarginBlockStart(
+        prev_child_margin_strut_, curr_margin_strut);
+
+  prev_child_margin_strut_ = curr_margin_strut;
+  // TODO(layout-ng): support other Margin Collapsing use cases,
+  // i.e. support 0 height elements etc.
+  return margin_block_start;
+}
+
 }  // namespace blink

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

@@ -46,6 +46,16 @@ class CORE_EXPORT NGBlockLayoutAlgorithm : public NGLayoutAlgorithm {
   }
 
  private:
+  // Computes collapsed margins for 2 adjoining blocks and updates the resultant
+  // fragment's MarginStrut if needed.
+  // See https://www.w3.org/TR/CSS2/box.html#collapsing-margins
+  //
+  // @param child_margins Margins information for the current child.
+  // @param children_margin_strut MarginStrut for children of the current child.
+  // @return Margin block start based on collapsed margins result.
+  LayoutUnit CollapseMargins(const NGBoxStrut& child_margins,
+                             const NGMarginStrut& children_margin_strut);
+
   RefPtr<const ComputedStyle> style_;
   Member<NGBox> first_child_;
 
@@ -56,6 +66,8 @@ class CORE_EXPORT NGBlockLayoutAlgorithm : public NGLayoutAlgorithm {
   Member<NGBox> current_child_;
   LayoutUnit content_size_;
   LayoutUnit max_inline_size_;
+  // MarginStrut for the previous child.
+  NGMarginStrut prev_child_margin_strut_;
 };
 
 }  // namespace blink

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

@@ -65,8 +65,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, LayoutBlockChildren) {
   while (!algorithm.Layout(space, &frag))
     ;
   EXPECT_EQ(frag->Width(), LayoutUnit(kWidth));
-  EXPECT_EQ(frag->Height(),
-            LayoutUnit(kHeight1 + kHeight2 + kMarginTop + kMarginBottom));
+  EXPECT_EQ(frag->Height(), LayoutUnit(kHeight1 + kHeight2 + kMarginTop));
   EXPECT_EQ(frag->Type(), NGPhysicalFragmentBase::FragmentBox);
   ASSERT_EQ(frag->Children().size(), 2UL);
 
@@ -79,5 +78,112 @@ TEST_F(NGBlockLayoutAlgorithmTest, LayoutBlockChildren) {
   EXPECT_EQ(child->TopOffset(), kHeight1 + kMarginTop);
 }
 
+// Verifies the collapsing margins case for the next pair:
+// - top margin of a box and top margin of its first in-flow child.
+//
+// Test case's HTML representation:
+// <div style="margin-top: 20px; height: 50px;">  <!-- DIV1 -->
+//    <div style="margin-top: 10px"></div>        <!-- DIV2 -->
+// </div>
+//
+// Expected:
+//   Margins are collapsed resulting a single margin 20px = max(20px, 10px)
+TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase1) {
+  const int kHeight = 50;
+  const int kDiv1MarginTop = 20;
+  const int kDiv2MarginTop = 10;
+
+  // DIV1
+  RefPtr<ComputedStyle> div1_style = ComputedStyle::create();
+  div1_style->setHeight(Length(kHeight, Fixed));
+  div1_style->setMarginTop(Length(kDiv1MarginTop, Fixed));
+  NGBox* div1 = new NGBox(div1_style.get());
+
+  // DIV2
+  RefPtr<ComputedStyle> div2_style = ComputedStyle::create();
+  div2_style->setMarginTop(Length(kDiv2MarginTop, Fixed));
+  NGBox* div2 = new NGBox(div2_style.get());
+
+  div1->SetFirstChild(div2);
+
+  NGConstraintSpace* space = new NGConstraintSpace(
+      HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
+  NGBlockLayoutAlgorithm algorithm(style_, div1);
+  NGPhysicalFragment* frag;
+  while (!algorithm.Layout(space, &frag))
+    ;
+
+  EXPECT_EQ(frag->MarginStrut().margin_block_start, kDiv1MarginTop);
+  ASSERT_EQ(frag->Children().size(), 1UL);
+  const NGPhysicalFragmentBase* div2_fragment = frag->Children()[0];
+  EXPECT_EQ(div2_fragment->MarginStrut().margin_block_start, kDiv2MarginTop);
+}
+
+// Verifies the collapsing margins case for the next pair:
+// - bottom margin of box and top margin of its next in-flow following sibling.
+//
+// Test case's HTML representation:
+// <div style="margin-bottom: 20px; height: 50px;">  <!-- DIV1 -->
+//    <div style="margin-bottom: -15px"></div>       <!-- DIV2 -->
+// </div>
+// <div style="margin-top: 10px; height: 50px;">     <!-- DIV3 -->
+//    <div style="margin-top: -30px"></div>          <!-- DIV4 -->
+// </div>
+//
+// Expected:
+//   Margins are collapsed resulting an overlap
+//   -10px = max(20px, 10px) - max(abs(-15px), abs(-30px))
+//   between DIV2 and DIV3.
+TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsCase2) {
+  const int kHeight = 50;
+  const int kDiv1MarginBottom = 20;
+  const int kDiv2MarginBottom = -15;
+  const int kDiv3MarginTop = 10;
+  const int kDiv4MarginTop = -30;
+  const int kExpectedCollapsedMargin = -10;
+
+  // DIV1
+  RefPtr<ComputedStyle> div1_style = ComputedStyle::create();
+  div1_style->setHeight(Length(kHeight, Fixed));
+  div1_style->setMarginBottom(Length(kDiv1MarginBottom, Fixed));
+  NGBox* div1 = new NGBox(div1_style.get());
+
+  // DIV2
+  RefPtr<ComputedStyle> div2_style = ComputedStyle::create();
+  div2_style->setMarginBottom(Length(kDiv2MarginBottom, Fixed));
+  NGBox* div2 = new NGBox(div2_style.get());
+
+  // DIV3
+  RefPtr<ComputedStyle> div3_style = ComputedStyle::create();
+  div3_style->setHeight(Length(kHeight, Fixed));
+  div3_style->setMarginTop(Length(kDiv3MarginTop, Fixed));
+  NGBox* div3 = new NGBox(div3_style.get());
+
+  // DIV4
+  RefPtr<ComputedStyle> div4_style = ComputedStyle::create();
+  div4_style->setMarginTop(Length(kDiv4MarginTop, Fixed));
+  NGBox* div4 = new NGBox(div4_style.get());
+
+  div1->SetFirstChild(div2);
+  div3->SetFirstChild(div4);
+  div1->SetNextSibling(div3);
+
+  NGConstraintSpace* space = new NGConstraintSpace(
+      HorizontalTopBottom, NGLogicalSize(LayoutUnit(100), NGSizeIndefinite));
+  NGBlockLayoutAlgorithm algorithm(style_, div1);
+  NGPhysicalFragment* frag;
+  while (!algorithm.Layout(space, &frag))
+    ;
+
+  ASSERT_EQ(frag->Children().size(), 2UL);
+
+  const NGPhysicalFragmentBase* child = frag->Children()[0];
+  EXPECT_EQ(child->Height(), kHeight);
+  EXPECT_EQ(child->TopOffset(), 0);
+
+  child = frag->Children()[1];
+  EXPECT_EQ(child->Height(), kHeight);
+  EXPECT_EQ(child->TopOffset(), kHeight + kExpectedCollapsedMargin);
+}
 }  // namespace
 }  // namespace blink

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

@@ -41,6 +41,11 @@ LayoutUnit NGFragmentBase::BlockOffset() const {
              : physical_fragment_->LeftOffset();
 }
 
+NGMarginStrut NGFragmentBase::MarginStrut() const {
+  // TODO(layout-ng): Change MarginStrut to support writing direction.
+  return physical_fragment_->MarginStrut();
+}
+
 DEFINE_TRACE(NGFragmentBase) {
   visitor->trace(physical_fragment_);
 }

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

@@ -8,6 +8,7 @@
 #include "core/CoreExport.h"
 #include "core/layout/ng/ng_physical_constraint_space.h"
 #include "core/layout/ng/ng_writing_mode.h"
+#include "core/layout/ng/ng_units.h"
 #include "platform/LayoutUnit.h"
 #include "platform/heap/Handle.h"
 #include "wtf/Vector.h"
@@ -35,6 +36,8 @@ class CORE_EXPORT NGFragmentBase : public GarbageCollected<NGFragmentBase> {
   LayoutUnit InlineOffset() const;
   LayoutUnit BlockOffset() const;
 
+  NGMarginStrut MarginStrut() const;
+
   NGPhysicalFragmentBase* PhysicalFragment() const {
     return physical_fragment_;
   };

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

@@ -10,7 +10,9 @@ NGFragmentBuilder::NGFragmentBuilder(
     NGPhysicalFragmentBase::NGFragmentType type)
     : type_(type),
       writing_mode_(HorizontalTopBottom),
-      direction_(LeftToRight) {}
+      direction_(LeftToRight),
+      is_margin_strut_block_start_updated_(false),
+      is_margin_strut_block_end_updated_(false) {}
 
 NGFragmentBuilder& NGFragmentBuilder::SetWritingMode(
     NGWritingMode writing_mode) {
@@ -52,6 +54,24 @@ NGFragmentBuilder& NGFragmentBuilder::AddChild(NGFragment* child,
   return *this;
 }
 
+NGFragmentBuilder& NGFragmentBuilder::SetMarginStrutBlockStart(
+    const NGMarginStrut& from) {
+  DCHECK(!is_margin_strut_block_start_updated_);
+  margin_strut_.margin_block_start = from.margin_block_start;
+  margin_strut_.negative_margin_block_start = from.negative_margin_block_start;
+  is_margin_strut_block_start_updated_ = true;
+  return *this;
+}
+
+NGFragmentBuilder& NGFragmentBuilder::SetMarginStrutBlockEnd(
+    const NGMarginStrut& from) {
+  DCHECK(!is_margin_strut_block_end_updated_);
+  margin_strut_.margin_block_end = from.margin_block_end;
+  margin_strut_.negative_margin_block_end = from.negative_margin_block_end;
+  is_margin_strut_block_end_updated_ = true;
+  return *this;
+}
+
 NGPhysicalFragment* NGFragmentBuilder::ToFragment() {
   // TODO(layout-ng): Support text fragments
   DCHECK_EQ(type_, NGPhysicalFragmentBase::FragmentBox);
@@ -67,9 +87,9 @@ NGPhysicalFragment* NGFragmentBuilder::ToFragment() {
         writing_mode_, direction_, physical_size, child->Size()));
     children.append(child);
   }
-
-  return new NGPhysicalFragment(
-      physical_size, overflow_.ConvertToPhysical(writing_mode_), children);
+  return new NGPhysicalFragment(physical_size,
+                                overflow_.ConvertToPhysical(writing_mode_),
+                                children, margin_strut_);
 }
 
 }  // namespace blink

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

@@ -26,6 +26,12 @@ class CORE_EXPORT NGFragmentBuilder final
 
   NGFragmentBuilder& AddChild(NGFragment*, NGLogicalOffset);
 
+  // Sets MarginStrut for the resultant fragment.
+  // These 2 methods below should be only called once as we update the
+  // fragment's MarginStrut block start/end from first/last children MarginStrut
+  NGFragmentBuilder& SetMarginStrutBlockStart(const NGMarginStrut& from);
+  NGFragmentBuilder& SetMarginStrutBlockEnd(const NGMarginStrut& from);
+
   // Offsets are not supposed to be set during fragment construction, so we
   // do not provide a setter here.
 
@@ -42,8 +48,15 @@ class CORE_EXPORT NGFragmentBuilder final
   NGLogicalSize size_;
   NGLogicalSize overflow_;
 
+  NGMarginStrut margin_strut_;
+
   HeapVector<Member<NGPhysicalFragmentBase>> children_;
   Vector<NGLogicalOffset> offsets_;
+
+  // Whether MarginStrut block start/end was updated.
+  // It's used for DCHECK safety check.
+  bool is_margin_strut_block_start_updated_ : 1;
+  bool is_margin_strut_block_end_updated_ : 1;
 };
 
 }  // namespace blink

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

@@ -5,7 +5,7 @@
 #include "core/layout/ng/ng_length_utils.h"
 
 #include "core/layout/ng/ng_constraint_space.h"
-#include "core/layout/ng/ng_margin_strut.h"
+#include "core/layout/ng/ng_units.h"
 #include "core/style/ComputedStyle.h"
 #include "platform/CalculationValue.h"
 #include "platform/LayoutUnit.h"

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

-// 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.
-
-#ifndef NGMarginStrut_h
-#define NGMarginStrut_h
-
-#include "core/CoreExport.h"
-#include "platform/LayoutUnit.h"
-
-namespace blink {
-
-// This struct is used for the margin collapsing calculation.
-struct NGMarginStrut {
-  LayoutUnit margin_block_start;
-  LayoutUnit margin_block_end;
-
-  LayoutUnit negative_margin_block_start;
-  LayoutUnit negative_margin_block_end;
-};
-
-}  // namespace blink
-
-#endif  // NGMarginStrut_h

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

@@ -18,8 +18,9 @@ class CORE_EXPORT NGPhysicalFragment final : public NGPhysicalFragmentBase {
   // This modifies the passed-in children vector.
   NGPhysicalFragment(NGPhysicalSize size,
                      NGPhysicalSize overflow,
-                     HeapVector<Member<const NGPhysicalFragmentBase>>& children)
-      : NGPhysicalFragmentBase(size, overflow, FragmentBox) {
+                     HeapVector<Member<const NGPhysicalFragmentBase>>& children,
+                     NGMarginStrut margin_strut)
+      : NGPhysicalFragmentBase(size, overflow, FragmentBox, margin_strut) {
     children_.swap(children);
   }
 

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

@@ -7,6 +7,7 @@
 
 #include "core/CoreExport.h"
 #include "core/layout/ng/ng_constraint_space.h"
+#include "core/layout/ng/ng_units.h"
 #include "platform/LayoutUnit.h"
 #include "platform/heap/Handle.h"
 #include "wtf/Vector.h"
@@ -58,21 +59,26 @@ class CORE_EXPORT NGPhysicalFragmentBase
     has_been_placed_ = true;
   }
 
+  NGMarginStrut MarginStrut() const { return margin_strut_; }
+
   DEFINE_INLINE_TRACE_AFTER_DISPATCH() {}
   DECLARE_TRACE();
 
  protected:
   NGPhysicalFragmentBase(NGPhysicalSize size,
                          NGPhysicalSize overflow,
-                         NGFragmentType type)
+                         NGFragmentType type,
+                         NGMarginStrut margin_strut)
       : size_(size),
         overflow_(overflow),
+        margin_strut_(margin_strut),
         type_(type),
         has_been_placed_(false) {}
 
   NGPhysicalSize size_;
   NGPhysicalSize overflow_;
   NGPhysicalOffset offset_;
+  NGMarginStrut margin_strut_;
 
   unsigned type_ : 1;
   unsigned has_been_placed_ : 1;

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

@@ -17,7 +17,7 @@ namespace blink {
 class CORE_EXPORT NGPhysicalTextFragment final : public NGPhysicalFragmentBase {
  public:
   NGPhysicalTextFragment(NGPhysicalSize size, NGPhysicalSize overflow)
-      : NGPhysicalFragmentBase(size, overflow, FragmentText) {}
+      : NGPhysicalFragmentBase(size, overflow, FragmentText, {}) {}
 
   String Text() const { return text_list_->Text(start_offset_, end_offset_); }
 

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

@@ -61,4 +61,29 @@ NGPhysicalOffset NGLogicalOffset::ConvertToPhysical(
   }
 }
 
+void NGMarginStrut::AppendMarginBlockStart(const LayoutUnit& value) {
+  if (value < 0) {
+    negative_margin_block_start =
+        -std::max(value.abs(), negative_margin_block_start.abs());
+  } else {
+    margin_block_start = std::max(value, margin_block_start);
+  }
+}
+
+void NGMarginStrut::AppendMarginBlockEnd(const LayoutUnit& value) {
+  if (value < 0) {
+    negative_margin_block_end =
+        -std::max(value.abs(), negative_margin_block_end.abs());
+  } else {
+    margin_block_end = std::max(value, margin_block_end);
+  }
+}
+
+String NGMarginStrut::ToString() const {
+  return String::format(
+      "Start: (%d %d) End: (%d %d)", negative_margin_block_start.toInt(),
+      margin_block_start.toInt(), negative_margin_block_end.toInt(),
+      margin_block_end.toInt());
+}
+
 }  // namespace blink

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

@@ -9,6 +9,7 @@
 #include "core/layout/ng/ng_direction.h"
 #include "core/layout/ng/ng_writing_mode.h"
 #include "platform/LayoutUnit.h"
+#include "wtf/text/WTFString.h"
 
 namespace blink {
 
@@ -116,6 +117,25 @@ struct NGBoxStrut {
   }
 };
 
+// This struct is used for the margin collapsing calculation.
+struct NGMarginStrut {
+  LayoutUnit margin_block_start;
+  LayoutUnit margin_block_end;
+
+  LayoutUnit negative_margin_block_start;
+  LayoutUnit negative_margin_block_end;
+
+  void AppendMarginBlockStart(const LayoutUnit& value);
+  void AppendMarginBlockEnd(const LayoutUnit& value);
+
+  String ToString() const;
+};
+
+inline std::ostream& operator<<(std::ostream& stream,
+                                const NGMarginStrut& value) {
+  return stream << value.ToString();
+}
+
 }  // namespace blink
 
 #endif  // NGUnits_h