[layout] Ensure fragment tree consistency.

This patch ensures that we always have a "consistent" fragment-tree (or
rather a portion of a fragment-tree if we have legacy nodes).

Previously it was possible for a fragment-tree to become inconsistent
whenever we performed layout from a subtree root. Now when we layout
from a node like this, we rebuild the "spine" of the tree.

After this change we'll be able to remove most "PostLayout" calls,
except where they are used for building this "consistent" tree.

Bug: 1066616
Change-Id: I3993fcc5be1cd9043555ecad553fc569fc985314
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407032
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807866}

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

@@ -677,33 +677,51 @@ void LayoutBox::UpdateFromStyle() {
 
 void LayoutBox::LayoutSubtreeRoot() {
   if (RuntimeEnabledFeatures::LayoutNGEnabled() &&
-      !NGBlockNode::CanUseNewLayout(*this)) {
+      !NGBlockNode::CanUseNewLayout(*this) && GetCachedLayoutResult()) {
     // If this object is laid out by the legacy engine, while its containing
     // block is laid out by NG, it means that we normally (when laying out
     // starting at the real root, i.e. LayoutView) enter layout of this object
     // from NG code. This takes care of setting up a BoxLayoutExtraInput
     // structure, which makes legacy layout behave when managed by NG. Make a
     // short detour via NG just to set things up to re-enter legacy layout
     // correctly.
-    if (const NGLayoutResult* result = GetCachedLayoutResult()) {
-      DCHECK_EQ(PhysicalFragmentCount(), 1u);
-      LayoutPoint old_location = Location();
+    DCHECK_EQ(PhysicalFragmentCount(), 1u);
+    LayoutPoint old_location = Location();
 
-      // Make a copy of the cached constraint space, since we'll overwrite the
-      // layout result object as part of performing layout.
-      auto constraint_space = result->GetConstraintSpaceForCaching();
+    // Make a copy of the cached constraint space, since we'll overwrite the
+    // layout result object as part of performing layout.
+    auto constraint_space =
+        GetCachedLayoutResult()->GetConstraintSpaceForCaching();
 
-      NGBlockNode(this).Layout(constraint_space);
+    NGBlockNode(this).Layout(constraint_space);
 
-      // Restore the old location. While it's usually the job of the containing
-      // block to position its children, out-of-flow positioned objects set
-      // their own position, which could be wrong in this case.
-      SetLocation(old_location);
-      return;
-    }
+    // Restore the old location. While it's usually the job of the containing
+    // block to position its children, out-of-flow positioned objects set
+    // their own position, which could be wrong in this case.
+    SetLocation(old_location);
+  } else {
+    UpdateLayout();
   }
 
-  UpdateLayout();
+  // If this box has an associated layout-result, rebuild the spine of the
+  // fragment-tree to ensure consistency.
+  if (PhysicalFragmentCount() &&
+      RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
+    LayoutBlock* cb = ContainingBlock();
+    while (NGBlockNode::CanUseNewLayout(*cb)) {
+      if (cb->measure_result_) {
+        cb->measure_result_ =
+            NGLayoutResult::CloneWithPostLayoutFragments(*cb->measure_result_);
+      }
+      for (scoped_refptr<const NGLayoutResult>& layout_result :
+           cb->layout_results_) {
+        // Create and set a new identical result.
+        layout_result =
+            NGLayoutResult::CloneWithPostLayoutFragments(*layout_result);
+      }
+      cb = cb->ContainingBlock();
+    }
+  }
 }
 
 void LayoutBox::UpdateLayout() {

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

@@ -134,7 +134,7 @@ NGFragmentItem::NGFragmentItem(const NGPhysicalLineBoxFragment& line)
 NGFragmentItem::NGFragmentItem(const NGPhysicalBoxFragment& box,
                                TextDirection resolved_direction)
     : layout_object_(box.GetLayoutObject()),
-      box_({&box, /* descendants_count */ 1}),
+      box_(&box, /* descendants_count */ 1),
       rect_({PhysicalOffset(), box.Size()}),
       type_(kBox),
       style_variant_(static_cast<unsigned>(box.StyleVariant())),
@@ -214,10 +214,7 @@ NGFragmentItem::NGFragmentItem(const NGFragmentItem& source)
       break;
   }
 
-  // Copy |ink_overflow_| only for text items, because ink overflow for other
-  // items may be chnaged even in simplified layout or when reusing lines, and
-  // that they need to be re-computed anyway.
-  if (IsText() && source.IsInkOverflowComputed()) {
+  if (source.IsInkOverflowComputed()) {
     ink_overflow_type_ = source.InkOverflowType();
     new (&ink_overflow_)
         NGInkOverflow(source.InkOverflowType(), source.ink_overflow_);
@@ -335,6 +332,16 @@ bool NGFragmentItem::HasSelfPaintingLayer() const {
   return false;
 }
 
+NGFragmentItem::BoxItem::BoxItem(const BoxItem& other)
+    : box_fragment(other.box_fragment->PostLayout()),
+      descendants_count(other.descendants_count) {}
+
+NGFragmentItem::BoxItem::BoxItem(
+    scoped_refptr<const NGPhysicalBoxFragment> box_fragment,
+    wtf_size_t descendants_count)
+    : box_fragment(std::move(box_fragment)),
+      descendants_count(descendants_count) {}
+
 const NGPhysicalBoxFragment* NGFragmentItem::BoxItem::PostLayout() const {
   if (box_fragment)
     return box_fragment->PostLayout();

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

@@ -33,9 +33,8 @@ class CORE_EXPORT NGFragmentItem {
   // Represents regular text that exists in the DOM.
   struct TextItem {
     scoped_refptr<const ShapeResultView> shape_result;
-    // TODO(kojii): |start_offset| and |end_offset| should match to the offset
-    // in |shape_result|. Consider if we should remove them, or if keeping them
-    // is easier.
+    // TODO(kojii): |text_offset| should match to the offset in |shape_result|.
+    // Consider if we should remove them, or if keeping them is easier.
     const NGTextOffset text_offset;
   };
   // Represents text generated by the layout engine, e.g., hyphen or ellipsis.
@@ -51,6 +50,11 @@ class CORE_EXPORT NGFragmentItem {
   // Represents a box fragment appeared in a line. This includes inline boxes
   // (e.g., <span>text</span>) and atomic inlines.
   struct BoxItem {
+    // This copy constructor looks up the "post-layout" fragment.
+    BoxItem(const BoxItem&);
+    BoxItem(scoped_refptr<const NGPhysicalBoxFragment>,
+            wtf_size_t descendants_count);
+
     // If this item is an inline box, its children are stored as following
     // items. |descendants_count_| has the number of such items.
     //

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

@@ -106,15 +106,14 @@ TEST_F(NGFragmentItemTest, CopyMove) {
   EXPECT_NE(line_item->LineBoxFragment(), nullptr);
   NGFragmentItem copy_of_line(*line_item);
   EXPECT_EQ(copy_of_line.LineBoxFragment(), line_item->LineBoxFragment());
-  // Ink overflow is not copied for line items. See |NGFragmentItem| copy ctor.
-  EXPECT_FALSE(copy_of_line.IsInkOverflowComputed());
+  EXPECT_TRUE(copy_of_line.IsInkOverflowComputed());
 
   // Test moving a line item.
   NGFragmentItem move_of_line(std::move(copy_of_line));
   EXPECT_EQ(move_of_line.LineBoxFragment(), line_item->LineBoxFragment());
   // After the move, the source fragment should be released.
   EXPECT_EQ(copy_of_line.LineBoxFragment(), nullptr);
-  EXPECT_FALSE(move_of_line.IsInkOverflowComputed());
+  EXPECT_TRUE(move_of_line.IsInkOverflowComputed());
 
   // To test moving ink overflow, add an ink overflow to |move_of_line|.
   PhysicalRect not_small_ink_overflow_rect(0, 0, 5000, 100);

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

@@ -44,13 +44,22 @@ NGFragmentItems::NGFragmentItems(NGFragmentItemsBuilder* builder)
       size_(builder->items_.size()),
       size_of_earlier_fragments_(0) {
   NGFragmentItemsBuilder::ItemWithOffsetList& source_items = builder->items_;
-  for (unsigned i = 0; i < size_; ++i) {
+  for (wtf_size_t i = 0; i < size_; ++i) {
     // Call the move constructor to move without |AddRef|. Items in
     // |NGFragmentItemsBuilder| are not used after |this| was constructed.
     new (&items_[i]) NGFragmentItem(std::move(source_items[i].item));
   }
 }
 
+NGFragmentItems::NGFragmentItems(const NGFragmentItems& other)
+    : text_content_(other.text_content_),
+      first_line_text_content_(other.first_line_text_content_),
+      size_(other.size_),
+      size_of_earlier_fragments_(other.size_of_earlier_fragments_) {
+  for (wtf_size_t i = 0; i < size_; ++i)
+    new (&items_[i]) NGFragmentItem(other.items_[i]);
+}
+
 NGFragmentItems::~NGFragmentItems() {
   for (unsigned i = 0; i < size_; ++i)
     items_[i].~NGFragmentItem();

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

@@ -19,6 +19,7 @@ class NGFragmentItemsBuilder;
 // transformed to a flat list of |NGFragmentItem| and stored in this class.
 class CORE_EXPORT NGFragmentItems {
  public:
+  NGFragmentItems(const NGFragmentItems& other);
   explicit NGFragmentItems(NGFragmentItemsBuilder* builder);
   ~NGFragmentItems();
 

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

@@ -38,6 +38,14 @@ ASSERT_SIZE(NGLayoutResult, SameSizeAsNGLayoutResult);
 
 }  // namespace
 
+// static
+scoped_refptr<const NGLayoutResult>
+NGLayoutResult::CloneWithPostLayoutFragments(const NGLayoutResult& other) {
+  return base::AdoptRef(new NGLayoutResult(
+      other, NGPhysicalBoxFragment::CloneWithPostLayoutFragments(
+                 To<NGPhysicalBoxFragment>(other.PhysicalFragment()))));
+}
+
 NGLayoutResult::NGLayoutResult(
     NGBoxFragmentBuilderPassKey passkey,
     scoped_refptr<const NGPhysicalContainerFragment> physical_fragment,
@@ -159,6 +167,29 @@ NGLayoutResult::NGLayoutResult(const NGLayoutResult& other,
 #endif
 }
 
+NGLayoutResult::NGLayoutResult(
+    const NGLayoutResult& other,
+    scoped_refptr<const NGPhysicalContainerFragment> physical_fragment)
+    : space_(other.space_),
+      physical_fragment_(std::move(physical_fragment)),
+      intrinsic_block_size_(other.intrinsic_block_size_),
+      bitfields_(other.bitfields_) {
+  if (HasRareData()) {
+    rare_data_ = new RareData(*other.rare_data_);
+  } else if (!bitfields_.has_oof_positioned_offset) {
+    bfc_offset_ = other.bfc_offset_;
+  } else {
+    DCHECK(physical_fragment_->IsOutOfFlowPositioned());
+    oof_positioned_offset_ = other.oof_positioned_offset_;
+  }
+
+  DCHECK_EQ(physical_fragment_->Size(), other.physical_fragment_->Size());
+
+#if DCHECK_IS_ON()
+  has_valid_space_ = other.has_valid_space_;
+#endif
+}
+
 NGLayoutResult::NGLayoutResult(
     scoped_refptr<const NGPhysicalContainerFragment> physical_fragment,
     NGContainerFragmentBuilder* builder)

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

@@ -47,6 +47,11 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
     // large enough to store.
   };
 
+  // Creates a copy of |other| but uses the "post-layout" fragments to ensure
+  // fragment-tree consistency.
+  static scoped_refptr<const NGLayoutResult> CloneWithPostLayoutFragments(
+      const NGLayoutResult& other);
+
   // Create a copy of NGLayoutResult with |BfcBlockOffset| replaced by the given
   // parameter. Note, when |bfc_block_offset| is |nullopt|, |BfcBlockOffset| is
   // still replaced with |nullopt|.
@@ -340,6 +345,11 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
  private:
   friend class MutableForOutOfFlow;
 
+  // Creates a copy of NGLayoutResult with a new (but "identical") fragment.
+  NGLayoutResult(
+      const NGLayoutResult& other,
+      scoped_refptr<const NGPhysicalContainerFragment> physical_fragment);
+
   // We don't need the copy constructor, move constructor, copy
   // assigmnment-operator, or move assignment-operator today.
   // Delete these to clarify that they will not work because a |RefCounted|

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

@@ -38,6 +38,7 @@ bool HasControlClip(const NGPhysicalBoxFragment& self) {
 
 }  // namespace
 
+// static
 scoped_refptr<const NGPhysicalBoxFragment> NGPhysicalBoxFragment::Create(
     NGBoxFragmentBuilder* builder,
     WritingMode block_or_line_writing_mode) {
@@ -54,16 +55,12 @@ scoped_refptr<const NGPhysicalBoxFragment> NGPhysicalBoxFragment::Create(
       builder->table_collapsed_borders_ ||
       builder->table_collapsed_borders_geometry_ ||
       builder->table_cell_column_index_;
-  size_t byte_size = sizeof(NGPhysicalBoxFragment) +
-                     sizeof(NGLink) * builder->children_.size() +
-                     (borders.IsZero() ? 0 : sizeof(borders)) +
-                     (padding.IsZero() ? 0 : sizeof(padding)) +
-                     (has_rare_data ? sizeof(RareData) : 0);
-  if (const NGFragmentItemsBuilder* items_builder = builder->ItemsBuilder()) {
-    // Omit |NGFragmentItems| if there were no items; e.g., display-lock.
-    if (items_builder->Size())
-      byte_size += NGFragmentItems::ByteSizeFor(items_builder->Size());
-  }
+
+  wtf_size_t num_fragment_items =
+      builder->ItemsBuilder() ? builder->ItemsBuilder()->Size() : 0;
+  size_t byte_size =
+      ByteSize(num_fragment_items, builder->children_.size(), !borders.IsZero(),
+               !padding.IsZero(), has_rare_data);
 
   // We store the children list inline in the fragment as a flexible
   // array. Therefore, we need to make sure to allocate enough space for
@@ -77,6 +74,36 @@ scoped_refptr<const NGPhysicalBoxFragment> NGPhysicalBoxFragment::Create(
   return base::AdoptRef(static_cast<NGPhysicalBoxFragment*>(data));
 }
 
+// static
+scoped_refptr<const NGPhysicalBoxFragment>
+NGPhysicalBoxFragment::CloneWithPostLayoutFragments(
+    const NGPhysicalBoxFragment& other) {
+  // The size of the new fragment shouldn't differ from the old one.
+  wtf_size_t num_fragment_items = other.Items() ? other.Items()->Size() : 0;
+  size_t byte_size =
+      ByteSize(num_fragment_items, other.num_children_, other.has_borders_,
+               other.has_padding_, other.has_rare_data_);
+
+  void* data = ::WTF::Partitions::FastMalloc(
+      byte_size, ::WTF::GetStringWithTypeName<NGPhysicalBoxFragment>());
+  new (data) NGPhysicalBoxFragment(PassKey(), other);
+  return base::AdoptRef(static_cast<NGPhysicalBoxFragment*>(data));
+}
+
+// static
+size_t NGPhysicalBoxFragment::ByteSize(wtf_size_t num_fragment_items,
+                                       wtf_size_t num_children,
+                                       bool has_borders,
+                                       bool has_padding,
+                                       bool has_rare_data) {
+  return sizeof(NGPhysicalBoxFragment) +
+         NGFragmentItems::ByteSizeFor(num_fragment_items) +
+         sizeof(NGLink) * num_children +
+         (has_borders ? sizeof(NGPhysicalBoxStrut) : 0) +
+         (has_padding ? sizeof(NGPhysicalBoxStrut) : 0) +
+         (has_rare_data ? sizeof(NGPhysicalBoxFragment::RareData) : 0);
+}
+
 NGPhysicalBoxFragment::NGPhysicalBoxFragment(
     PassKey key,
     NGBoxFragmentBuilder* builder,
@@ -155,6 +182,30 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
 #endif
 }
 
+NGPhysicalBoxFragment::NGPhysicalBoxFragment(PassKey key,
+                                             const NGPhysicalBoxFragment& other)
+    : NGPhysicalContainerFragment(other, children_),
+      baseline_(other.baseline_),
+      last_baseline_(other.last_baseline_) {
+  if (has_fragment_items_) {
+    NGFragmentItems* items =
+        const_cast<NGFragmentItems*>(ComputeItemsAddress());
+    new (items) NGFragmentItems(*other.ComputeItemsAddress());
+  }
+  if (has_borders_) {
+    *const_cast<NGPhysicalBoxStrut*>(ComputeBordersAddress()) =
+        *other.ComputeBordersAddress();
+  }
+  if (has_padding_) {
+    *const_cast<NGPhysicalBoxStrut*>(ComputePaddingAddress()) =
+        *other.ComputePaddingAddress();
+  }
+  if (has_rare_data_) {
+    new (const_cast<RareData*>(ComputeRareDataAddress()))
+        RareData(*other.ComputeRareDataAddress());
+  }
+}
+
 NGPhysicalBoxFragment::RareData::RareData(NGBoxFragmentBuilder* builder,
                                           PhysicalSize size)
     : mathml_paint_info(std::move(builder->mathml_paint_info_)) {
@@ -190,6 +241,22 @@ NGPhysicalBoxFragment::RareData::RareData(NGBoxFragmentBuilder* builder,
     table_cell_column_index = *builder->table_cell_column_index_;
 }
 
+NGPhysicalBoxFragment::RareData::RareData(const RareData& other)
+    : oof_positioned_fragmentainer_descendants(
+          other.oof_positioned_fragmentainer_descendants),
+      mathml_paint_info(other.mathml_paint_info
+                            ? new NGMathMLPaintInfo(*other.mathml_paint_info)
+                            : nullptr),
+      table_grid_rect(other.table_grid_rect),
+      table_column_geometries(other.table_column_geometries),
+      table_collapsed_borders(other.table_collapsed_borders),
+      table_collapsed_borders_geometry(
+          other.table_collapsed_borders_geometry
+              ? new NGTableFragmentData::CollapsedBordersGeometry(
+                    *other.table_collapsed_borders_geometry)
+              : nullptr),
+      table_cell_column_index(other.table_cell_column_index) {}
+
 scoped_refptr<const NGLayoutResult>
 NGPhysicalBoxFragment::CloneAsHiddenForPaint() const {
   const ComputedStyle& style = Style();