Move inline box placeholders to before children

This patch changes |NGInlineLayoutAlgorithm| to place inline
box placeholders to before children.

Before this patch, they were created after children, only
when the inline box is not culled. This change:
1. Reduces the insertion to |Vector| in |CreateBoxFragments|,
   which involves memory copy. This is not common today, but
   will be common when |NGFragmentItem| is enabled.
2. Allows easier implementation to "disable culled inline
   boxes" when |NGFragmetnItem| is enabled.

There should be no behavior changes.

Bug: 982194
Change-Id: I0d7077e4c9b78b99276afc2e7919258b33bd4ea6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980663Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728439}

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

@@ -82,15 +82,15 @@ void NGFragmentItemsBuilder::AddItems(Child* child_begin, Child* child_end) {
       // Floats are in the fragment tree, not in the fragment item list.
       DCHECK(!box.IsFloating());
 
+      // Take the fast path when we know |child| does not have child items.
       if (child.children_count <= 1) {
         // Compute |has_floating_descendants_for_paint_| to optimize tree
         // traversal in paint.
         if (!has_floating_descendants_for_paint_ && box.IsFloating())
           has_floating_descendants_for_paint_ = true;
 
-        DCHECK(child.HasBidiLevel());
         items_.push_back(std::make_unique<NGFragmentItem>(
-            box, 1, DirectionFromLevel(child.bidi_level)));
+            box, 1, child.ResolvedDirection()));
         offsets_.push_back(child.offset);
         ++child_iter;
         continue;
@@ -116,9 +116,8 @@ void NGFragmentItemsBuilder::AddItems(Child* child_begin, Child* child_end) {
       wtf_size_t item_count = items_.size() - box_start_index;
 
       // Create an item for the start of the box.
-      DCHECK(child.HasBidiLevel());
       items_[box_start_index] = std::make_unique<NGFragmentItem>(
-          box, item_count, DirectionFromLevel(child.bidi_level));
+          box, item_count, child.ResolvedDirection());
       continue;
     }
 

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

