[LayoutNG] Move should_create_box_fragment to LayoutInline

This patch moves the flag whether to force to create a box
fragment for LayoutInline or not from NGInlineItem to
LayoutInline.

This is part of an effort to reduce re-collecting
NGInlineItem when styles were changed. Re-collecting is
rather an expensive operation that changes in, for instance,
background should not require it.

Note, this patch itself does not reduce the collection,
because we require re-collection whenever NeedsLayout is set.
It is planned in future patches, but this change is needed to
make it happen.

Also benefits to keep the flag turned on once it's on, until
the LayoutInline is reattached.

The change has minor performance gain in most blink_perf.layout
https://pinpoint-dot-chromeperf.appspot.com/job/15f8c74be40000

Bug: 636993
Change-Id: I8d4e7c421d5fad26fee60832cdfbacd575f7ad34
Reviewed-on: https://chromium-review.googlesource.com/c/1333588Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608292}

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

@@ -35,6 +35,7 @@
 #include "third_party/blink/renderer/core/layout/line/inline_text_box.h"
 #include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_fragment_traversal.h"
 #include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_outline_utils.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
 #include "third_party/blink/renderer/core/paint/box_painter.h"
 #include "third_party/blink/renderer/core/paint/inline_painter.h"
@@ -59,6 +60,19 @@ const NGPhysicalBoxFragment* ContainingBlockFlowFragmentOf(
   return node.ContainingBlockFlowFragment();
 }
 
