[LayoutNG] Skip column spanners when laying out OOF positioned elements

When adding fragmented OOF positioned layout results to a multi-col,
we need to skip over any spanning fragments.

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

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

@@ -605,6 +605,11 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::LayoutCandidate(
 
 void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendants(
     Vector<NGLogicalOutOfFlowPositionedNode>* descendants) {
+  original_column_block_size_ =
+      ShrinkLogicalSize(container_builder_->InitialBorderBoxSize(),
+                        container_builder_->BorderScrollbarPadding())
+          .block_size;
+
   while (descendants->size() > 0) {
     for (auto& descendant : *descendants) {
       LayoutFragmentainerDescendant(descendant);
@@ -629,8 +634,6 @@ void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendants(
 
 void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendant(
     const NGLogicalOutOfFlowPositionedNode& descendant) {
-  // TODO(almaher): Properly implement the layout algorithm for fragmented
-  // positioned elements.
   NGBlockNode node = descendant.node;
   const NGPhysicalContainerFragment* containing_block_fragment =
       descendant.containing_block_fragment.get();
@@ -848,8 +851,9 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
   // Determine in which fragmentainer this OOF element will start its layout and
   // adjust the offset to be relative to that fragmentainer.
   wtf_size_t start_index = 0;
+  wtf_size_t num_children = container_builder_->Children().size();
   if (is_fragmentainer_descendant) {
-    DCHECK_GT(container_builder_->Children().size(), 0u);
+    DCHECK_GT(num_children, 0u);
     ComputeStartFragmentIndexAndRelativeOffset(
         container_info, default_writing_mode, &start_index, &offset);
   }
@@ -907,8 +911,15 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
   do {
     if (break_token) {
       layout_result = nullptr;
-      // TODO(almaher): You might want to skip spanning fragments here.
       start_index++;
+
+      // Skip over spanning fragments when finding the next fragmentainer to add
+      // an OOF result to.
+      while (start_index < num_children &&
+             !container_builder_->Children()[start_index]
+                  .fragment->IsFragmentainerBox()) {
+        start_index++;
+      }
       offset.block_offset = LayoutUnit();
     }
 
@@ -1048,18 +1059,21 @@ void NGOutOfFlowLayoutPart::AddOOFResultsToFragmentainer(
     }
     DCHECK_EQ(index, container_builder_->Children().size());
   }
+  const NGConstraintSpace& space = GetFragmentainerConstraintSpace(index);
 
-  // TODO(almaher): Ensure that we are skipping over spanning fragments when
-  // creating new fragmentainers and when calculating |num_children|.
-  const auto& fragmentainer =
-      is_new_fragment ? container_builder_->Children()[num_children - 1]
-                      : container_builder_->Children()[index];
+  // If we are a new fragment, find a non-spanner fragmentainer as a basis.
+  while (
+      index >= num_children ||
+      !container_builder_->Children()[index].fragment->IsFragmentainerBox()) {
+    DCHECK_GT(num_children, 0u);
+    index--;
+  }
 
+  const auto& fragmentainer = container_builder_->Children()[index];
   DCHECK(fragmentainer.fragment->IsFragmentainerBox());
   const NGBlockNode& node = container_builder_->Node();
   const auto& fragment =
       To<NGPhysicalBoxFragment>(*fragmentainer.fragment.get());
-  const NGConstraintSpace& space = GetFragmentainerConstraintSpace(index);
   NGFragmentGeometry fragment_geometry =
       CalculateInitialFragmentGeometry(space, node);
   NGLayoutAlgorithmParams params(node, fragment_geometry, space,
@@ -1112,26 +1126,43 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
 
   wtf_size_t num_children = container_builder_->Children().size();
   bool is_new_fragment = index >= num_children;
+  bool is_first_fragmentainer = index == 0;
+
+  // If we are a new fragment, find a non-spanner fragmentainer to base our
+  // constraint space off of.
+  while (
+      index >= num_children ||
+      !container_builder_->Children()[index].fragment->IsFragmentainerBox()) {
+    DCHECK_GT(num_children, 0u);
+    index--;
+  }
 
-  // TODO(almaher): Ensure that we are skipping over spanning fragments when
-  // creating the constraint space for new fragmentainers and when calculating
-  // |num_children|.
-  const auto& fragmentainer =
-      is_new_fragment ? container_builder_->Children()[num_children - 1]
-                      : container_builder_->Children()[index];
+  const auto& fragmentainer = container_builder_->Children()[index];
   DCHECK(fragmentainer.fragment->IsFragmentainerBox());
   const auto& fragment =
       To<NGPhysicalBoxFragment>(*fragmentainer.fragment.get());
   const WritingMode container_writing_mode =
       container_builder_->Style().GetWritingMode();
+  LogicalSize column_size =
+      fragment.Size().ConvertToLogical(container_writing_mode);
+
+  // If we are a new fragment and are separated from other columns by a
+  // spanner, compute the correct column block size to use.
+  if (is_new_fragment && index != num_children - 1 &&
+      original_column_block_size_ != kIndefiniteSize &&
+      !container_builder_->Children()[index + 1]
+           .fragment->IsFragmentainerBox()) {
+    // TODO(almaher): Should we take the consumed column block size into
+    // account?
+    column_size.block_size = original_column_block_size_;
+  }
 
   // TODO(bebeaudr): Need to handle different fragmentation types. It won't
   // always be multi-column.
   NGConstraintSpace fragmentainer_constraint_space =
       CreateConstraintSpaceForColumns(
           *container_builder_->ConstraintSpace(), container_writing_mode,
-          fragment.Size().ConvertToLogical(container_writing_mode),
-          /* is_first_fragmentainer */ index == 0, /* balance_columns */ false);
+          column_size, is_first_fragmentainer, /* balance_columns */ false);
 
   return fragmentainer_constraint_space_map_
       .insert(stored_index, fragmentainer_constraint_space)
@@ -1190,11 +1221,20 @@ 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;
+
+  // If we are a new fragment and are separated from other columns by a
+  // spanner, compute the correct fragmentainer_block_size.
+  if (original_column_block_size_ != kIndefiniteSize &&
+      !container_builder_->Children()[child_index - 1]
+           .fragment->IsFragmentainerBox()) {
+    // TODO(almaher): Should we take the consumed column block size into
+    // account?
+    fragmentainer_block_size = original_column_block_size_;
+  }
+
   wtf_size_t additional_fragment_count =
       int(std::floorf(remaining_block_offset / fragmentainer_block_size));
   *start_index = child_index + additional_fragment_count;
-  // TODO(almaher): This might need to be updated in the case where we add proxy
-  // fragments directly after a spanner.
   offset->block_offset = remaining_block_offset -
                          additional_fragment_count * fragmentainer_block_size;
 }

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

@@ -163,6 +163,10 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
       fragmentainer_descendant_results_;
   const WritingMode writing_mode_;
   LayoutUnit column_inline_progression_ = kIndefiniteSize;
+  // The block size of the multi-column (before adjustment for spanners, etc.)
+  // This is used to calculate the column size of any newly added proxy
+  // fragments when handling fragmentation for abspos elements.
+  LayoutUnit original_column_block_size_ = kIndefiniteSize;
   bool is_absolute_container_;
   bool is_fixed_container_;
   bool allow_first_tier_oof_cache_;

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

@@ -19,20 +19,20 @@ NGSimplifiedOOFLayoutAlgorithm::NGSimplifiedOOFLayoutAlgorithm(
     : NGLayoutAlgorithm(params),
       writing_direction_(Style().GetWritingDirection()) {
   DCHECK(fragment.IsFragmentainerBox());
+  DCHECK(params.space.HasKnownFragmentainerBlockSize());
 
   container_builder_.SetBoxType(fragment.BoxType());
-  LayoutUnit old_block_size =
-      NGFragment(writing_direction_.GetWritingMode(), fragment).BlockSize();
-  container_builder_.SetFragmentBlockSize(old_block_size);
-
-  // We need the previous physical container size to calculate the position of
-  // any child fragments.
-  previous_physical_container_size_ = fragment.Size();
+  container_builder_.SetFragmentBlockSize(
+      params.space.FragmentainerBlockSize());
 
   // Don't apply children to new fragments.
   if (is_new_fragment)
     return;
 
+  // We need the previous physical container size to calculate the position of
+  // any child fragments.
+  previous_physical_container_size_ = fragment.Size();
+
   // The OOF fragments need to be added after the already existing child
   // fragments. Add them now so they are added before we append the OOF results.
   for (const auto& child_link : fragment.Children()) {

third_party/blink/web_tests/TestExpectations

@@ -1093,7 +1093,7 @@ crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spann
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-unconstrained-width.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/relpos-becomes-static-has-abspos.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/remove-column-content-next-to-abspos-between-spanners.html [ Failure ]
-crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/static-becomes-relpos-has-abspos.html [ Crash Failure ]
+crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/dynamic/static-becomes-relpos-has-abspos.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/event-offset-in-nested.html [ Failure ]
 crbug.com/875235 virtual/layout_ng_block_frag/fast/multicol/fieldset-as-multicol.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/file-upload-as-multicol.html [ Failure ]