Avoid double relpos offset in NGSimplifiedOOFLayoutAlgorithm.

The offsets stored in NGLink are "paint offsets", which include any
offsets caused by relative positioning. The fragment builder, on the
other hand, expects "layout offsets", which shouldn't include that. The
fragment builder adds the relative offset on its own at the right time.

I struggled with coming up with a very sensible test, since
fragmentation of out-of-flow descendants is incorrectly affected by the
inset specified on their relatively positioned containing block (see
crbug.com/1158387 and crbug.com/1158756 ).

Bug: 1079031
Change-Id: I803dd89a11be2ba9179e20ea06d0b709d08ab534
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593005Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837692}

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

@@ -7,13 +7,14 @@
 
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/layout/geometry/logical_size.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
+#include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/text/text_direction.h"
 
 namespace blink {
 
-class ComputedStyle;
 class NGConstraintSpace;
-class NGPhysicalBoxFragment;
 
 // Implements relative positioning:
 // https://www.w3.org/TR/css-position-3/#rel-pos
@@ -32,6 +33,22 @@ CORE_EXPORT LogicalOffset
 ComputeRelativeOffsetForInline(const NGConstraintSpace& space,
                                const ComputedStyle& child_style);
 
+// Un-apply any offset caused by relative positioning. When re-using a previous
+// fragment's offset (from the cache), we need to convert from "paint offset" to
+// "layout offset" before re-adding the fragment, in order to get overflow
+// calculation right.
+inline void RemoveRelativeOffset(const NGBoxFragmentBuilder& builder,
+                                 const NGPhysicalFragment& fragment,
+                                 LogicalOffset* offset) {
+  if (fragment.Style().GetPosition() != EPosition::kRelative)
+    return;
+  if (const auto* box_fragment = DynamicTo<NGPhysicalBoxFragment>(&fragment)) {
+    *offset -= ComputeRelativeOffsetForBoxFragment(
+        *box_fragment, builder.GetWritingDirection(),
+        builder.ChildAvailableSize());
+  }
+}
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_NG_RELATIVE_UTILS_H_

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

@@ -271,14 +271,7 @@ void NGSimplifiedLayoutAlgorithm::AddChildFragment(
                            previous_physical_container_size_)
           .ToLogical(old_fragment.Offset(), new_fragment.Size());
 
-  // Un-apply the relative position offset.
-  if (const auto* box_child = DynamicTo<NGPhysicalBoxFragment>(*old_fragment)) {
-    if (box_child->Style().GetPosition() == EPosition::kRelative) {
-      child_offset -= ComputeRelativeOffsetForBoxFragment(
-          *box_child, ConstraintSpace().GetWritingDirection(),
-          container_builder_.ChildAvailableSize());
-    }
-  }
+  RemoveRelativeOffset(container_builder_, *old_fragment, &child_offset);
 
   // Add the new fragment to the builder.
   container_builder_.AddChild(new_fragment, child_offset,

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

@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/core/layout/ng/ng_fragment.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_relative_utils.h"
 
 namespace blink {
 
@@ -118,6 +119,8 @@ void NGSimplifiedOOFLayoutAlgorithm::AddChildFragment(const NGLink& child) {
                            previous_physical_container_size_)
           .ToLogical(child.Offset(), fragment->Size());
 
+  RemoveRelativeOffset(container_builder_, *fragment, &child_offset);
+
   // Add the fragment to the builder.
   container_builder_.AddChild(*fragment, child_offset);
 }

third_party/blink/web_tests/external/wpt/css/css-break/abspos-in-offset-relpos.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-gap:0; column-fill:auto; width:100px; height:100px; background:red;">
+  <div style="height:0;">
+    <div style="height:100px;"></div>
+    <!-- Fill the second column, in a parallel flow (overflow): -->
+    <div style="height:100px; background:green;"></div>
+  </div>
+  <div style="position:relative; top:25px; height:75px; background:red;">
+    <!-- Fill the first column: -->
+    <div style="position:absolute; top:-25px; width:100%; height:100px; background:green;"></div>
+  </div>
+</div>