+// TODO(xiaochengh): Deduplicate with a similar function in ng_paint_fragment.cc
+// ::before, ::after and ::first-letter can be hit test targets.
+bool CanBeHitTestTargetPseudoNodeStyle(const ComputedStyle& style) {
+  switch (style.StyleType()) {
+    case kPseudoIdBefore:
+    case kPseudoIdAfter:
+    case kPseudoIdFirstLetter:
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // anonymous namespace
 
 struct SameSizeAsLayoutInline : public LayoutBoxModelObject {
@@ -299,17 +313,23 @@ void LayoutInline::StyleDidChange(StyleDifference diff,
     }
   }
 
-  if (!AlwaysCreateLineBoxes()) {
-    bool always_create_line_boxes_new =
-        HasSelfPaintingLayer() || HasBoxDecorationBackground() ||
-        new_style.HasPadding() || new_style.HasMargin() ||
-        new_style.HasOutline();
-    if (old_style && always_create_line_boxes_new) {
-      DirtyLineBoxes(false);
-      SetNeedsLayoutAndFullPaintInvalidation(
-          LayoutInvalidationReason::kStyleChange);
+  if (!IsInLayoutNGInlineFormattingContext()) {
+    if (!AlwaysCreateLineBoxes()) {
+      bool always_create_line_boxes_new =
+          HasSelfPaintingLayer() || HasBoxDecorationBackground() ||
+          new_style.HasPadding() || new_style.HasMargin() ||
+          new_style.HasOutline();
+      if (old_style && always_create_line_boxes_new) {
+        DirtyLineBoxes(false);
+        SetNeedsLayoutAndFullPaintInvalidation(
+            LayoutInvalidationReason::kStyleChange);
+      }
+      SetAlwaysCreateLineBoxes(always_create_line_boxes_new);
+    }
+  } else {
+    if (!ShouldCreateBoxFragment()) {
+      UpdateShouldCreateBoxFragment();
     }
-    SetAlwaysCreateLineBoxes(always_create_line_boxes_new);
   }
 
   // If we are changing to/from static, we need to reposition
@@ -336,6 +356,8 @@ void LayoutInline::StyleDidChange(StyleDifference diff,
 }
 
 void LayoutInline::UpdateAlwaysCreateLineBoxes(bool full_layout) {
+  DCHECK(!IsInLayoutNGInlineFormattingContext());
+
   // Once we have been tainted once, just assume it will happen again. This way
   // effects like hover highlighting that change the background color will only
   // cause a layout on the first rollover.
@@ -375,6 +397,49 @@ void LayoutInline::UpdateAlwaysCreateLineBoxes(bool full_layout) {
   }
 }
 
+bool LayoutInline::ComputeInitialShouldCreateBoxFragment(
+    const ComputedStyle& style) const {
+  if (style.HasBoxDecorationBackground() || style.HasPadding() ||
+      style.HasMargin())
+    return true;
+
+  return style.CanContainAbsolutePositionObjects() ||
+         style.CanContainFixedPositionObjects(false) ||
+         NGOutlineUtils::HasPaintedOutline(style, GetNode()) ||
+         CanBeHitTestTargetPseudoNodeStyle(style);
+}
+
+bool LayoutInline::ComputeInitialShouldCreateBoxFragment() const {
+  const ComputedStyle& style = StyleRef();
+  if (HasSelfPaintingLayer() || ComputeInitialShouldCreateBoxFragment(style) ||
+      ShouldApplyPaintContainment() || ShouldApplyLayoutContainment())
+    return true;
+
+  const ComputedStyle& first_line_style = FirstLineStyleRef();
+  if (UNLIKELY(&style != &first_line_style &&
+               ComputeInitialShouldCreateBoxFragment(first_line_style)))
+    return true;
+
+  return false;
+}
+
+void LayoutInline::UpdateShouldCreateBoxFragment() {
+  // Once we have been tainted once, just assume it will happen again. This way
+  // effects like hover highlighting that change the background color will only
+  // cause a layout on the first rollover.
+  if (IsInLayoutNGInlineFormattingContext()) {
+    if (ShouldCreateBoxFragment())
+      return;
+  } else {
+    SetIsInLayoutNGInlineFormattingContext(true);
+    SetShouldCreateBoxFragment(false);
+  }
+
+  if (ComputeInitialShouldCreateBoxFragment()) {
+    SetShouldCreateBoxFragment();
+  }
+}
+
 LayoutRect LayoutInline::LocalCaretRect(
     const InlineBox* inline_box,
     int,

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

@@ -213,13 +213,25 @@ class CORE_EXPORT LayoutInline : public LayoutBoxModelObject {
   using LayoutBoxModelObject::SetContinuation;
 
   bool AlwaysCreateLineBoxes() const {
+    DCHECK(!IsInLayoutNGInlineFormattingContext());
     return AlwaysCreateLineBoxesForLayoutInline();
   }
   void SetAlwaysCreateLineBoxes(bool always_create_line_boxes = true) {
+    DCHECK(!IsInLayoutNGInlineFormattingContext());
     SetAlwaysCreateLineBoxesForLayoutInline(always_create_line_boxes);
   }
   void UpdateAlwaysCreateLineBoxes(bool full_layout);
 
+  // True if this inline box should force creation of NGPhysicalBoxFragment.
+  bool ShouldCreateBoxFragment() const {
+    DCHECK(IsInLayoutNGInlineFormattingContext());
+    return AlwaysCreateLineBoxesForLayoutInline();
+  }
+  void SetShouldCreateBoxFragment(bool value = true) {
+    SetAlwaysCreateLineBoxesForLayoutInline(value);
+  }
+  void UpdateShouldCreateBoxFragment();
+
   LayoutRect LocalCaretRect(const InlineBox*,
                             int,
                             LayoutUnit* extra_width_to_end_of_line) const final;
@@ -276,6 +288,11 @@ class CORE_EXPORT LayoutInline : public LayoutBoxModelObject {
 
   bool IsLayoutInline() const final { return true; }
 
+  // Compute the initial value of |ShouldCreateBoxFragment()| for this
+  // LayoutInline. It maybe flipped to true later for other conditions.
+  bool ComputeInitialShouldCreateBoxFragment() const;
+  bool ComputeInitialShouldCreateBoxFragment(const ComputedStyle& style) const;
+
   LayoutRect CulledInlineVisualOverflowBoundingBox() const;
   InlineBox* CulledInlineFirstLineBox() const;
   InlineBox* CulledInlineLastLineBox() const;

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

@@ -6,7 +6,6 @@
 
 #include "third_party/blink/renderer/core/layout/layout_inline.h"
 #include "third_party/blink/renderer/core/layout/layout_object.h"
-#include "third_party/blink/renderer/core/layout/ng/ng_outline_utils.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/fonts/shaping/shape_result_buffer.h"
 
@@ -56,19 +55,6 @@ bool IsInlineBoxEndEmpty(const ComputedStyle& style,
   return true;
 }
 
-// TODO(xiaochengh): Deduplicate with a similar function in ng_paint_fragment.cc
-// ::before, ::after and ::first-letter can be hit test targets.
-bool CanBeHitTestTargetPseudoNodeStyle(const ComputedStyle& style) {
-  switch (style.StyleType()) {
-    case kPseudoIdBefore:
-    case kPseudoIdAfter:
-    case kPseudoIdFirstLetter:
-      return true;
-    default:
-      return false;
-  }
-}
-
 }  // namespace
 
 NGInlineItem::NGInlineItem(NGInlineItemType type,
@@ -87,7 +73,6 @@ NGInlineItem::NGInlineItem(NGInlineItemType type,
       bidi_level_(UBIDI_LTR),
       shape_options_(kPreContext | kPostContext),
       is_empty_item_(false),
-      should_create_box_fragment_(false),
       style_variant_(static_cast<unsigned>(NGStyleVariant::kStandard)),
       end_collapse_type_(kNotCollapsible),
       is_end_collapsible_newline_(false),
@@ -113,7 +98,6 @@ NGInlineItem::NGInlineItem(const NGInlineItem& other,
       bidi_level_(other.bidi_level_),
       shape_options_(other.shape_options_),
       is_empty_item_(other.is_empty_item_),
-      should_create_box_fragment_(other.should_create_box_fragment_),
       style_variant_(other.style_variant_),
       end_collapse_type_(other.end_collapse_type_),
       is_end_collapsible_newline_(other.is_end_collapsible_newline_),
@@ -124,9 +108,20 @@ NGInlineItem::NGInlineItem(const NGInlineItem& other,
 
 NGInlineItem::~NGInlineItem() = default;
 
+bool NGInlineItem::ShouldCreateBoxFragment() const {
+  if (Type() == kOpenTag || Type() == kCloseTag)
+    return ToLayoutInline(layout_object_)->ShouldCreateBoxFragment();
+  DCHECK_EQ(Type(), kAtomicInline);
+  return false;
+}
+
+void NGInlineItem::SetShouldCreateBoxFragment() {
+  DCHECK(Type() == kOpenTag || Type() == kCloseTag);
+  ToLayoutInline(layout_object_)->SetShouldCreateBoxFragment();
+}
+
 void NGInlineItem::ComputeBoxProperties() {
   DCHECK(!is_empty_item_);
-  DCHECK(!should_create_box_fragment_);
 
   if (type_ == NGInlineItem::kText || type_ == NGInlineItem::kAtomicInline ||
       type_ == NGInlineItem::kControl)
@@ -134,24 +129,7 @@ void NGInlineItem::ComputeBoxProperties() {
 
   if (type_ == NGInlineItem::kOpenTag) {
     DCHECK(style_ && layout_object_ && layout_object_->IsLayoutInline());
-    if (style_->HasBoxDecorationBackground() || style_->HasPadding() ||
-        style_->HasMargin()) {
-      is_empty_item_ = IsInlineBoxStartEmpty(*style_, *layout_object_);
-      should_create_box_fragment_ = true;
-    } else {
-      is_empty_item_ = true;
-      should_create_box_fragment_ =
-          ToLayoutBoxModelObject(layout_object_)->HasSelfPaintingLayer() ||
-          style_->CanContainAbsolutePositionObjects() ||
-          style_->CanContainFixedPositionObjects(false) ||
-          NGOutlineUtils::HasPaintedOutline(*style_,
-                                            layout_object_->GetNode()) ||
-          ToLayoutBoxModelObject(layout_object_)
-              ->ShouldApplyPaintContainment() ||
-          ToLayoutBoxModelObject(layout_object_)
-              ->ShouldApplyLayoutContainment() ||
-          CanBeHitTestTargetPseudoNodeStyle(*style_);
-    }
+    is_empty_item_ = IsInlineBoxStartEmpty(*style_, *layout_object_);
     return;
   }
 

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

@@ -91,8 +91,8 @@ class CORE_EXPORT NGInlineItem {
 
   // If this item should create a box fragment. Box fragments can be omitted for
   // optimization if this is false.
-  bool ShouldCreateBoxFragment() const { return should_create_box_fragment_; }
-  void SetShouldCreateBoxFragment() { should_create_box_fragment_ = true; }
+  bool ShouldCreateBoxFragment() const;
+  void SetShouldCreateBoxFragment();
 
   unsigned StartOffset() const { return start_offset_; }
   unsigned EndOffset() const { return end_offset_; }
@@ -196,7 +196,6 @@ class CORE_EXPORT NGInlineItem {
   unsigned bidi_level_ : 8;              // UBiDiLevel is defined as uint8_t.
   unsigned shape_options_ : 2;
   unsigned is_empty_item_ : 1;
-  unsigned should_create_box_fragment_ : 1;
   unsigned style_variant_ : 2;
   unsigned end_collapse_type_ : 2;  // NGCollapseType
   unsigned is_end_collapsible_newline_ : 1;

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

@@ -405,6 +405,7 @@ static std::unique_ptr<LayoutInline> CreateLayoutInline(
   initialize_style(style.get());
   std::unique_ptr<LayoutInline> node = std::make_unique<LayoutInline>(nullptr);
   node->SetStyleInternal(std::move(style));
+  node->SetIsInLayoutNGInlineFormattingContext(true);
   return node;
 }
 

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

@@ -159,20 +159,22 @@ void CollectInlinesInternal(
       // should not appear. LayoutObject tree should have created an anonymous
       // box to prevent having inline/block-mixed children.
       DCHECK(node->IsInline());
+      LayoutInline* layout_inline = ToLayoutInline(node);
+      layout_inline->UpdateShouldCreateBoxFragment();
 
-      builder->EnterInline(node);
+      builder->EnterInline(layout_inline);
 
       // Traverse to children if they exist.
-      if (LayoutObject* child = node->SlowFirstChild()) {
+      if (LayoutObject* child = layout_inline->FirstChild()) {
         node = child;
         continue;
       }
 
       // An empty inline node.
-      builder->ExitInline(node);
+      builder->ExitInline(layout_inline);
 
       if (update_layout)
-        ClearNeedsLayout(node);
+        ClearNeedsLayout(layout_inline);
     }
 
     // Find the next sibling, or parent, until we reach |block|.
@@ -661,7 +663,7 @@ void NGInlineNode::ShapeTextForFirstLineIfNeeded(NGInlineNodeData* data) {
           item.layout_object_->IsLayoutInline() &&
           item.layout_object_->Parent() == GetLayoutBox() &&
           ToLayoutInline(item.layout_object_)->IsFirstLineAnonymous()) {
-        item.should_create_box_fragment_ = true;
+        item.SetShouldCreateBoxFragment();
       }
       break;
     }