[overflow] Fix negative margin compat issues.

Many sites use negative block-end margins to "pull" in the end padding.
(This is likely because FF doesn't support this end padding, and
"working" around this fact").

For webcompat this change allows negative margins for determining the
inflow bounds, but only in the scrollable direction.

While manual testing I found also that the simplified layout pass
had a issue of not calling AddChild correctly with all the necessary
information.

Bug: 1157299
Change-Id: I90f24301a7015915b0cd0a4446940ee562876dc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582625Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835640}

third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h

@@ -47,13 +47,6 @@ struct CORE_EXPORT NGBoxStrut {
 
   bool IsEmpty() const { return *this == NGBoxStrut(); }
 
-  void ClampNegativeToZero() {
-    inline_start = inline_start.ClampNegativeToZero();
-    inline_end = inline_end.ClampNegativeToZero();
-    block_start = block_start.ClampNegativeToZero();
-    block_end = block_end.ClampNegativeToZero();
-  }
-
   inline NGPhysicalBoxStrut ConvertToPhysical(WritingDirectionMode) const;
 
   // The following two operators exist primarily to have an easy way to access

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

@@ -225,7 +225,7 @@ void NGBoxFragmentBuilder::AddResult(const NGLayoutResult& child_layout_result,
     }
   }
 
-  NGMarginStrut end_margin_strut = child_layout_result.EndMarginStrut();
+  const NGMarginStrut end_margin_strut = child_layout_result.EndMarginStrut();
   // No margins should pierce outside formatting-context roots.
   DCHECK(!fragment.IsFormattingContextRoot() || end_margin_strut.IsEmpty());
 
@@ -295,17 +295,51 @@ void NGBoxFragmentBuilder::AddChild(const NGPhysicalContainerFragment& child,
                                 : end_margin_strut.Sum();
       }
 
+      // Use the original offset (*without* relative-positioning applied).
       NGFragment fragment(GetWritingDirection(), child);
+      LogicalRect bounds = {child_offset, fragment.Size()};
+
+      // Margins affect the inflow-bounds in interesting ways.
+      //
+      // For the margin which is closest to the direction which we are
+      // scrolling, we allow negative margins, but only up to the size of the
+      // fragment. For the margin furthest away we disallow negative margins.
+      if (!margins.IsEmpty()) {
+        // Convert the physical overflow directions to logical.
+        const bool has_top_overflow = Node().HasTopOverflow();
+        const bool has_left_overflow = Node().HasLeftOverflow();
+        PhysicalToLogical<bool> converter(GetWritingDirection(),
+                                          has_top_overflow, !has_left_overflow,
+                                          !has_top_overflow, has_left_overflow);
+
+        if (converter.InlineStart()) {
+          margins.inline_end = margins.inline_end.ClampNegativeToZero();
+          margins.inline_start =
+              std::max(margins.inline_start, -fragment.InlineSize());
+        } else {
+          margins.inline_start = margins.inline_start.ClampNegativeToZero();
+          margins.inline_end =
+              std::max(margins.inline_end, -fragment.InlineSize());
+        }
+        if (converter.BlockStart()) {
+          margins.block_end = margins.block_end.ClampNegativeToZero();
+          margins.block_start =
+              std::max(margins.block_start, -fragment.BlockSize());
+        } else {
+          margins.block_start = margins.block_start.ClampNegativeToZero();
+          margins.block_end =
+              std::max(margins.block_end, -fragment.BlockSize());
+        }
 
-      // Use the original offset (*without* relative-positioning applied), and
-      // clamp any negative margins to zero.
-      margins.ClampNegativeToZero();
-      LogicalRect bounds = {
-          LogicalOffset(child_offset.inline_offset - margins.inline_start,
-                        child_offset.block_offset - margins.block_start),
-          LogicalSize(
-              margins.inline_start + fragment.InlineSize() + margins.inline_end,
-              margins.block_start + fragment.BlockSize() + margins.block_end)};
+        // Shift the bounds by the (potentially clamped) margins.
+        bounds.offset -= {margins.inline_start, margins.block_start};
+        bounds.size.inline_size += margins.InlineSum();
+        bounds.size.block_size += margins.BlockSum();
+
+        // Our bounds size should never go negative.
+        DCHECK_GE(bounds.size.inline_size, LayoutUnit());
+        DCHECK_GE(bounds.size.block_size, LayoutUnit());
+      }
 
       // Even an empty (0x0) fragment contributes to the inflow-bounds.
       if (!inflow_bounds_)

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

@@ -199,7 +199,13 @@ scoped_refptr<const NGLayoutResult> NGSimplifiedLayoutAlgorithm::Layout() {
     if (!result)
       return nullptr;
 
-    AddChildFragment(child_link, result->PhysicalFragment());
+    const NGMarginStrut end_margin_strut = result->EndMarginStrut();
+    // No margins should pierce outside formatting-context roots.
+    DCHECK(!result->PhysicalFragment().IsFormattingContextRoot() ||
+           end_margin_strut.IsEmpty());
+
+    AddChildFragment(child_link, result->PhysicalFragment(), &end_margin_strut,
+                     result->IsSelfCollapsing());
   }
 
   // Iterate through all our OOF-positioned children and add them as candidates.
@@ -256,7 +262,9 @@ NGSimplifiedLayoutAlgorithm::LayoutWithItemsBuilder() {
 
 void NGSimplifiedLayoutAlgorithm::AddChildFragment(
     const NGLink& old_fragment,
-    const NGPhysicalContainerFragment& new_fragment) {
+    const NGPhysicalContainerFragment& new_fragment,
+    const NGMarginStrut* margin_strut,
+    bool is_self_collapsing) {
   DCHECK_EQ(old_fragment->Size(), new_fragment.Size());
 
   // Determine the previous position in the logical coordinate system.
@@ -275,7 +283,9 @@ void NGSimplifiedLayoutAlgorithm::AddChildFragment(
   }
 
   // Add the new fragment to the builder.
-  container_builder_.AddChild(new_fragment, child_offset);
+  container_builder_.AddChild(new_fragment, child_offset,
+                              /* inline_container */ nullptr, margin_strut,
+                              is_self_collapsing);
 }
 
 }  // namespace blink

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

@@ -52,7 +52,9 @@ class CORE_EXPORT NGSimplifiedLayoutAlgorithm
 
  private:
   void AddChildFragment(const NGLink& old_fragment,
-                        const NGPhysicalContainerFragment& new_fragment);
+                        const NGPhysicalContainerFragment& new_fragment,
+                        const NGMarginStrut* margin_strut = nullptr,
+                        bool is_self_collapsing = false);
 
   const NGLayoutResult& previous_result_;
   NGBoxStrut border_scrollbar_padding_;

third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-negative-margin-dynamic.html

+<!DOCTYPE html>
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1157299">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<p>Test passes if there is a filled green square.</p>
+<div style="position: relative; width: 100px; height: 100px; box-sizing: border-box; overflow: auto; padding-bottom: 10px;">
+  <div style="background: green; margin-bottom: 10px;">
+    <div style="height: 100px; margin-bottom: -20px;"></div>
+  </div>
+  <div id="target" style="position: absolute; top: 0; left: 0;"></div>
+</div>
+<script>
+document.body.offsetTop;
+document.getElementById('target').style.top = '10px';
+</script>

third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-negative-margin.html

+<!DOCTYPE html>
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1157299">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<p>Test passes if there is a filled green square.</p>
+<div style="width: 100px; height: 100px; box-sizing: border-box; overflow: auto; padding-bottom: 10px;">
+  <div style="height: 100px; background: green; margin-bottom: -10px;"></div>
+</div>