[FragmentItem] Make NGFragmentItems flexible array

This patch changes |NGFragmentItems| to flexible array,
saving the overhead of |Vector| and possible unused capacity.

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

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

@@ -18,29 +18,49 @@ inline bool ShouldAssociateWithLayoutObject(const NGFragmentItem& item) {
 }  // namespace
 
 NGFragmentItems::NGFragmentItems(NGFragmentItemsBuilder* builder)
-    : items_(std::move(builder->items_)),
-      text_content_(std::move(builder->text_content_)),
-      first_line_text_content_(std::move(builder->first_line_text_content_)) {}
+    : text_content_(std::move(builder->text_content_)),
+      first_line_text_content_(std::move(builder->first_line_text_content_)),
+      size_(builder->items_.size()) {
+  Vector<scoped_refptr<const NGFragmentItem>>& source_items = builder->items_;
+  for (unsigned i = 0; i < size_; ++i) {
+    // Call the move constructor to move without |AddRef|. Items in
+    // |NGFragmentItemsBuilder| are not used after |this| was constructed.
+    DCHECK(source_items[i]);
+    new (&items_[i])
+        scoped_refptr<const NGFragmentItem>(std::move(source_items[i]));
+    DCHECK(!source_items[i]);  // Ensure the source was moved.
+  }
+}
+
+NGFragmentItems::~NGFragmentItems() {
+  for (unsigned i = 0; i < size_; ++i)
+    items_[i]->Release();
+}
+
+bool NGFragmentItems::IsSubSpan(const Span& span) const {
+  return span.empty() ||
+         (span.data() >= ItemsData() && &span.back() < ItemsData() + Size());
+}
 
 void NGFragmentItems::AssociateWithLayoutObject() const {
-  const Vector<scoped_refptr<const NGFragmentItem>>* items = &items_;
-  DCHECK(std::all_of(items->begin(), items->end(), [](const auto& item) {
+  DCHECK(std::all_of(Items().begin(), Items().end(), [](const auto& item) {
     return !item->DeltaToNextForSameLayoutObject();
   }));
   // items_[0] can be:
   //  - kBox  for list marker, e.g. <li>abc</li>
   //  - kLine for line, e.g. <div>abc</div>
-  // Calling get() is necessary below because operator<< in std::unique_ptr is
-  // a C++20 feature.
-  // TODO(https://crbug.com/980914): Drop .get() once we move to C++20.
-  DCHECK(items->IsEmpty() || (*items)[0]->IsContainer()) << (*items)[0].get();
+  DCHECK(Items().empty() || Items()[0]->IsContainer());
+  if (Size() <= 1)
+    return;
   HashMap<const LayoutObject*, wtf_size_t> last_fragment_map;
-  for (wtf_size_t index = 1u; index < items->size(); ++index) {
-    const NGFragmentItem& item = *(*items)[index];
-    if (!ShouldAssociateWithLayoutObject(item))
+  const Span items = Items();
+  wtf_size_t index = 0;
+  for (const scoped_refptr<const NGFragmentItem>& item : items.subspan(1)) {
+    ++index;
+    if (!ShouldAssociateWithLayoutObject(*item))
       continue;
-    LayoutObject* const layout_object = item.GetMutableLayoutObject();
-    DCHECK(layout_object->IsInLayoutNGInlineFormattingContext()) << item;
+    LayoutObject* const layout_object = item->GetMutableLayoutObject();
+    DCHECK(layout_object->IsInLayoutNGInlineFormattingContext()) << *item;
     auto insert_result = last_fragment_map.insert(layout_object, index);
     if (insert_result.is_new_entry) {
       DCHECK_EQ(layout_object->FirstInlineFragmentItemIndex(), 0u);
@@ -49,20 +69,20 @@ void NGFragmentItems::AssociateWithLayoutObject() const {
     }
     const wtf_size_t last_index = insert_result.stored_value->value;
     insert_result.stored_value->value = index;
-    DCHECK_GT(last_index, 0u) << item;
-    DCHECK_LT(last_index, items->size());
+    DCHECK_GT(last_index, 0u) << *item;
+    DCHECK_LT(last_index, items.size());
     DCHECK_LT(last_index, index);
-    DCHECK_EQ((*items)[last_index]->DeltaToNextForSameLayoutObject(), 0u);
-    (*items)[last_index]->SetDeltaToNextForSameLayoutObject(index - last_index);
+    DCHECK_EQ(items[last_index]->DeltaToNextForSameLayoutObject(), 0u);
+    items[last_index]->SetDeltaToNextForSameLayoutObject(index - last_index);
   }
 }
 
 void NGFragmentItems::ClearAssociatedFragments() const {
-  DCHECK(items_.IsEmpty() || items_[0]->IsContainer());
-  if (items_.size() <= 1)
+  DCHECK(Items().empty() || Items()[0]->IsContainer());
+  if (Size() <= 1)
     return;
   LayoutObject* last_object = nullptr;
-  for (const auto& item : base::make_span(items_.begin() + 1, items_.end())) {
+  for (const scoped_refptr<const NGFragmentItem>& item : Items().subspan(1)) {
     if (!ShouldAssociateWithLayoutObject(*item)) {
       // These items are not associated and that no need to clear.
       DCHECK_EQ(item->DeltaToNextForSameLayoutObject(), 0u);

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

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_INLINE_NG_FRAGMENT_ITEMS_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_INLINE_NG_FRAGMENT_ITEMS_H_
 
+#include "base/containers/span.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/layout/ng/inline/ng_fragment_item.h"
 
@@ -18,11 +19,14 @@ class NGFragmentItemsBuilder;
 // transformed to a flat list of |NGFragmentItem| and stored in this class.
 class CORE_EXPORT NGFragmentItems {
  public:
-  NGFragmentItems(NGFragmentItemsBuilder* builder);
+  explicit NGFragmentItems(NGFragmentItemsBuilder* builder);
+  ~NGFragmentItems();
 
-  const Vector<scoped_refptr<const NGFragmentItem>>& Items() const {
-    return items_;
-  }
+  wtf_size_t Size() const { return size_; }
+
+  using Span = base::span<const scoped_refptr<const NGFragmentItem>>;
+  Span Items() const { return base::make_span(ItemsData(), size_); }
+  bool IsSubSpan(const Span& span) const;
 
   const String& Text(bool first_line) const {
     return UNLIKELY(first_line) ? first_line_text_content_ : text_content_;
@@ -36,11 +40,30 @@ class CORE_EXPORT NGFragmentItems {
   static void LayoutObjectWillBeDestroyed(const LayoutObject& layout_object);
   static void LayoutObjectWillBeMoved(const LayoutObject& layout_object);
 
+  // The byte size of this instance.
+  constexpr static wtf_size_t ByteSizeFor(wtf_size_t count) {
+    return sizeof(NGFragmentItems) + count * sizeof(items_[0]);
+  }
+  wtf_size_t ByteSize() const { return ByteSizeFor(Size()); }
+
  private:
-  // TODO(kojii): inline capacity TBD.
-  Vector<scoped_refptr<const NGFragmentItem>> items_;
+  const scoped_refptr<const NGFragmentItem>* ItemsData() const {
+    return reinterpret_cast<const scoped_refptr<const NGFragmentItem>*>(items_);
+  }
+
   String text_content_;
   String first_line_text_content_;
+
+  wtf_size_t size_;
+
+  // Semantically, |items_| is a flexible array of |scoped_refptr<const
+  // NGFragmentItem>|, but |scoped_refptr| has non-trivial destruction which
+  // causes an error in clang. Declare as a flexible array of |NGFragmentItem*|
+  // instead. Please see |ItemsData()|.
+  static_assert(
+      sizeof(NGFragmentItem*) == sizeof(scoped_refptr<const NGFragmentItem>),
+      "scoped_refptr must be the size of a pointer for |ItemsData()| to work");
+  NGFragmentItem* items_[];
 };
 
 }  // namespace blink

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

@@ -154,7 +154,7 @@ void NGFragmentItemsBuilder::AddItems(const NGFragmentItems& items,
                                       TextDirection direction,
                                       const PhysicalSize& container_size) {
   DCHECK(items_.IsEmpty());
-  items_.AppendVector(items.Items());
+  items_.Append(items.Items().data(), items.Size());
 
   // Convert offsets to logical. The logic is opposite to |ConvertToPhysical|.
   // This is needed because the container size may be different, in that case,

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

@@ -25,6 +25,8 @@ class CORE_EXPORT NGFragmentItemsBuilder {
  public:
   NGFragmentItemsBuilder(NGBoxFragmentBuilder* box_builder) {}
 
+  wtf_size_t Size() const { return items_.size(); }
+
   // Returns true if we have any floating descendants which need to be
   // traversed during the float paint phase.
   bool HasFloatingDescendantsForPaint() const {

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

@@ -28,8 +28,7 @@ void NGInlineCursor::SetRoot(const NGFragmentItems& fragment_items,
   DCHECK(!HasRoot());
   fragment_items_ = &fragment_items;
   items_ = items;
-  DCHECK(items_.empty() || (items_.data() >= fragment_items_->Items().data() &&
-                            items_.data() < fragment_items_->Items().end()));
+  DCHECK(fragment_items_->IsSubSpan(items_));
   MoveToItem(items_.begin());
 }
 
@@ -872,8 +871,8 @@ void NGInlineCursor::MoveTo(const NGInlineCursorPosition& position) {
 
 inline unsigned NGInlineCursor::SpanIndexFromItemIndex(unsigned index) const {
   DCHECK(IsItemCursor());
-  DCHECK_GE(items_.data(), fragment_items_->Items().data());
-  DCHECK_LT(items_.data(), fragment_items_->Items().end());
+  DCHECK(!items_.empty());
+  DCHECK(fragment_items_->IsSubSpan(items_));
   if (items_.data() == fragment_items_->Items().data())
     return index;
   unsigned span_index = fragment_items_->Items().data() - items_.data() + index;

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

@@ -78,7 +78,7 @@ void GatherInlineContainerFragmentsFromLinebox(
 }
 
 void GatherInlineContainerFragmentsFromItems(
-    const Vector<scoped_refptr<const NGFragmentItem>>& items,
+    const NGFragmentItems::Span& items,
     const PhysicalOffset& box_offset,
     NGBoxFragmentBuilder::InlineContainingBlockMap* inline_containing_block_map,
     HashMap<const LayoutObject*, LineBoxPair>* containing_linebox_map) {

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

@@ -56,8 +56,8 @@ scoped_refptr<const NGPhysicalBoxFragment> NGPhysicalBoxFragment::Create(
                      sizeof(NGLink) * builder->children_.size() +
                      (borders.IsZero() ? 0 : sizeof(borders)) +
                      (padding.IsZero() ? 0 : sizeof(padding));
-  if (builder->ItemsBuilder())
-    byte_size += sizeof(NGFragmentItems);
+  if (const NGFragmentItemsBuilder* items_builder = builder->ItemsBuilder())
+    byte_size += NGFragmentItems::ByteSizeFor(items_builder->Size());
   // We store the children list inline in the fragment as a flexible
   // array. Therefore, we need to make sure to allocate enough space for
   // that array here, which requires a manual allocation + placement new.

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

@@ -159,9 +159,10 @@ class CORE_EXPORT NGPhysicalBoxFragment final
   const NGPhysicalBoxStrut* ComputeBordersAddress() const {
     DCHECK(has_borders_ || has_padding_);
     const NGFragmentItems* items = ComputeItemsAddress();
-    if (has_fragment_items_)
-      ++items;
-    return reinterpret_cast<const NGPhysicalBoxStrut*>(items);
+    if (!has_fragment_items_)
+      return reinterpret_cast<const NGPhysicalBoxStrut*>(items);
+    return reinterpret_cast<const NGPhysicalBoxStrut*>(
+        reinterpret_cast<const uint8_t*>(items) + items->ByteSize());
   }
 
   const NGPhysicalBoxStrut* ComputePaddingAddress() const {