[LayoutNG] Fix bug with positioning fragmented oof elements

In cases where an oof element's static position was taken in a later
fragmentainer than the start of its containing block, we would
calculate the wrong start offset and starting fragmentainer for the
oof.

To fix this, instead of passing fragmentainer_consumed_block_size
(which is the consumed column block size up to the column that the
static position was taken in), update the containing_block_offset to
be the offset from the start of the containing block to the root
fragmentation context, as well as update the static position to be
relative to this adjusted containing_block_offset.

Bug: 1079031
Change-Id: I48c08e7debd291fa3acfa77af0bb6d242cedbbd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422213
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#809917}

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

@@ -245,6 +245,15 @@ class CORE_EXPORT NGBoxFragmentBuilder final
   bool DidBreakSelf() const { return did_break_self_; }
   void SetDidBreakSelf() { did_break_self_ = true; }
 
+  // Store the previous break token, if one exists.
+  void SetPreviousBreakToken(
+      scoped_refptr<const NGBlockBreakToken> break_token) {
+    previous_break_token_ = std::move(break_token);
+  }
+  const NGBlockBreakToken* PreviousBreakToken() const {
+    return previous_break_token_.get();
+  }
+
   // Return true if we need to break before or inside any child, doesn't matter
   // if it's in-flow or not. As long as there are only breaks in parallel flows,
   // we may continue layout, but when we're done, we'll need to create a break
@@ -587,6 +596,8 @@ class CORE_EXPORT NGBoxFragmentBuilder final
 
   std::unique_ptr<NGMathMLPaintInfo> mathml_paint_info_;
 
+  scoped_refptr<const NGBlockBreakToken> previous_break_token_;
+
 #if DCHECK_IS_ON()
   // Describes what size_.block_size represents; either the size of a single
   // fragment (false), or the size of all fragments for a node (true).

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

@@ -78,15 +78,16 @@ void NGContainerFragmentBuilder::PropagateChildData(
             descendant.containing_block_offset, PhysicalSize());
         if (!child.IsFragmentainerBox())
           containing_block_offset.block_offset += child_offset.block_offset;
+        if (IsBlockFragmentationContextRoot()) {
+          containing_block_offset.block_offset +=
+              fragmentainer_consumed_block_size_;
+        }
 
         NGLogicalStaticPosition static_position =
             descendant.static_position.ConvertToLogical(empty_outer_size);
         oof_positioned_fragmentainer_descendants_.emplace_back(
             descendant.node, static_position, descendant.inline_container,
             /* needs_block_offset_adjustment */ false,
-            IsBlockFragmentationContextRoot()
-                ? fragmentainer_consumed_block_size_
-                : LayoutUnit(),
             containing_block_offset, containing_block_fragment);
       }
     }

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

