Stop storing NGInlineItemResult in NGLogicalLineItem

Because |NGLogicalLineItem| can live longer than
|NGInlineItemResult|, the pointer may become stale.
Instead, this patch changes |NGLogicalLineItem| to copy
necessary data from |NGInlineItemResult|.

This is to prepare for vectorizing |NGFragmentItem|, by
allowing to defer creating |NGFragmentItem| to after
|NGInlineItemResult| is destroyed.

This patch has no behavior changes.

Bug: 982194
Change-Id: Idfad9fe4d4421ddd76b2348e6c17e50e327911a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212187Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771489}

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

@@ -61,16 +61,19 @@ NGFragmentItem::NGFragmentItem(const NGPhysicalTextFragment& text)
   DCHECK(!IsFormattingContextRoot());
 }
 
-NGFragmentItem::NGFragmentItem(NGInlineItemResult&& item_result,
-                               const PhysicalSize& size)
-    : layout_object_(item_result.item->GetLayoutObject()),
-      text_({std::move(item_result.shape_result), item_result.TextOffset()}),
+NGFragmentItem::NGFragmentItem(
+    const NGInlineItem& inline_item,
+    scoped_refptr<const ShapeResultView> shape_result,
+    const NGTextOffset& text_offset,
+    const PhysicalSize& size)
+    : layout_object_(inline_item.GetLayoutObject()),
+      text_({std::move(shape_result), text_offset}),
       rect_({PhysicalOffset(), size}),
       type_(kText),
-      sub_type_(static_cast<unsigned>(item_result.item->TextType())),
-      style_variant_(static_cast<unsigned>(item_result.item->StyleVariant())),
+      sub_type_(static_cast<unsigned>(inline_item.TextType())),
+      style_variant_(static_cast<unsigned>(inline_item.StyleVariant())),
       is_hidden_for_paint_(false),  // TODO(kojii): not supported yet.
-      text_direction_(static_cast<unsigned>(item_result.item->Direction())),
+      text_direction_(static_cast<unsigned>(inline_item.Direction())),
       ink_overflow_computed_(false),
       is_dirty_(false),
       is_last_for_node_(true) {
@@ -128,11 +131,10 @@ void NGFragmentItem::Create(NGLogicalLineItems* child_list,
       continue;
     }
 
-    if (child.item_result) {
+    if (child.inline_item) {
       child.fragment_item = base::AdoptRef(new NGFragmentItem(
-          std::move(*child.item_result),
-          ToPhysicalSize({child.inline_size, child.rect.size.block_size},
-                         writing_mode)));
+          *child.inline_item, std::move(child.shape_result), child.text_offset,
+          ToPhysicalSize(child.MarginSize(), writing_mode)));
       continue;
     }
 

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

@@ -24,7 +24,6 @@ class NGInlineBreakToken;
 class NGLogicalLineItems;
 class NGPhysicalTextFragment;
 struct NGTextFragmentPaintInfo;
-struct NGInlineItemResult;
 
 // This class represents a text run or a box in an inline formatting context.
 //
@@ -353,7 +352,10 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
 
  private:
   // Create a text item.
-  NGFragmentItem(NGInlineItemResult&& item_result, const PhysicalSize& size);
+  NGFragmentItem(const NGInlineItem& inline_item,
+                 scoped_refptr<const ShapeResultView> shape_result,
+                 const NGTextOffset& text_offset,
+                 const PhysicalSize& size);
 
   const LayoutBox* InkOverflowOwnerBox() const;
   LayoutBox* MutableInkOverflowOwnerBox();

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

@@ -244,13 +244,16 @@ void NGInlineLayoutAlgorithm::CreateLine(
              item.TextType() == NGTextType::kSymbolMarker);
       if (UNLIKELY(item_result.hyphen_shape_result)) {
         LayoutUnit hyphen_inline_size = item_result.HyphenInlineSize();
-        line_box_.AddChild(&item_result, box->text_top,
+        line_box_.AddChild(item, std::move(item_result.shape_result),
+                           item_result.TextOffset(), box->text_top,
                            item_result.inline_size - hyphen_inline_size,
                            box->text_height, item.BidiLevel());
         PlaceHyphen(item_result, hyphen_inline_size, box);
       } else {
-        line_box_.AddChild(&item_result, box->text_top, item_result.inline_size,
-                           box->text_height, item.BidiLevel());
+        line_box_.AddChild(item, std::move(item_result.shape_result),
+                           item_result.TextOffset(), box->text_top,
+                           item_result.inline_size, box->text_height,
+                           item.BidiLevel());
       }
       has_logical_text_items = true;
 
@@ -458,8 +461,10 @@ void NGInlineLayoutAlgorithm::PlaceControlItem(const NGInlineItem& item,
     box->EnsureTextMetrics(*item.Style(), baseline_type_);
 
   NGTextFragmentBuilder text_builder(ConstraintSpace().GetWritingMode());
-  text_builder.SetItem(line_info.ItemsData().text_content, item_result,
-                       box->text_height);
+  text_builder.SetItem(line_info.ItemsData().text_content, item,
+                       std::move(item_result->shape_result),
+                       item_result->TextOffset(),
+                       {item_result->inline_size, box->text_height});
   line_box_.AddChild(text_builder.ToTextFragment(), box->text_top,
                      item_result->inline_size, item.BidiLevel());
 }
@@ -1141,8 +1146,8 @@ void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction) {
   for (unsigned logical_index : indices_in_visual_order) {
     visual_items.AddChild(std::move(line_box_[logical_index]));
     DCHECK(!line_box_[logical_index].HasInFlowFragment() ||
-           // |item_result| will not be null by moving.
-           line_box_[logical_index].item_result);
+           // |inline_item| will not be null by moving.
+           line_box_[logical_index].inline_item);
   }
   DCHECK_EQ(line_box_.size(), visual_items.size());
   line_box_ = std::move(visual_items);

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

