Always add positioned objects to Legacy OOF container.

Add LayoutNGBlockFlow::CachedLayoutResultForTesting method to enable 
fragment access from unit tests.

# Reason for patch:

Legacy has a fast layout code path that does not call
LayoutNGBlockFlow::UpdateBlockLayout. This path updates 
overflow, and position of OOF descendants. Because of this, 
Legacy containers must know about all OOF descendants.

Existing code did not let Legacy containers know about NG OOF 
descendants. This CL fixes this.

# Unit tests problems.

This patch will affect future webkit_unit_tests. NGBlockNode::Layout 
will DCHECK if done outside of the performLayout cycle if 
NGBlockNode has OOF descendants. 

The core cause is this:
NGBlockNode::CopyFragmentToLegacyLayout calls 
LayoutBox::LayoutPositionedObjects. LayoutPositionedObjects 
asserts that View()->IsInPerformLayout()

This patch fixes four existing failing tests.

1) NGBlockLayoutAlgorithmTest.CollapsingMarginsEmptyBlockWithClearance
Fixed by inspecting LayoutObjects instead of fragments.

2) NGOutOfFlowLayoutPartTest.FixedInsideAbs
Fixed by 

3) NGOutOfFlowLayoutPartTest.OrthogonalWritingMode1
4) NGOutOfFlowLayoutPartTest.OrthogonalWritingMode2
Removed, code already covered by existing LayoutTests.

Bug: 740993
Change-Id: I3a87ef5cdac883517bc3db33e0569668161ff806
Reviewed-on: https://chromium-review.googlesource.com/722760
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510149}

third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc

@@ -140,6 +140,16 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() {
     }
     if (!logical_top.IsAuto())
       static_block = ValueForLength(logical_top, container->LogicalHeight());
+
+    // Legacy static position is relative to padding box.
+    // Convert to border box.
+    NGBoxStrut border_strut =
+        ComputeBorders(*constraint_space, *container_style);
+    if (parent_style->IsLeftToRightDirection())
+      static_inline += border_strut.inline_start;
+    else
+      static_inline -= border_strut.inline_end;
+    static_block += border_strut.block_start;
   }
   if (!parent_style->IsLeftToRightDirection())
     static_inline = containing_block_logical_width - static_inline;
