[LayoutNG] Lay out the pending OOFs of any inner multicols

Loop through the inner multicols collected in CL:2612392 once we've hit
the outermost fragmentation context, and lay out the pending OOF
positioned nodes associated with those multicols.

Currently, we are laying out the OOFs inside the outermost
fragmentation context rather than the inner mutlicols. This results in
incorrect offsets. However, this CL allows us to test that everything
is bubbling up correctly and layout is happening at the correct time.

In follow-up changes, the layout of these OOF nodes will move from
the outer context to the inner.

Note: I also updated how we were converting to logical offsets
in the normal case of fragmented OOF nodes since I realized those
didn't look quite right (although it doesn't change the result of any
of the existing tests).

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

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

@@ -283,10 +283,10 @@ struct HashTraits<blink::NGBlockNode>
   }
   static blink::NGBlockNode EmptyValue() { return nullptr; }
   static void ConstructDeletedValue(blink::NGBlockNode& slot, bool) {
-    slot = nullptr;
+    slot = blink::NGBlockNode(reinterpret_cast<blink::LayoutBox*>(-1));
   }
   static bool IsDeletedValue(const blink::NGBlockNode& value) {
-    return IsEmptyValue(value);
+    return value.GetLayoutBox() == reinterpret_cast<blink::LayoutBox*>(-1);
   }
 };
 

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

@@ -304,8 +304,6 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::Layout() {
     // out at the outermost context. If this multicol has OOF positioned
     // elements pending layout, store its node for later use.
     if (container_builder_.HasOutOfFlowFragmentainerDescendants()) {
-      // TODO(almaher): Run layout on the pending OOFs once we hit the
-      // outermost fragmentation context.
       container_builder_.AddMulticolWithPendingOOFs(Node());
     }
   }

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

@@ -312,9 +312,6 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
   }
 
   const WritingModeConverter converter(GetWritingDirection(), fragment.Size());