@@ -13,15 +13,14 @@ void NGLogicalLineItems::CreateTextFragments(WritingMode writing_mode,
                                              const String& text_content) {
   NGTextFragmentBuilder text_builder(writing_mode);
   for (auto& child : *this) {
-    if (NGInlineItemResult* item_result = child.item_result) {
-      DCHECK(item_result->item);
-      const NGInlineItem& item = *item_result->item;
-      DCHECK(item.Type() == NGInlineItem::kText ||
-             item.Type() == NGInlineItem::kControl);
-      DCHECK(item.TextType() == NGTextType::kNormal ||
-             item.TextType() == NGTextType::kSymbolMarker);
-      text_builder.SetItem(text_content, item_result,
-                           child.rect.size.block_size);
+    if (const NGInlineItem* inline_item = child.inline_item) {
+      DCHECK(inline_item->Type() == NGInlineItem::kText ||
+             inline_item->Type() == NGInlineItem::kControl);
+      DCHECK(inline_item->TextType() == NGTextType::kNormal ||
+             inline_item->TextType() == NGTextType::kSymbolMarker);
+      text_builder.SetItem(text_content, *inline_item,
+                           std::move(child.shape_result), child.text_offset,
+                           child.MarginSize());
       DCHECK(!child.fragment);
       child.fragment = text_builder.ToTextFragment();
     }

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

@@ -14,7 +14,6 @@
 namespace blink {
 
 class LayoutObject;
-struct NGInlineItemResult;
 
 // This class represents an item in a line, after line break, but still mutable
 // and in the logical coordinate system.
@@ -50,12 +49,16 @@ struct NGLogicalLineItem {
         children_count(children_count),
         bidi_level(bidi_level) {}
   // Create an in-flow text fragment.
-  NGLogicalLineItem(NGInlineItemResult* item_result,
+  NGLogicalLineItem(const NGInlineItem& inline_item,
+                    scoped_refptr<const ShapeResultView> shape_result,
+                    const NGTextOffset& text_offset,
                     LayoutUnit block_offset,
                     LayoutUnit inline_size,
                     LayoutUnit text_height,
                     UBiDiLevel bidi_level)
-      : item_result(item_result),
+      : inline_item(&inline_item),
+        shape_result(std::move(shape_result)),
+        text_offset(text_offset),
         rect(LayoutUnit(), block_offset, LayoutUnit(), text_height),
         inline_size(inline_size),
         bidi_level(bidi_level) {}
@@ -94,16 +97,11 @@ struct NGLogicalLineItem {
         bidi_level(bidi_level) {}
 
   bool HasInFlowFragment() const {
-    if (fragment_item)
-      return true;
-    if (fragment)
-      return true;
-    if (item_result)
-      return true;
-    if (layout_result && !layout_result->PhysicalFragment().IsFloating())
-      return true;
-
-    return false;
+    return fragment_item || fragment || inline_item ||
+           (layout_result && !layout_result->PhysicalFragment().IsFloating());
+  }
+  bool HasInFlowOrFloatingFragment() const {
+    return fragment_item || fragment || inline_item || layout_result;
   }
   bool HasOutOfFlowFragment() const { return out_of_flow_positioned_box; }
   bool HasFragment() const {
@@ -128,6 +126,7 @@ struct NGLogicalLineItem {
   const LogicalOffset& Offset() const { return rect.offset; }
   LayoutUnit InlineOffset() const { return rect.offset.inline_offset; }
   const LogicalSize& Size() const { return rect.size; }
+  LogicalSize MarginSize() const { return {inline_size, Size().block_size}; }
   const NGPhysicalFragment* PhysicalFragment() const {
     if (layout_result)
       return &layout_result->PhysicalFragment();
@@ -143,8 +142,12 @@ struct NGLogicalLineItem {
   scoped_refptr<NGFragmentItem> fragment_item;
   scoped_refptr<const NGLayoutResult> layout_result;
   scoped_refptr<const NGPhysicalTextFragment> fragment;
-  // |NGInlineItemResult| to create a text fragment from.
-  NGInlineItemResult* item_result = nullptr;
+
+  // Data to create a text fragment from.
+  const NGInlineItem* inline_item = nullptr;
+  scoped_refptr<const ShapeResultView> shape_result;
+  NGTextOffset text_offset;
+
   LayoutObject* out_of_flow_positioned_box = nullptr;
   LayoutObject* unpositioned_float = nullptr;
   // The offset of the border box, initially in this child coordinate system.

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

@@ -60,7 +60,7 @@ NGPhysicalTextFragment::NGPhysicalTextFragment(NGTextFragmentBuilder* builder)
                          kFragmentText,
                          static_cast<unsigned>(builder->text_type_)),
       text_(builder->text_),
-      text_offset_({builder->start_offset_, builder->end_offset_}),
+      text_offset_(builder->text_offset_),
       shape_result_(std::move(builder->shape_result_)) {
   DCHECK(shape_result_ || IsFlowControl()) << *this;
   base_or_resolved_direction_ =

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

@@ -17,30 +17,28 @@ NGTextFragmentBuilder::NGTextFragmentBuilder(
     const NGPhysicalTextFragment& fragment)
     : NGFragmentBuilder(fragment),
       text_(fragment.text_),
-      start_offset_(fragment.StartOffset()),
-      end_offset_(fragment.EndOffset()),
+      text_offset_(fragment.TextOffset()),
       shape_result_(fragment.TextShapeResult()),
       text_type_(fragment.TextType()) {}
 
-void NGTextFragmentBuilder::SetItem(const String& text_content,
-                                    NGInlineItemResult* item_result,
-                                    LayoutUnit line_height) {
-  DCHECK(item_result);
-  const NGInlineItem* item = item_result->item;
-  DCHECK(item);
-  DCHECK_NE(item->TextType(), NGTextType::kLayoutGenerated)
+void NGTextFragmentBuilder::SetItem(
+    const String& text_content,
+    const NGInlineItem& item,
+    scoped_refptr<const ShapeResultView> shape_result,
+    const NGTextOffset& text_offset,
+    const LogicalSize& size) {
+  DCHECK_NE(item.TextType(), NGTextType::kLayoutGenerated)
       << "Please use SetText() instead.";
-  DCHECK(item->Style());
+  DCHECK(item.Style());
 
-  text_type_ = item->TextType();
+  text_type_ = item.TextType();
   text_ = text_content;
-  start_offset_ = item_result->start_offset;
-  end_offset_ = item_result->end_offset;
-  resolved_direction_ = item->Direction();
-  SetStyle(item->Style(), item->StyleVariant());
-  size_ = {item_result->inline_size, line_height};
-  shape_result_ = std::move(item_result->shape_result);
-  layout_object_ = item->GetLayoutObject();
+  text_offset_ = text_offset;
+  resolved_direction_ = item.Direction();
+  SetStyle(item.Style(), item.StyleVariant());
+  size_ = size;
+  shape_result_ = std::move(shape_result);
+  layout_object_ = item.GetLayoutObject();
 }
 
 void NGTextFragmentBuilder::SetText(
@@ -55,8 +53,7 @@ void NGTextFragmentBuilder::SetText(
 
   text_type_ = NGTextType::kLayoutGenerated;
   text_ = text;
-  start_offset_ = shape_result->StartIndex();
-  end_offset_ = shape_result->EndIndex();
+  text_offset_ = {shape_result->StartIndex(), shape_result->EndIndex()};
   resolved_direction_ = shape_result->Direction();
   SetStyle(style, is_ellipsis_style ? NGStyleVariant::kEllipsis
                                     : NGStyleVariant::kStandard);

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

@@ -15,7 +15,6 @@ namespace blink {
 
 class LayoutObject;
 class ShapeResultView;
-struct NGInlineItemResult;
 
 class CORE_EXPORT NGTextFragmentBuilder final : public NGFragmentBuilder {
   STACK_ALLOCATED();
@@ -30,8 +29,10 @@ class CORE_EXPORT NGTextFragmentBuilder final : public NGFragmentBuilder {
 
   // NOTE: Takes ownership of the shape result within the item result.
   void SetItem(const String& text_content,
-               NGInlineItemResult*,
-               LayoutUnit line_height);
+               const NGInlineItem& inline_item,
+               scoped_refptr<const ShapeResultView> shape_result,
+               const NGTextOffset& text_offset,
+               const LogicalSize& size);
 
   // Set text for generated text, e.g. hyphen and ellipsis.
   void SetText(LayoutObject*,
@@ -45,8 +46,7 @@ class CORE_EXPORT NGTextFragmentBuilder final : public NGFragmentBuilder {
 
  private:
   String text_;
-  unsigned start_offset_;
-  unsigned end_offset_;
+  NGTextOffset text_offset_;
   scoped_refptr<const ShapeResultView> shape_result_;
 
   NGTextType text_type_ = NGTextType::kNormal;