[LayoutNG] Unify LayoutNG invalidation logic to |LayoutObjectChildList|

This patch unifies the same logic separated in two places
into one.

Originally, the logic needed for new LayoutObjects in the
tree is put into |InsertedIntoTree()|. However, some logic
are added to |LayoutObjectChildList| too because they are
needed to run regardless of |notify_layout_object|, which
is an optimization when some of existing notifications can
be skipped for in-tree moves.

As it turned out that all LayoutNG logic are needed
regardless of |notify_layout_object|, this patch moves all
of them to |LayoutObjectChildList|.

The |SplitFlow()| case was found in crbug.com/964619.

Bug: 964854
Change-Id: I777b05fa2355e301ceb713b49d83734546e5ecf7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617055
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661378}
diff --git a/third_party/blink/renderer/core/layout/layout_object.cc b/third_party/blink/renderer/core/layout/layout_object.cc
index 39ba4d2..1ff1c9cc 100644
--- a/third_party/blink/renderer/core/layout/layout_object.cc
+++ b/third_party/blink/renderer/core/layout/layout_object.cc
@@ -3076,11 +3076,9 @@
       layer->DirtyVisibleContentStatus();
   }
 
-  if (IsInLayoutNGInlineFormattingContext()) {
-    // In case of |this| layout object is moved, to avoid paint fragments in old
-    // tree live longer than |this|, we reset associated paint fragment list.
-    SetFirstInlineFragment(nullptr);
-  }
+  // |FirstInlineFragment()| should be cleared. |LayoutObjectChildList| does
+  // this, just check here for all new objects in the tree.
+  DCHECK_EQ(FirstInlineFragment(), nullptr);
 
   if (Parent()->ChildrenInline())
     Parent()->DirtyLinesFromChangedChild(this);
diff --git a/third_party/blink/renderer/core/layout/layout_object_child_list.cc b/third_party/blink/renderer/core/layout/layout_object_child_list.cc
index 54faafe..f7a3bd2 100644
--- a/third_party/blink/renderer/core/layout/layout_object_child_list.cc
+++ b/third_party/blink/renderer/core/layout/layout_object_child_list.cc
@@ -37,23 +37,34 @@
 
 namespace {
 
-// Invalidate InineItems() in LayoutNGText.
+// Invalidate LayoutNG properties for insertion.
 //
 // They need to be invalidated when moving across inline formatting context
 // (i.e., to a different LayoutBlockFlow.)
 void InvalidateInlineItems(LayoutObject* object) {
-  if (object->IsInLayoutNGInlineFormattingContext())
-    object->SetFirstInlineFragment(nullptr);
-  if (object->IsText()) {
-    ToLayoutText(object)->InvalidateInlineItems();
-  } else if (object->IsLayoutInline()) {
-    // When moving without |notify_layout_object|, only top-level objects are
-    // moved. Ensure to invalidate all LayoutNGText in this inline formatting
-    // context.
-    for (LayoutObject* curr = object->SlowFirstChild(); curr;
-         curr = curr->NextSibling())
-      InvalidateInlineItems(curr);
+  DCHECK(object->IsInLayoutNGInlineFormattingContext());
+
+  if (auto* layout_text = ToLayoutTextOrNull(object)) {
+    layout_text->SetFirstInlineFragment(nullptr);
+    layout_text->InvalidateInlineItems();
+    return;
   }
+
+  if (auto* layout_inline = ToLayoutInlineOrNull(object)) {
+    layout_inline->SetFirstInlineFragment(nullptr);
+
+    // In some cases, only top-level objects are moved, when |SplitFlow()| moves
+    // subtree, or when moving without |notify_layout_object|. Ensure to
+    // invalidate all descendants in this inline formatting context.
+    for (LayoutObject* child = layout_inline->FirstChild(); child;
+         child = child->NextSibling()) {
+      if (child->IsInLayoutNGInlineFormattingContext())
+        InvalidateInlineItems(child);
+    }
+    return;
+  }
+
+  object->SetFirstInlineFragment(nullptr);
 }
 
 }  // namespace
@@ -193,14 +204,16 @@
   }
 
   if (!owner->DocumentBeingDestroyed()) {
+    // Run LayoutNG invalidations outside of |InsertedIntoTree| because it needs
+    // to run regardless of |notify_layout_object|. |notify_layout_object| is an
+    // optimization to skip notifications when moving within the same tree.
+    if (new_child->IsInLayoutNGInlineFormattingContext()) {
+      InvalidateInlineItems(new_child);
+    }
+
     if (notify_layout_object) {
       new_child->InsertedIntoTree();
       LayoutCounter::LayoutObjectSubtreeAttached(new_child);
-    } else if (RuntimeEnabledFeatures::LayoutNGEnabled()) {
-      // |notify_layout_object| is an optimization to skip notifications when
-      // moving within the same tree. Inline items need to be invalidated even
-      // when moving.
-      InvalidateInlineItems(new_child);
     }
 
     if (owner->IsInLayoutNGInlineFormattingContext() ||
diff --git a/third_party/blink/renderer/core/layout/ng/inline/layout_ng_text.h b/third_party/blink/renderer/core/layout/ng/inline/layout_ng_text.h
index d922dbf..4e2517f 100644
--- a/third_party/blink/renderer/core/layout/ng/inline/layout_ng_text.h
+++ b/third_party/blink/renderer/core/layout/ng/inline/layout_ng_text.h
@@ -23,12 +23,6 @@
   }
   bool IsLayoutNGObject() const override { return true; }
 
- protected:
-  void InsertedIntoTree() override {
-    valid_ng_items_ = false;
-    LayoutText::InsertedIntoTree();
-  }
-
  private:
   const NGInlineItems* GetNGInlineItems() const final { return &inline_items_; }
   NGInlineItems* GetNGInlineItems() final { return &inline_items_; }
diff --git a/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc b/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc
index 5f5ad0b..c187f34 100644
--- a/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc
+++ b/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc
@@ -1339,6 +1339,55 @@
   EXPECT_FALSE(layout_block_flow_->HasNGInlineNodeData());
 }
 