@@ -168,9 +178,9 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() {
   DCHECK(css_container->IsBox());
   NGOutOfFlowLayoutPart(NGBlockNode(ToLayoutBox(css_container)),
                         *constraint_space, *container_style, &container_builder)
-      .Run();
+      .Run(/* update_legacy */ false);
   RefPtr<NGLayoutResult> result = container_builder.ToBoxFragment();
-  // These are the OOF descendants of the current OOF block.
+  // These are the unpositioned OOF descendants of the current OOF block.
   for (NGOutOfFlowPositionedDescendant descendant :
        result->OutOfFlowPositionedDescendants())
     descendant.node.UseOldOutOfFlowPositioning();
@@ -326,6 +336,10 @@ void LayoutNGBlockFlow::SetCachedLayoutResult(
   cached_result_ = layout_result;
 }
 
+RefPtr<NGLayoutResult> LayoutNGBlockFlow::CachedLayoutResultForTesting() {
+  return cached_result_;
+}
+
 void LayoutNGBlockFlow::SetPaintFragment(
     RefPtr<const NGPhysicalFragment> fragment) {
   paint_fragment_ = WTF::MakeUnique<NGPaintFragment>(std::move(fragment));

third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.h

@@ -55,6 +55,8 @@ class CORE_EXPORT LayoutNGBlockFlow : public LayoutBlockFlow {
   void SetCachedLayoutResult(const NGConstraintSpace&,
                              NGBreakToken*,
                              RefPtr<NGLayoutResult>);
+  // For testing only.
+  RefPtr<NGLayoutResult> CachedLayoutResultForTesting();
 
   NGPaintFragment* PaintFragment() const { return paint_fragment_.get(); }
   void SetPaintFragment(RefPtr<const NGPhysicalFragment>);

third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc

@@ -709,9 +709,9 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
     <div id="inflow"></div>
   )HTML");
 
-  const NGPhysicalBoxFragment* zero;
-  const NGPhysicalBoxFragment* abs;
-  const NGPhysicalBoxFragment* inflow;
+  const LayoutNGBlockFlow* zero;
+  const LayoutNGBlockFlow* abs;
+  const LayoutNGBlockFlow* inflow;
   RefPtr<const NGPhysicalBoxFragment> fragment;
   auto run_test = [&](const Length& zero_top_margin_bottom,
                       const Length& zero_inner_margin_top,
@@ -721,36 +721,36 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
     // Set the style of the elements we care about.
     Element* zero_top = GetDocument().getElementById("zero-top");
     zero_top->MutableComputedStyle()->SetMarginBottom(zero_top_margin_bottom);
-
+    zero_top->GetLayoutObject()->SetNeedsLayout("");
     Element* zero_inner = GetDocument().getElementById("zero-inner");
     zero_inner->MutableComputedStyle()->SetMarginTop(zero_inner_margin_top);
     zero_inner->MutableComputedStyle()->SetMarginBottom(
         zero_inner_margin_bottom);
-
+    zero_inner->GetLayoutObject()->SetNeedsLayout("");
     Element* zero_element = GetDocument().getElementById("zero");
     zero_element->MutableComputedStyle()->SetMarginBottom(zero_margin_bottom);
+    zero_element->GetLayoutObject()->SetNeedsLayout("");
 
     Element* inflow_element = GetDocument().getElementById("inflow");
     inflow_element->MutableComputedStyle()->SetMarginTop(inflow_margin_top);
+    inflow_element->GetLayoutObject()->SetNeedsLayout("");
 
-    std::tie(fragment, std::ignore) = RunBlockLayoutAlgorithmForElement(
-        GetDocument().getElementsByTagName("html")->item(0));
-    FragmentChildIterator iterator(fragment.get());
-
-    // body
-    const NGPhysicalBoxFragment* child = iterator.NextChild();
+    GetDocument().View()->UpdateAllLifecyclePhases();
 
+    LayoutNGBlockFlow* child;
     // #float
-    iterator.SetParent(child);
-    child = iterator.NextChild();
-    EXPECT_EQ(NGPhysicalSize(LayoutUnit(50), LayoutUnit(50)), child->Size());
-    EXPECT_EQ(NGPhysicalOffset(LayoutUnit(0), LayoutUnit(0)), child->Offset());
+    child = ToLayoutNGBlockFlow(
+        GetDocument().getElementById("float")->GetLayoutObject());
+    EXPECT_EQ(LayoutSize(LayoutUnit(50), LayoutUnit(50)), child->Size());
+    EXPECT_EQ(LayoutPoint(LayoutUnit(0), LayoutUnit(0)), child->Location());
 
     // We need to manually test the position of #zero, #abs, #inflow.
-    iterator.NextChild();  // #zero-top.
-    zero = iterator.NextChild();
-    inflow = iterator.NextChild();  // NOTE: Layout reordered the fragments.
-    abs = iterator.NextChild();
+    zero = ToLayoutNGBlockFlow(
+        GetDocument().getElementById("zero")->GetLayoutObject());
+    inflow = ToLayoutNGBlockFlow(
+        GetDocument().getElementById("inflow")->GetLayoutObject());
+    abs = ToLayoutNGBlockFlow(
+        GetDocument().getElementById("abs")->GetLayoutObject());
   };
 
   // Base case of no margins.
@@ -762,9 +762,9 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
       /* #inflow margin-top */ Length(0, kFixed));
 
   // #zero, #abs, #inflow should all be positioned at the float.
-  EXPECT_EQ(LayoutUnit(50), zero->Offset().top);
-  EXPECT_EQ(LayoutUnit(50), abs->Offset().top);
-  EXPECT_EQ(LayoutUnit(50), inflow->Offset().top);
+  EXPECT_EQ(LayoutUnit(50), zero->Location().Y());
+  EXPECT_EQ(LayoutUnit(50), abs->Location().Y());
+  EXPECT_EQ(LayoutUnit(50), inflow->Location().Y());
 
   // A margin strut which resolves to -50 (-70 + 20) adjusts the position of
   // #zero to the float clearance.
@@ -777,15 +777,15 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
 
   // #zero is placed at the float, the margin strut is at:
   // 90 = (50 - (-60 + 20)).
-  EXPECT_EQ(LayoutUnit(50), zero->Offset().top);
+  EXPECT_EQ(LayoutUnit(50), zero->Location().Y());
 
   // #abs estimates its position with the margin strut:
   // 40 = (90 + (-70 + 20)).
-  EXPECT_EQ(LayoutUnit(40), abs->Offset().top);
+  EXPECT_EQ(LayoutUnit(40), abs->Location().Y());
 
   // #inflow has similar behaviour to #abs, but includes its margin.
   // 70 = (90 + (-70 + 50))
-  EXPECT_EQ(LayoutUnit(70), inflow->Offset().top);
+  EXPECT_EQ(LayoutUnit(70), inflow->Location().Y());
 
   // A margin strut which resolves to 60 (-10 + 70) means that #zero doesn't
   // get adjusted to clear the float, and we have normal behaviour.
@@ -800,15 +800,15 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
       /* #inflow margin-top */ Length(80, kFixed));
 
   // #zero is placed at 60 (-10 + 70).