@@ -195,6 +195,7 @@ void SetupFragmentBuilderForFragmentation(
     const NGBlockBreakToken* previous_break_token,
     NGBoxFragmentBuilder* builder) {
   builder->SetHasBlockFragmentation();
+  builder->SetPreviousBreakToken(previous_break_token);
 
   // The whereabouts of our container's so far best breakpoint is none of our
   // business, but we store its appeal, so that we don't look for breakpoints

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

@@ -321,9 +321,8 @@ NGOutOfFlowLayoutPart::GetContainingBlockInfo(
         LogicalOffset(border.inline_start, border.block_start);
     container_offset += candidate.containing_block_offset;
 
-    ContainingBlockInfo containing_block_info{
-        style.Direction(), content_size, content_size, container_offset,
-        candidate.fragmentainer_consumed_block_size};
+    ContainingBlockInfo containing_block_info{style.Direction(), content_size,
+                                              content_size, container_offset};
 
     return containing_blocks_map_
         .insert(containing_block, containing_block_info)
@@ -488,6 +487,19 @@ void NGOutOfFlowLayoutPart::LayoutCandidates(
       if (IsContainingBlockForCandidate(candidate) &&
           (!only_layout || layout_box == only_layout)) {
         if (container_space_.HasBlockFragmentation()) {
+          // If the containing block is fragmented, adjust the offset to be from
+          // the first containing block fragment to the fragmentation context
+          // root. Also, adjust the static position to be relative to the
+          // adjusted containing block offset.
+          if (const auto* previous_break_token =
+                  container_builder_->PreviousBreakToken()) {
+            LayoutUnit previous_consumed_block_size =
+                previous_break_token->ConsumedBlockSize();
+            candidate.containing_block_offset.block_offset -=
+                previous_consumed_block_size;
+            candidate.static_position.offset.block_offset +=
+                previous_consumed_block_size;
+          }
           container_builder_->AddOutOfFlowFragmentainerDescendant(candidate);
           continue;
         }
@@ -1210,9 +1222,6 @@ void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
     WritingMode default_writing_mode,
     wtf_size_t* start_index,
     LogicalOffset* offset) const {
-  LayoutUnit block_offset_from_root =
-      offset->block_offset + container_info.fragmentainer_consumed_block_size;
-
   wtf_size_t child_index = 0;
   // The sum of all previous fragmentainers' block size.
   LayoutUnit used_block_size;
@@ -1229,9 +1238,9 @@ void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
                                      .block_size;
       current_max_block_size += fragmentainer_block_size;
 
-      if (block_offset_from_root < current_max_block_size) {
+      if (offset->block_offset < current_max_block_size) {
         *start_index = child_index;
-        offset->block_offset = block_offset_from_root - used_block_size;
+        offset->block_offset -= used_block_size;
         return;
       }
       used_block_size = current_max_block_size;
@@ -1240,7 +1249,7 @@ void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
   }
   // If the right fragmentainer hasn't been found yet, the OOF element will
   // start its layout in a proxy fragment.
-  LayoutUnit remaining_block_offset = block_offset_from_root - used_block_size;
+  LayoutUnit remaining_block_offset = offset->block_offset - used_block_size;
 
   // If we are a new fragment and are separated from other columns by a
   // spanner, compute the correct fragmentainer_block_size.

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

@@ -87,9 +87,6 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
     // Offset of the container's padding-box.
     LogicalOffset container_offset;
 
-    // The block size consumed by all previous fragmentainers.
-    LayoutUnit fragmentainer_consumed_block_size;
-
     LogicalSize ContentSize(EPosition position) const {
       return position == EPosition::kAbsolute ? content_size_for_absolute
                                               : content_size_for_fixed;

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

@@ -727,8 +727,6 @@ TEST_F(NGOutOfFlowLayoutPartTest, PositionedFragmentationAndColumnSpanners) {
       )HTML");
   String dump = DumpFragmentTree(GetElementById("container"));
 
-  // TODO(almaher): The height of fragmentainer `offset:0,30 size:492x10` might
-  // need to be updated in your CL about column spanners.
   String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:1000x40
     offset:0,0 size:1000x40
@@ -964,14 +962,13 @@ TEST_F(NGOutOfFlowLayoutPartTest,
       )HTML");
   String dump = DumpFragmentTree(GetElementById("container"));
 
-  // TODO(1079031): With top set to 0px, the abspos should start in the first
-  // column with an offset of (0,0).
   String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:1000x40
     offset:0,0 size:1000x40
       offset:0,0 size:492x10
         offset:0,0 size:30x10
           offset:0,0 size:30x10
+        offset:0,0 size:5x10
       offset:508,0 size:492x10
         offset:0,0 size:30x10
           offset:0,0 size:30x10
@@ -986,7 +983,7 @@ TEST_F(NGOutOfFlowLayoutPartTest,
       offset:1016,10 size:492x30
         offset:0,0 size:5x30
       offset:1524,10 size:492x30
-        offset:0,0 size:5x20
+        offset:0,0 size:5x10
 )DUMP";
   EXPECT_EQ(expectation, dump);
 }
@@ -1022,8 +1019,6 @@ TEST_F(NGOutOfFlowLayoutPartTest,
       )HTML");
   String dump = DumpFragmentTree(GetElementById("container"));
 
-  // TODO(1079031): With top set to 25px, the abspos should start in the third
-  // column with an offset of (0,5).
   String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:1000x40
     offset:0,0 size:1000x40
@@ -1035,11 +1030,9 @@ TEST_F(NGOutOfFlowLayoutPartTest,
           offset:0,0 size:30x10
       offset:0,10 size:1000x0
       offset:0,10 size:492x30
-        offset:0,15 size:5x15
+        offset:0,5 size:5x25
       offset:508,10 size:492x30
-        offset:0,0 size:5x30
-      offset:1016,10 size:492x30
-        offset:0,0 size:5x5
+        offset:0,0 size:5x25
 )DUMP";
   EXPECT_EQ(expectation, dump);
 }

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

@@ -28,9 +28,8 @@ namespace blink {
 // This needs its static position [1] to be placed correctly in its containing
 // block. And in the case of fragmentation, this also needs the containing block
 // fragment to be placed correctly within the fragmentation context root. In
-// addition, the block size consumed by previous fragmentainers and the
-// containing block offset are needed to compute the start offset and the
-// initial fragmentainer of an out-of-flow positioned-node.
+// addition, the containing block offset is needed to compute the start offset
+// and the initial fragmentainer of an out-of-flow positioned-node.
 //
 // This is struct is allowed to be stored/persisted.
 //
@@ -40,7 +39,6 @@ struct CORE_EXPORT NGPhysicalOutOfFlowPositionedNode {
   NGPhysicalStaticPosition static_position;
   // Continuation root of the optional inline container.
   const LayoutInline* inline_container;
-  const LayoutUnit fragmentainer_consumed_block_size;
   PhysicalOffset containing_block_offset;
   scoped_refptr<const NGPhysicalContainerFragment> containing_block_fragment;
 
@@ -48,14 +46,12 @@ struct CORE_EXPORT NGPhysicalOutOfFlowPositionedNode {
       NGBlockNode node,
       NGPhysicalStaticPosition static_position,
       const LayoutInline* inline_container = nullptr,
-      LayoutUnit fragmentainer_consumed_block_size = LayoutUnit(),
       PhysicalOffset containing_block_offset = PhysicalOffset(),
       scoped_refptr<const NGPhysicalContainerFragment>
           containing_block_fragment = nullptr)
       : node(node),
         static_position(static_position),
         inline_container(inline_container),
-        fragmentainer_consumed_block_size(fragmentainer_consumed_block_size),
         containing_block_offset(containing_block_offset),
         containing_block_fragment(std::move(containing_block_fragment)) {
     DCHECK(!inline_container ||
@@ -74,7 +70,6 @@ struct NGLogicalOutOfFlowPositionedNode {
   NGLogicalStaticPosition static_position;
   // Continuation root of the optional inline container.
   const LayoutInline* inline_container;
-  const LayoutUnit fragmentainer_consumed_block_size;
   bool needs_block_offset_adjustment;
   LogicalOffset containing_block_offset;
   scoped_refptr<const NGPhysicalContainerFragment> containing_block_fragment;
@@ -84,14 +79,12 @@ struct NGLogicalOutOfFlowPositionedNode {
       NGLogicalStaticPosition static_position,
       const LayoutInline* inline_container = nullptr,
       bool needs_block_offset_adjustment = false,
-      LayoutUnit fragmentainer_consumed_block_size = LayoutUnit(),
       LogicalOffset containing_block_offset = LogicalOffset(),
       scoped_refptr<const NGPhysicalContainerFragment>
           containing_block_fragment = nullptr)
       : node(node),
         static_position(static_position),
         inline_container(inline_container),
-        fragmentainer_consumed_block_size(fragmentainer_consumed_block_size),
         needs_block_offset_adjustment(needs_block_offset_adjustment),
         containing_block_offset(containing_block_offset),
         containing_block_fragment(std::move(containing_block_fragment)) {

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

@@ -219,7 +219,6 @@ NGPhysicalBoxFragment::RareData::RareData(NGBoxFragmentBuilder* builder,
         descendant.node,
         descendant.static_position.ConvertToPhysical(converter),
         descendant.inline_container,
-        descendant.fragmentainer_consumed_block_size,
         descendant.containing_block_offset.ConvertToPhysical(
             builder->Style().GetWritingDirection(), size,
             descendant.containing_block_fragment