+// Test inline objects are initialized when |SplitFlow()| moves them.
+TEST_F(NGInlineNodeTest, ClearFirstInlineFragmentOnSplitFlow) {
+  SetBodyInnerHTML(R"HTML(
+    <div>
+      <span id=outer_span>
+        <span id=inner_span>1234</span>
+      </span>
+    </div>
+  )HTML");
+
+  // Keep the text fragment to compare later.
+  Element* inner_span = GetElementById("inner_span");
+  Node* text = inner_span->firstChild();
+  scoped_refptr<NGPaintFragment> text_fragment_before_split =
+      text->GetLayoutObject()->FirstInlineFragment();
+  EXPECT_NE(text_fragment_before_split.get(), nullptr);
+
+  // Append <div> to <span>. causing SplitFlow().
+  Element* outer_span = GetElementById("outer_span");
+  Element* div = GetDocument().CreateRawElement(html_names::kDivTag);
+  outer_span->appendChild(div);
+
+  // Update tree but do NOT update layout. At this point, there's no guarantee,
+  // but there are some clients (e.g., Schroll Anchor) who try to read
+  // associated fragments.
+  //
+  // NGPaintFragment is owned by LayoutNGBlockFlow. Because the original owner
+  // no longer has an inline formatting context, the NGPaintFragment subtree is
+  // destroyed, and should not be accessible.
+  GetDocument().UpdateStyleAndLayoutTree();
+  scoped_refptr<NGPaintFragment> text_fragment_before_layout =
+      text->GetLayoutObject()->FirstInlineFragment();
+  EXPECT_EQ(text_fragment_before_layout, nullptr);
+
+  // Update layout. There should be a different instance of the text fragment.
+  UpdateAllLifecyclePhasesForTest();
+  scoped_refptr<NGPaintFragment> text_fragment_after_layout =
+      text->GetLayoutObject()->FirstInlineFragment();
+  EXPECT_NE(text_fragment_before_split, text_fragment_after_layout);
+
+  // Check it is the one owned by the new root inline formatting context.
+  LayoutBlock* anonymous_block =
+      inner_span->GetLayoutObject()->ContainingBlock();
+  EXPECT_TRUE(anonymous_block->IsAnonymous());
+  const NGPaintFragment* block_fragment = anonymous_block->PaintFragment();
+  const NGPaintFragment* line_box_fragment = block_fragment->FirstChild();
+  EXPECT_EQ(line_box_fragment->FirstChild(), text_fragment_after_layout);
+}
+
 // https://crbug.com/911220
 TEST_F(NGInlineNodeTest, PreservedNewlineWithBidiAndRelayout) {
   SetupHtml("container",