-  EXPECT_EQ(LayoutUnit(60), zero->Offset().top);
+  EXPECT_EQ(LayoutUnit(60), zero->Location().Y());
 
   // #abs estimates its position with the margin strut:
   // 50 = (0 + (-20 + 70)).
-  EXPECT_EQ(LayoutUnit(50), abs->Offset().top);
+  EXPECT_EQ(LayoutUnit(50), abs->Location().Y());
 
   // #inflow has similar behaviour to #abs, but includes its margin.
   // 60 = (0 + (-20 + 80))
-  EXPECT_EQ(LayoutUnit(60), inflow->Offset().top);
+  EXPECT_EQ(LayoutUnit(60), inflow->Location().Y());
 
   // #zero-top produces a margin which needs to be ignored, as #zero is
   // affected by clearance, it needs to have layout performed again, starting
@@ -822,11 +822,11 @@ TEST_F(NGBlockLayoutAlgorithmTest, CollapsingMarginsEmptyBlockWithClearance) {
 
   // #zero is placed at the float, the margin strut is at:
   // 40 = (50 - (-10 + 20)).
-  EXPECT_EQ(LayoutUnit(50), zero->Offset().top);
+  EXPECT_EQ(LayoutUnit(50), zero->Location().Y());
 
   // The margin strut is now disjoint, this is placed at:
   // 55 = (40 + (-10 + 25))
-  EXPECT_EQ(LayoutUnit(55), inflow->Offset().top);
+  EXPECT_EQ(LayoutUnit(55), inflow->Location().Y());
 }
 
 // Tests that when auto margins are applied to a new formatting context, they

third_party/WebKit/Source/core/layout/ng/ng_block_node.cc

@@ -340,7 +340,7 @@ void NGBlockNode::CopyFragmentDataToLayoutBox(
       intrinsic_block_size +=
           PreviouslyUsedBlockSpace(constraint_space, physical_fragment);
     }