-  const WritingModeConverter empty_outer_size(GetWritingDirection(),
-                                              PhysicalSize());
-
   const auto& out_of_flow_fragmentainer_descendants =
       box_fragment->OutOfFlowPositionedFragmentainerDescendants();
   for (const auto& descendant : out_of_flow_fragmentainer_descendants) {
@@ -323,8 +320,8 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
     if (!containing_block_fragment)
       containing_block_fragment = box_fragment;
 
-    LogicalOffset containing_block_offset =
-        converter.ToLogical(descendant.containing_block_offset, PhysicalSize());
+    LogicalOffset containing_block_offset = converter.ToLogical(
+        descendant.containing_block_offset, containing_block_fragment->Size());
     if (!fragment.IsFragmentainerBox())
       containing_block_offset.block_offset += offset.block_offset;
     if (IsBlockFragmentationContextRoot()) {
@@ -332,8 +329,12 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
           fragmentainer_consumed_block_size_;
     }
 
+    // The static position should remain relative to its containing block
+    // fragment.
+    const WritingModeConverter containing_block_converter(
+        GetWritingDirection(), containing_block_fragment->Size());
     NGLogicalStaticPosition static_position =
-        descendant.static_position.ConvertToLogical(empty_outer_size);
+        descendant.static_position.ConvertToLogical(containing_block_converter);
     AddOutOfFlowFragmentainerDescendant(
         {descendant.node, static_position, descendant.inline_container,
          /* needs_block_offset_adjustment */ false, containing_block_offset,

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

@@ -135,14 +135,23 @@ NGOutOfFlowLayoutPart::NGOutOfFlowLayoutPart(
 
 void NGOutOfFlowLayoutPart::Run(const LayoutBox* only_layout) {
   if (container_builder_->IsBlockFragmentationContextRoot() &&
-      !has_block_fragmentation_ &&
-      container_builder_->HasOutOfFlowFragmentainerDescendants()) {
-    Vector<NGLogicalOutOfFlowPositionedNode> fragmentainer_descendants;
-    container_builder_->SwapOutOfFlowFragmentainerDescendants(
-        &fragmentainer_descendants);
-
-    if (!fragmentainer_descendants.IsEmpty())
+      !has_block_fragmentation_) {
+    if (container_builder_->HasOutOfFlowFragmentainerDescendants()) {
+      Vector<NGLogicalOutOfFlowPositionedNode> fragmentainer_descendants;
+      container_builder_->SwapOutOfFlowFragmentainerDescendants(
+          &fragmentainer_descendants);
+      DCHECK(!fragmentainer_descendants.IsEmpty());
       LayoutFragmentainerDescendants(&fragmentainer_descendants);
+    }
+
+    if (container_builder_->HasMulticolsWithPendingOOFs()) {
+      HashSet<NGBlockNode> multicols_with_pending_oofs;
+      container_builder_->SwapMulticolsWithPendingOOFs(
+          &multicols_with_pending_oofs);
+      DCHECK(!multicols_with_pending_oofs.IsEmpty());
+      for (const NGBlockNode& multicol : multicols_with_pending_oofs)
+        LayoutOOFsInMulticol(multicol);
+    }
   }
 
   const LayoutObject* current_container = container_builder_->GetLayoutObject();
@@ -630,6 +639,60 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::LayoutCandidate(
   } while (true);
 }
 
+// TODO(almaher): Look into moving this to NGColumnLayoutAlgorithm instead.
+void NGOutOfFlowLayoutPart::LayoutOOFsInMulticol(const NGBlockNode& multicol) {
+  Vector<NGLogicalOutOfFlowPositionedNode> oof_nodes_to_layout;
+
+  // Accumulate all of the pending OOF positioned nodes that are stored inside
+  // |multicol|.
+  for (auto& multicol_fragment : multicol.GetLayoutBox()->PhysicalFragments()) {
+    const NGPhysicalBoxFragment* multicol_box_fragment =
+        To<NGPhysicalBoxFragment>(&multicol_fragment);
+    if (!multicol_box_fragment
+             ->HasOutOfFlowPositionedFragmentainerDescendants())
+      continue;
+
+    WritingDirectionMode writing_direction =
+        multicol_box_fragment->Style().GetWritingDirection();
+    const WritingModeConverter converter(writing_direction,
+                                         multicol_box_fragment->Size());
+
+    // Convert the OOF fragmentainer descendants to the logical coordinate space
+    // and store the resulting nodes inside |oof_nodes_to_layout|.
+    for (const auto& descendant :
+         multicol_box_fragment->OutOfFlowPositionedFragmentainerDescendants()) {
+      const NGPhysicalContainerFragment* containing_block_fragment =
+          descendant.containing_block_fragment.get();
+      LogicalOffset containing_block_offset =
+          converter.ToLogical(descendant.containing_block_offset,
+                              containing_block_fragment->Size());
+
+      // The static position should remain relative to its containing block
+      // fragment.
+      const WritingModeConverter containing_block_converter(
+          writing_direction, containing_block_fragment->Size());
+      NGLogicalStaticPosition static_position =
+          descendant.static_position.ConvertToLogical(
+              containing_block_converter);
+
+      NGLogicalOutOfFlowPositionedNode node = {
+          descendant.node,
+          static_position,
+          descendant.inline_container,
+          /* needs_block_offset_adjustment */ false,
+          containing_block_offset,
+          containing_block_fragment};
+      oof_nodes_to_layout.push_back(node);
+    }
+  }
+  DCHECK(!oof_nodes_to_layout.IsEmpty());
+
+  // TODO(almaher): This lays out the OOF nodes in the outer fragmentation
+  // context. We want to lay these out inside the column children of |multicol|
+  // instead.
+  LayoutFragmentainerDescendants(&oof_nodes_to_layout);
+}
+
 void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendants(
     Vector<NGLogicalOutOfFlowPositionedNode>* descendants) {
   original_column_block_size_ =

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

@@ -102,6 +102,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
       const NGLogicalOutOfFlowPositionedNode&,
       const LayoutBox* only_layout);
 
+  void LayoutOOFsInMulticol(const NGBlockNode& multicol);
+
   void LayoutFragmentainerDescendants(
       Vector<NGLogicalOutOfFlowPositionedNode>* descendants);
 

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

@@ -1305,9 +1305,7 @@ TEST_F(NGOutOfFlowLayoutPartTest,
 }
 
 // Fragmented OOF element inside a nested multi-column.
-// TODO(almaher): Re-enable once layout is run on the pending OOFs of inner
-// multicols inside a nested fragmentation context.
-TEST_F(NGOutOfFlowLayoutPartTest, DISABLED_AbsposNestedFragmentation) {
+TEST_F(NGOutOfFlowLayoutPartTest, AbsposNestedFragmentation) {
   SetBodyInnerHTML(
       R"HTML(
       <style>
@@ -1335,9 +1333,10 @@ TEST_F(NGOutOfFlowLayoutPartTest, DISABLED_AbsposNestedFragmentation) {
       )HTML");
   String dump = DumpFragmentTree(GetElementById("container"));
 
-  // TODO(almaher): There should be two abspos fragments with height 60 in the
-  // first outer column, and two with height 100/30 in the second outer column.
-  // There should not be a third outer column.
+  // TODO(almaher): The abspos element should be placed in the inner multicol
+  // rather than the outer multicol. The offset is also incorrectly computed due
+  // to the fact that the containing block offset is now relative to the inner
+  // multicol rather than the outer.
   String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
   offset:unplaced size:1000x100
     offset:0,0 size:1000x100
@@ -1350,7 +1349,7 @@ TEST_F(NGOutOfFlowLayoutPartTest, DISABLED_AbsposNestedFragmentation) {
           offset:250,0 size:250x60
             offset:0,0 size:55x60
               offset:0,0 size:25x60
-        offset:0,40 size:5x60
+        offset:0,0 size:5x100
       offset:500,0 size:500x100
         offset:0,0 size:500x100
           offset:0,0 size:250x100
@@ -1361,7 +1360,62 @@ TEST_F(NGOutOfFlowLayoutPartTest, DISABLED_AbsposNestedFragmentation) {
               offset:0,0 size:25x30
         offset:0,0 size:5x100
       offset:1000,0 size:500x100
-        offset:0,0 size:5x90
+        offset:0,0 size:5x50
+)DUMP";
+  EXPECT_EQ(expectation, dump);
+}
+
+// Test the static position of a fragmented OOF element inside a nested
+// multi-column.
+TEST_F(NGOutOfFlowLayoutPartTest, AbsposNestedFragmentationStaticPos) {
+  SetBodyInnerHTML(
+      R"HTML(
+      <style>
+        .multicol {
+          columns:2; column-fill:auto; column-gap:0px;
+        }
+        .rel {
+          position: relative; width:55px;
+        }
+        .abs {
+          position:absolute; width:5px; height:70px;
+        }
+      </style>
+      <div id="container">
+        <div class="multicol" id="outer" style="height:100px;">
+          <div class="multicol" id="inner">
+            <div class="rel">
+              <div style="height:250px; width:25px;"></div>
+              <div class="abs"></div>
+            </div>
+          </div>
+        </div>
+      </div>
+      )HTML");
+  String dump = DumpFragmentTree(GetElementById("container"));
+
+  // TODO(almaher): The abspos element should be placed in the inner multicol
+  // rather than the outer multicol. The static offset is also incorrectly
+  // computed due to the fact that the containing block offset is now relative
+  // to the inner multicol rather than the outer.
+  String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
+  offset:unplaced size:1000x100
+    offset:0,0 size:1000x100
+      offset:0,0 size:500x100
+        offset:0,0 size:500x100
+          offset:0,0 size:250x100
+            offset:0,0 size:55x100
+              offset:0,0 size:25x100
+          offset:250,0 size:250x100
+            offset:0,0 size:55x100
+              offset:0,0 size:25x100
+        offset:0,50 size:5x50
+      offset:500,0 size:500x100
+        offset:0,0 size:500x100
+          offset:0,0 size:250x100
+            offset:0,0 size:55x50
+              offset:0,0 size:25x50
+        offset:0,0 size:5x20
 )DUMP";
   EXPECT_EQ(expectation, dump);
 }