Make NGAbstractInlineTextBox to handle both NGFragmentItem and NGPaintFragment

This patch makes |NGAbstractInlineTextBox| to handle both |NGFragmentItem| and
|NGPaintFragment| by utilizing |NGInlineCursor|.

This patch reduces web tests failures of accessibility/ from 18 to 1.

Note: This patch enablies |LayoutObject::FirstInlineFragmentItemIndex()|
partially. It is used for deattaching |NGAbstractInlineTextBox| but not for
caching for fast traversing |NGInlineCursor::MoveToNextForSameLayoutObject()|.
Following patch will enable it.

Bug: 982194
Change-Id: Ic95739e355c14487a21044c3a360a3014f37dceb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1941651
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729673}

third_party/blink/renderer/core/layout/layout_box.cc

@@ -2365,7 +2365,9 @@ void LayoutBox::SetFirstInlineFragmentItemIndex(wtf_size_t index) {
   CHECK(IsInLayoutNGInlineFormattingContext()) << *this;
   DCHECK(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
   DCHECK_NE(index, 0u);
-  first_fragment_item_index_ = index;
+  // TDOO(yosin): Once we update all |LayoutObject::FirstInlineFragment()|,
+  // we should enable below.
+  // first_fragment_item_index_ = index;
 }
 
 void LayoutBox::InLayoutNGInlineFormattingContextWillChange(bool new_value) {

third_party/blink/renderer/core/layout/layout_inline.cc

@@ -171,7 +171,9 @@ void LayoutInline::SetFirstInlineFragmentItemIndex(wtf_size_t index) {
   CHECK(IsInLayoutNGInlineFormattingContext()) << *this;
   DCHECK(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
   DCHECK_NE(index, 0u);
-  first_fragment_item_index_ = index;
+  // TDOO(yosin): Once we update all |LayoutObject::FirstInlineFragment()|,
+  // we should enable below.
+  // first_fragment_item_index_ = index;
 }
 
 void LayoutInline::InLayoutNGInlineFormattingContextWillChange(bool new_value) {

third_party/blink/renderer/core/layout/layout_text.cc

@@ -222,13 +222,20 @@ void LayoutText::RemoveAndDestroyTextBoxes() {
       for (InlineTextBox* box : TextBoxes())
         box->Remove();
     } else {
-      if (NGPaintFragment* first_inline_fragment = FirstInlineFragment()) {
+      if (RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
+        if (has_abstract_inline_text_box_)
+          ClearFirstInlineFragmentItemIndex();
+      } else if (NGPaintFragment* first_inline_fragment =
+                     FirstInlineFragment()) {
         first_inline_fragment->LayoutObjectWillBeDestroyed();
         SetFirstInlineFragment(nullptr);
       }
       if (Parent())
         Parent()->DirtyLinesFromChangedChild(this);
     }
+  } else if (RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
+    if (has_abstract_inline_text_box_)
+      ClearFirstInlineFragmentItemIndex();
   } else if (NGPaintFragment* first_inline_fragment = FirstInlineFragment()) {
     // Still do this to clear the global hash map in  NGAbstractInlineTextBox.
     SetFirstInlineFragment(nullptr);
@@ -266,7 +273,20 @@ void LayoutText::RemoveTextBox(InlineTextBox* box) {
 
 void LayoutText::DeleteTextBoxes() {
   if (!IsInLayoutNGInlineFormattingContext())
-    MutableTextBoxes().DeleteLineBoxes();
+    return MutableTextBoxes().DeleteLineBoxes();
+  DetachAbstractInlineTextBoxesIfNeeded();
+}
+
+void LayoutText::DetachAbstractInlineTextBoxesIfNeeded() {
+  // TODO(layout-dev): Because We should call |WillDestroy()| once for
+  // associated fragments, when you reuse fragments, you should construct
+  // NGAbstractInlineTextBox for them.
+  if (!has_abstract_inline_text_box_)
+    return;
+  NGInlineCursor cursor;
+  for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject())
+    NGAbstractInlineTextBox::WillDestroy(cursor);
+  has_abstract_inline_text_box_ = false;
 }
 
 void LayoutText::SetFirstInlineFragment(NGPaintFragment* first_fragment) {
@@ -275,20 +295,14 @@ void LayoutText::SetFirstInlineFragment(NGPaintFragment* first_fragment) {
   // |!fragment|.
   DCHECK(!first_fragment ||
          !RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
-  // TODO(layout-dev): Because We should call |WillDestroy()| once for
-  // associated fragments, when you reuse fragments, you should construct
-  // NGAbstractInlineTextBox for them.
-  if (has_abstract_inline_text_box_) {
-    for (NGPaintFragment* fragment : NGPaintFragment::InlineFragmentsFor(this))
-      NGAbstractInlineTextBox::WillDestroy(fragment);
-    has_abstract_inline_text_box_ = false;
-  }
+  DetachAbstractInlineTextBoxesIfNeeded();
   first_paint_fragment_ = first_fragment;
 }
 
 void LayoutText::ClearFirstInlineFragmentItemIndex() {
   CHECK(IsInLayoutNGInlineFormattingContext()) << *this;
   DCHECK(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
+  DetachAbstractInlineTextBoxesIfNeeded();
   first_fragment_item_index_ = 0u;
 }
 
@@ -297,7 +311,10 @@ void LayoutText::SetFirstInlineFragmentItemIndex(wtf_size_t index) {
   // TODO(yosin): Call |NGAbstractInlineTextBox::WillDestroy()|.
   DCHECK(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
   DCHECK_NE(index, 0u);
-  first_fragment_item_index_ = index;
+  DetachAbstractInlineTextBoxesIfNeeded();
+  // TDOO(yosin): Once we update all |LayoutObject::FirstInlineFragment()|,
+  // we should enable below.
+  // first_fragment_item_index_ = index;
 }
 
 void LayoutText::InLayoutNGInlineFormattingContextWillChange(bool new_value) {
@@ -306,6 +323,10 @@ void LayoutText::InLayoutNGInlineFormattingContextWillChange(bool new_value) {
   // Because |first_paint_fragment_| and |text_boxes_| are union, when one is
   // deleted, the other should be initialized to nullptr.
   DCHECK(new_value ? !first_paint_fragment_ : !text_boxes_.First());
+
+  // Because there are no inline boxes associated to this text, we should not
+  // have abstract inline text boxes too.
+  DCHECK(!has_abstract_inline_text_box_);
 }
 
 Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const {
@@ -2420,12 +2441,10 @@ void LayoutText::MomentarilyRevealLastTypedCharacter(
 }
 
 scoped_refptr<AbstractInlineTextBox> LayoutText::FirstAbstractInlineTextBox() {
-  if (RuntimeEnabledFeatures::LayoutNGEnabled()) {
-    auto fragments = NGPaintFragment::InlineFragmentsFor(this);
-    if (!fragments.IsEmpty() &&
-        fragments.IsInLayoutNGInlineFormattingContext()) {
-      return NGAbstractInlineTextBox::GetOrCreate(fragments.front());
-    }
+  if (IsInLayoutNGInlineFormattingContext()) {
+    NGInlineCursor cursor;
+    cursor.MoveTo(*this);
+    return NGAbstractInlineTextBox::GetOrCreate(cursor);
   }
   return LegacyAbstractInlineTextBox::GetOrCreate(LineLayoutText(this),
                                                   FirstTextBox());

third_party/blink/renderer/core/layout/layout_text.h

@@ -439,6 +439,7 @@ class CORE_EXPORT LayoutText : public LayoutObject {
 
  private:
   ContentCaptureManager* GetContentCaptureManager();
+  void DetachAbstractInlineTextBoxesIfNeeded();
 
   // Used for LayoutNG with accessibility. True if inline fragments are
   // associated to |NGAbstractInlineTextBox|.

third_party/blink/renderer/core/layout/ng/inline/ng_abstract_inline_text_box.h

@@ -9,34 +9,35 @@
 
 namespace blink {
 
+class NGFragmentItem;
+class NGInlineCursor;
 class NGPaintFragment;
-class NGPhysicalTextFragment;
 
 // The implementation of |AbstractInlineTextBox| for LayoutNG.
 // See also |LegacyAbstractInlineTextBox| for legacy layout.
 class CORE_EXPORT NGAbstractInlineTextBox final : public AbstractInlineTextBox {
  private:
   // Returns existing or newly created |NGAbstractInlineTextBox|.
-  // * |fragment| should be attached to |NGPhysicalTextFragment|.
+  // * |cursor| should be attached to |NGPhysicalTextFragment|.
   static scoped_refptr<AbstractInlineTextBox> GetOrCreate(
-      const NGPaintFragment& fragment);
-  static void WillDestroy(NGPaintFragment*);
+      const NGInlineCursor& cursor);
+  static void WillDestroy(const NGInlineCursor& cursor);
 
   friend class LayoutText;
-  friend class NGPaintFragment;
 
  public:
-  ~NGAbstractInlineTextBox() final;
-
- private:
   NGAbstractInlineTextBox(LineLayoutText line_layout_item,
                           const NGPaintFragment& fragment);
+  NGAbstractInlineTextBox(LineLayoutText line_layout_item,
+                          const NGFragmentItem& fragment);
 
-  const NGPhysicalTextFragment& PhysicalTextFragment() const;
-  bool NeedsLayout() const;
+  ~NGAbstractInlineTextBox() final;
+
+ private:
+  NGInlineCursor GetCursor() const;
+  NGInlineCursor GetCursorOnLine() const;
+  String GetTextContent() const;
   bool NeedsTrailingSpace() const;
-  // Returns next fragment associated to |LayoutText|.
-  const NGPaintFragment* NextTextFragmentForSameLayoutObject() const;
 
   // Implementations of AbstractInlineTextBox member functions.
   void Detach() final;
@@ -53,12 +54,10 @@ class CORE_EXPORT NGAbstractInlineTextBox final : public AbstractInlineTextBox {
   scoped_refptr<AbstractInlineTextBox> PreviousOnLine() const final;
   bool IsLineBreak() const final;
 
-  const NGPaintFragment* fragment_;
-
-  using FragmentToNGAbstractInlineTextBoxHashMap =
-      HashMap<const NGPaintFragment*, scoped_refptr<AbstractInlineTextBox>>;
-  static FragmentToNGAbstractInlineTextBoxHashMap*
-      g_abstract_inline_text_box_map_;
+  union {
+    const NGPaintFragment* fragment_;
+    const NGFragmentItem* fragment_item_;
+  };
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items_builder.cc

@@ -221,9 +221,7 @@ void NGFragmentItemsBuilder::AssociateNextForSameLayoutObject() {
     DCHECK(layout_object->IsInLayoutNGInlineFormattingContext()) << item;
     auto insert_result = last_fragment_map.insert(layout_object, index);
     if (insert_result.is_new_entry) {
-      // TDOO(yosin): Once we update all |LayoutObject::FirstInlineFragment()|,
-      // we should enable below.
-      // layout_object->SetFirstInlineFragmentItemIndex(index);
+      layout_object->SetFirstInlineFragmentItemIndex(index);
       continue;
     }
     const wtf_size_t last_index = insert_result.stored_value->value;

third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.cc

@@ -724,6 +724,17 @@ void NGInlineCursor::MoveTo(const LayoutObject& layout_object) {
     MoveToNext();
 }
 
+void NGInlineCursor::MoveTo(const NGFragmentItem& fragment_item) {
+  DCHECK(!root_paint_fragment_ && !current_paint_fragment_);
+  MoveTo(*fragment_item.GetLayoutObject());
+  while (IsNotNull()) {
+    if (CurrentItem() == &fragment_item)
+      return;
+    MoveToNext();
+  }
+  NOTREACHED();
+}
+
 void NGInlineCursor::MoveTo(const NGInlineCursor& cursor) {
   if (const NGPaintFragment* paint_fragment = cursor.CurrentPaintFragment()) {
     MoveTo(*paint_fragment);

third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.h

@@ -175,11 +175,15 @@ class CORE_EXPORT NGInlineCursor {
   // text and atomic inline. It is also error to call |IsGeneratedTextType()|.
   UBiDiLevel CurrentBidiLevel() const;
 
+  // Returns break token for line box. It is error to call other than line box.
+  const NGInlineBreakToken& CurrentInlineBreakToken() const;
+
   // Returns text direction of current text or atomic inline. It is error to
   // call at other than text or atomic inline. Note: <span> doesn't have
   // reserved direction.
   TextDirection CurrentResolvedDirection() const;
   const ComputedStyle& CurrentStyle() const;
+  bool UsesFirstLineStyle() const;
 
   // InkOverflow of itself, including contents if they contribute to the ink
   // overflow of this object (e.g. when not clipped,) in the local coordinate.
@@ -226,6 +230,9 @@ class CORE_EXPORT NGInlineCursor {
   // Functions to move the current position.
   //
 
+  // Move the current position at |fragment_item|.
+  void MoveTo(const NGFragmentItem& fragment_item);
+
   // Move the current position at |cursor|. Unlinke copy constrcutr, this
   // function doesn't copy root. Note: The current position in |cursor|
   // should be part of |this| cursor.
@@ -256,6 +263,9 @@ class CORE_EXPORT NGInlineCursor {
   // See also |TryToMoveToFirstChild()|.
   void MoveToLastChild();
 
+  // Move the current position to the last fragment on same layout object.
+  void MoveToLastForSameLayoutObject();
+
   // Move to last logical leaf of current line box. If current line box has
   // no children, curosr becomes null.
   void MoveToLastLogicalLeaf();
@@ -308,12 +318,8 @@ class CORE_EXPORT NGInlineCursor {
   // NextSkippingChildren, Previous, etc.
 
  private:
-  // Returns break token for line box. It is error to call other than line box.
-  const NGInlineBreakToken& CurrentInlineBreakToken() const;
-
   // Returns style variant of the current position.
   NGStyleVariant CurrentStyleVariant() const;
-  bool UsesFirstLineStyle() const;
 
   // True if current position is part of culled inline box |layout_inline|.
   bool IsPartOfCulledInlineBox(const LayoutInline& layout_inline) const;
@@ -329,9 +335,6 @@ class CORE_EXPORT NGInlineCursor {
   // Move the cursor position to the first fragment in tree.
   void MoveToFirst();
 
-  // Move the current position to the last fragment on same layout object.
-  void MoveToLastForSameLayoutObject();
-
   // Same as |MoveTo()| but not support culled inline.
   void InternalMoveTo(const LayoutObject& layout_object);
 

third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc

@@ -781,7 +781,9 @@ NGPaintFragment* NGPaintFragment::FirstLineBox() const {
 }
 
 const NGPaintFragment* NGPaintFragment::Root() const {
-  DCHECK(PhysicalFragment().IsInline());
+  // Because of this function can be called during |LayoutObject::Destroy()|,
+  // we use |physical_fragment_| to avoid calling |IsAlive()|.
+  DCHECK(physical_fragment_->IsInline());
   const NGPaintFragment* root = this;
   for (const NGPaintFragment* fragment :
        NGPaintFragmentTraversal::InclusiveAncestorsOf(*this)) {