-    block->LayoutPositionedObjects(true);
+    block->LayoutPositionedObjects(/* relayout_children */ false);
 
     if (flow_thread) {
       UpdateLegacyMultiColumnFlowThread(*this, flow_thread, constraint_space,

third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.cc

@@ -46,7 +46,7 @@ NGOutOfFlowLayoutPart::NGOutOfFlowLayoutPart(
   icb_size_ = container_space.InitialContainingBlockSize();
 }
 
-void NGOutOfFlowLayoutPart::Run() {
+void NGOutOfFlowLayoutPart::Run(bool update_legacy) {
   Vector<NGOutOfFlowPositionedDescendant> descendant_candidates;
   container_builder_->GetAndClearOutOfFlowDescendantCandidates(
       &descendant_candidates);
@@ -58,6 +58,8 @@ void NGOutOfFlowLayoutPart::Run() {
         RefPtr<NGLayoutResult> result = LayoutDescendant(
             candidate.node, candidate.static_position, &offset);
         container_builder_->AddChild(std::move(result), offset);
+        if (update_legacy)
+          candidate.node.UseOldOutOfFlowPositioning();
       } else {
         container_builder_->AddOutOfFlowDescendant(candidate);
       }

third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.h

@@ -31,7 +31,10 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
                         const NGConstraintSpace& container_space,
                         const ComputedStyle& container_style,
                         NGFragmentBuilder* container_builder);
-  void Run();
+
+  // update_legacy will place NG OOF descendants into their Legacy container.
+  // It should be false if OOF descendants have already been placed into Legacy.
+  void Run(bool update_legacy = true);
 
  private:
   RefPtr<NGLayoutResult> LayoutDescendant(NGBlockNode descendant,

third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part_test.cc

@@ -15,9 +15,11 @@ class NGOutOfFlowLayoutPartTest : public RenderingTest {
  public:
   NGOutOfFlowLayoutPartTest() {
     RuntimeEnabledFeatures::SetLayoutNGEnabled(true);
+    RuntimeEnabledFeatures::SetLayoutNGFragmentCachingEnabled(true);
   };
   ~NGOutOfFlowLayoutPartTest() {
     RuntimeEnabledFeatures::SetLayoutNGEnabled(false);
+    RuntimeEnabledFeatures::SetLayoutNGFragmentCachingEnabled(false);
   };
 };
 
@@ -67,8 +69,8 @@ TEST_F(NGOutOfFlowLayoutPartTest, FixedInsideAbs) {
   LayoutNGBlockFlow* block_flow = ToLayoutNGBlockFlow(rel->GetLayoutObject());
   RefPtr<NGConstraintSpace> space =
       NGConstraintSpace::CreateFromLayoutObject(*block_flow);
-  NGBlockNode node(block_flow);
-  RefPtr<NGLayoutResult> result = node.Layout(*space);
+  RefPtr<NGLayoutResult> result = block_flow->CachedLayoutResultForTesting();
+  EXPECT_TRUE(result);
   EXPECT_EQ(result->OutOfFlowPositionedDescendants().size(), (size_t)2);
 
   // Test the final result.
@@ -80,83 +82,6 @@ TEST_F(NGOutOfFlowLayoutPartTest, FixedInsideAbs) {
   EXPECT_EQ(fixed_2->OffsetTop(), LayoutUnit(9));
 };
 
-TEST_F(NGOutOfFlowLayoutPartTest, OrthogonalWritingMode1) {
-  SetBodyInnerHTML(
-      R"HTML(
-    <style>
-      #container {
-        position: relative;
-        writing-mode: horizontal-tb;
-        width: 200px;
-        height: 400px;
-      }
-      #abs-child {
-        position: absolute;
-        top: 10px;
-        writing-mode: vertical-rl;
-        width: auto;
-        height: 30px;
-      }
-    </style>
-    <div id="container">
-      <div id="abs-child"></div>
-    </div>
-    )HTML");
-
-  LayoutNGBlockFlow* block_flow =
-      ToLayoutNGBlockFlow(GetLayoutObjectByElementId("container"));
-  NGBlockNode node(block_flow);
-  RefPtr<NGConstraintSpace> space =
-      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
-
-  RefPtr<const NGPhysicalFragment> fragment =
-      node.Layout(*space)->PhysicalFragment();
-  EXPECT_EQ(NGPhysicalSize(LayoutUnit(200), LayoutUnit(400)), fragment->Size());
-
-  fragment = ToNGPhysicalBoxFragment(fragment.get())->Children()[0];
-  EXPECT_EQ(NGPhysicalSize(LayoutUnit(0), LayoutUnit(30)), fragment->Size());
-  EXPECT_EQ(NGPhysicalOffset(LayoutUnit(0), LayoutUnit(10)),
-            fragment->Offset());
-};
-
-TEST_F(NGOutOfFlowLayoutPartTest, OrthogonalWritingMode2) {
-  SetBodyInnerHTML(
-      R"HTML(
-    <style>
-      #container {
-        position: relative;
-        writing-mode: horizontal-tb;
-        width: 200px;
-        height: 400px;
-      }
-      #abs-child {
-        position: absolute;
-        top: 10px;
-        writing-mode: vertical-rl;
-        width: 20%;
-        height: 30px;
-      }
-    </style>
-    <div id="container">
-      <div id="abs-child"></div>
-    </div>
-    )HTML");
-
-  LayoutNGBlockFlow* block_flow =
-      ToLayoutNGBlockFlow(GetLayoutObjectByElementId("container"));
-  NGBlockNode node(block_flow);
-  RefPtr<NGConstraintSpace> space =
-      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
-
-  RefPtr<const NGPhysicalFragment> fragment =
-      node.Layout(*space)->PhysicalFragment();
-  EXPECT_EQ(NGPhysicalSize(LayoutUnit(200), LayoutUnit(400)), fragment->Size());
-
-  fragment = ToNGPhysicalBoxFragment(fragment.get())->Children()[0];
-  EXPECT_EQ(NGPhysicalSize(LayoutUnit(40), LayoutUnit(30)), fragment->Size());
-  EXPECT_EQ(NGPhysicalOffset(LayoutUnit(0), LayoutUnit(10)),
-            fragment->Offset());
-};
 
 }  // namespace
 }  // namespace blink