@@ -116,14 +116,22 @@ class CORE_EXPORT NGInlineLayoutStateStack {
 
   // Initialize the box state stack for a new line.
   // @return The initial box state for the line.
-  NGInlineBoxState* OnBeginPlaceItems(const ComputedStyle&, FontBaseline, bool);
+  NGInlineBoxState* OnBeginPlaceItems(
+      const ComputedStyle&,
+      FontBaseline,
+      bool line_height_quirk,
+      NGLineBoxFragmentBuilder::ChildList* line_box);
 
   // Push a box state stack.
   NGInlineBoxState* OnOpenTag(const NGInlineItem&,
                               const NGInlineItemResult&,
+                              FontBaseline baseline_type,
                               const NGLineBoxFragmentBuilder::ChildList&);
-  NGInlineBoxState* OnOpenTag(const ComputedStyle&,
-                              const NGLineBoxFragmentBuilder::ChildList&);
+  // This variation adds a box placeholder to |line_box|.
+  NGInlineBoxState* OnOpenTag(const NGInlineItem&,
+                              const NGInlineItemResult&,
+                              FontBaseline baseline_type,
+                              NGLineBoxFragmentBuilder::ChildList* line_box);
 
   // Pop a box state stack.
   NGInlineBoxState* OnCloseTag(NGLineBoxFragmentBuilder::ChildList*,
@@ -182,6 +190,7 @@ class CORE_EXPORT NGInlineLayoutStateStack {
   void AddBoxFragmentPlaceholder(NGInlineBoxState*,
                                  NGLineBoxFragmentBuilder::ChildList*,
                                  FontBaseline);
+  void AddBoxData(NGInlineBoxState*, NGLineBoxFragmentBuilder::ChildList*);
 
   enum PositionPending { kPositionNotPending, kPositionPending };
 

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

@@ -77,8 +77,10 @@ NGInlineLayoutAlgorithm::~NGInlineLayoutAlgorithm() = default;
 NGInlineBoxState* NGInlineLayoutAlgorithm::HandleOpenTag(
     const NGInlineItem& item,
     const NGInlineItemResult& item_result,
+    NGLineBoxFragmentBuilder::ChildList* line_box,
     NGInlineLayoutStateStack* box_states) const {
-  NGInlineBoxState* box = box_states->OnOpenTag(item, item_result, line_box_);
+  NGInlineBoxState* box =
+      box_states->OnOpenTag(item, item_result, baseline_type_, line_box);
   // Compute text metrics for all inline boxes since even empty inlines
   // influence the line height, except when quirks mode and the box is empty
   // for the purpose of empty block calculation.
@@ -160,12 +162,13 @@ void NGInlineLayoutAlgorithm::RebuildBoxStates(
   }
 
   // Create box states for tags that are not closed yet.
+  NGLineBoxFragmentBuilder::ChildList line_box;
   box_states->OnBeginPlaceItems(line_info.LineStyle(), baseline_type_,
-                                quirks_mode_);
+                                quirks_mode_, &line_box);
   for (const NGInlineItem* item : open_items) {
     NGInlineItemResult item_result;
     NGLineBreaker::ComputeOpenTagResult(*item, ConstraintSpace(), &item_result);
-    HandleOpenTag(*item, item_result, box_states);
+    HandleOpenTag(*item, item_result, &line_box, box_states);
   }
 }
 
@@ -175,9 +178,9 @@ void NGInlineLayoutAlgorithm::CheckBoxStates(
     const NGInlineBreakToken* break_token) const {
   NGInlineLayoutStateStack rebuilt;
   RebuildBoxStates(line_info, break_token, &rebuilt);
-  rebuilt.OnBeginPlaceItems(line_info.LineStyle(), baseline_type_,
-                            quirks_mode_);
-
+  NGLineBoxFragmentBuilder::ChildList line_box;
+  rebuilt.OnBeginPlaceItems(line_info.LineStyle(), baseline_type_, quirks_mode_,
+                            &line_box);
   DCHECK(box_states_);
   box_states_->CheckSame(rebuilt);
 }
@@ -201,8 +204,8 @@ void NGInlineLayoutAlgorithm::CreateLine(
   // The baseline is adjusted after the height of the line box is computed.
   const ComputedStyle& line_style = line_info->LineStyle();
   box_states_->SetIsEmptyLine(line_info->IsEmptyLine());
-  NGInlineBoxState* box =
-      box_states_->OnBeginPlaceItems(line_style, baseline_type_, quirks_mode_);
+  NGInlineBoxState* box = box_states_->OnBeginPlaceItems(
+      line_style, baseline_type_, quirks_mode_, &line_box_);
 #if DCHECK_IS_ON()
   if (is_box_states_from_context_)
     CheckBoxStates(*line_info, BreakToken());
@@ -261,7 +264,7 @@ void NGInlineLayoutAlgorithm::CreateLine(
     } else if (item.Type() == NGInlineItem::kControl) {
       PlaceControlItem(item, *line_info, &item_result, box);
     } else if (item.Type() == NGInlineItem::kOpenTag) {
-      box = HandleOpenTag(item, item_result, box_states_);
+      box = HandleOpenTag(item, item_result, &line_box_, box_states_);
     } else if (item.Type() == NGInlineItem::kCloseTag) {
       box = HandleCloseTag(item, item_result, box);
     } else if (item.Type() == NGInlineItem::kAtomicInline) {
@@ -459,7 +462,8 @@ NGInlineBoxState* NGInlineLayoutAlgorithm::PlaceAtomicInline(
   // position += item_result->margins.LineLeft(style.Direction());
 
   item_result->has_edge = true;
-  NGInlineBoxState* box = box_states_->OnOpenTag(item, *item_result, line_box_);
+  NGInlineBoxState* box =
+      box_states_->OnOpenTag(item, *item_result, baseline_type_, line_box_);
   PlaceLayoutResult(item_result, box, box->margin_inline_start);
   return box_states_->OnCloseTag(&line_box_, box, baseline_type_);
 }
@@ -485,7 +489,8 @@ void NGInlineLayoutAlgorithm::PlaceLayoutResult(NGInlineItemResult* item_result,
   LayoutUnit line_top = item_result->margins.line_over - metrics.ascent;
   line_box_.AddChild(std::move(item_result->layout_result),
                      LogicalOffset{inline_offset, line_top},
-                     item_result->inline_size, item.BidiLevel());
+                     item_result->inline_size, /* children_count */ 0,
+                     item.BidiLevel());
 }
 
 // Place all out-of-flow objects in |line_box_|.
@@ -1086,22 +1091,9 @@ void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction) {
 
   // For opaque items, copy bidi levels from adjacent items.
   if (has_opaque_items) {
-    UBiDiLevel last_level = levels.front();
-    if (last_level == kOpaqueBidiLevel) {
-      for (const UBiDiLevel level : levels) {
-        if (level != kOpaqueBidiLevel) {
-          last_level = level;
-          break;
-        }
-      }
-    }
-    // If all items are opaque, use the base direction.
-    if (last_level == kOpaqueBidiLevel) {
-      if (IsLtr(base_direction))
-        return;
-      last_level = 1;
-    }
-    for (UBiDiLevel& level : levels) {
+    // Use the paragraph level for trailing opaque items.
+    UBiDiLevel last_level = IsLtr(base_direction) ? 0 : 1;
+    for (UBiDiLevel& level : base::Reversed(levels)) {
       if (level == kOpaqueBidiLevel)
         level = last_level;
       else

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

@@ -69,6 +69,7 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
 
   NGInlineBoxState* HandleOpenTag(const NGInlineItem&,
                                   const NGInlineItemResult&,
+                                  NGLineBoxFragmentBuilder::ChildList*,
                                   NGInlineLayoutStateStack*) const;
   NGInlineBoxState* HandleCloseTag(const NGInlineItem&,
                                    const NGInlineItemResult&,

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

@@ -82,6 +82,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
     // The offset of the border box, initially in this child coordinate system.
     // |ComputeInlinePositions()| converts it to the offset within the line box.
     LogicalOffset offset;
+    LogicalSize size;
     // The offset of a positioned float wrt. the root BFC. This should only be
     // set for positioned floats.
     NGBfcOffset bfc_offset;
@@ -103,7 +104,8 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
     Child() = default;
     // Create a placeholder. A placeholder does not have a fragment nor a bidi
     // level.
-    Child(LogicalOffset offset) : offset(offset) {}
+    Child(LayoutUnit block_offset, LayoutUnit block_size)
+        : offset(LayoutUnit(), block_offset), size(LayoutUnit(), block_size) {}
     // Crete a bidi control. A bidi control does not have a fragment, but has
     // bidi level and affects bidi reordering.
     Child(UBiDiLevel bidi_level) : bidi_level(bidi_level) {}
@@ -111,10 +113,12 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
     Child(scoped_refptr<const NGLayoutResult> layout_result,
           LogicalOffset offset,
           LayoutUnit inline_size,
+          unsigned children_count,
           UBiDiLevel bidi_level)
         : layout_result(std::move(layout_result)),
           offset(offset),
           inline_size(inline_size),
+          children_count(children_count),
           bidi_level(bidi_level) {}
     // Create an in-flow |NGPhysicalTextFragment|.
     Child(scoped_refptr<const NGPhysicalTextFragment> fragment,
@@ -185,6 +189,12 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
         return &layout_result->PhysicalFragment();
       return fragment.get();
     }
+    TextDirection ResolvedDirection() const {
+      // Inline boxes are not leaves that they don't have directions.
+      DCHECK(HasBidiLevel() || layout_result->PhysicalFragment().IsInlineBox());
+      return HasBidiLevel() ? DirectionFromLevel(bidi_level)
+                            : TextDirection::kLtr;
+    }
   };
 
   // A vector of Child.
@@ -235,10 +245,10 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
     void InsertChild(unsigned index,
                      scoped_refptr<const NGLayoutResult> layout_result,
                      const LogicalOffset& offset,
-                     LayoutUnit inline_size,
-                     UBiDiLevel bidi_level) {
+                     unsigned children_count) {
       children_.insert(index, Child{std::move(layout_result), offset,
-                                    inline_size, bidi_level});
+                                    /* inline_size */ LayoutUnit(),
+                                    children_count, /* bidi_level */ 0});
     }
 
     void MoveInInlineDirection(LayoutUnit);

third_party/blink/web_tests/FlagExpectations/layout-ng-fragment-item

@@ -78,7 +78,7 @@ crbug.com/982194 external/wpt/css/CSS2/floats/floats-rule3-outside-right-002.xht
 crbug.com/982194 external/wpt/css/CSS2/floats/floats-rule7-outside-left-001.xht [ Failure ]
 crbug.com/982194 external/wpt/css/CSS2/floats/floats-rule7-outside-right-001.xht [ Failure ]
 crbug.com/982194 external/wpt/css/CSS2/floats/hit-test-floats-001.html [ Failure ]
-crbug.com/982194 external/wpt/css/CSS2/positioning/abspos-float-with-inline-container.html [ Crash ]
+crbug.com/982194 external/wpt/css/CSS2/positioning/abspos-float-with-inline-container.html [ Crash Failure ]
 crbug.com/982194 external/wpt/css/CSS2/positioning/toogle-abspos-on-relpos-inline-child.html [ Failure ]
 crbug.com/982194 external/wpt/css/css-contain/contain-layout-017.html [ Pass ]
 crbug.com/982194 external/wpt/css/css-contain/contain-paint-021.html [ Pass ]