[LayoutNG] Fix creating inline box when it has only bidi controls

This patch fixes a failure to create inline box fragments
when they contain only bidi controls.

A pair of PrepareForReorder() and UpdateAfterReorder()
re-establishes relationship between inline boxes and its
children after bidi reorder. When inline boxes have only
bidi controls, the relationship was not re-established
correctly, resulting DCHECK failures and incorrect
positioning of such inline box fragments.

Found in r570132 (CL:1108509) thanks to xiaochengh@.

Bug: 636993
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I2fbbd60e230fea872af38d6c133bb26a50e67e4e
Reviewed-on: https://chromium-review.googlesource.com/1116415
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571057}

third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

@@ -23,11 +23,6 @@ crbug.com/851075 fast/writing-mode/vertical-inline-block-hittest.html [ Failure
 # Wrong image alignment in vertical-rl writing mode
 crbug.com/636993 fast/writing-mode/vertical-baseline-alignment.html [ Failure ]
 
-# Need bidi inline fragmentation
-crbug.com/636993 fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html [ Crash ]
-crbug.com/636993 fast/text/international/unicode-bidi-isolate-nested-with-removes.html [ Crash ]
-crbug.com/636993 fast/text/nested-bidi-with-continuation-crash.html [ Crash ]
-
 # Wrong quirks mode line height for pattern <div><a><img></a></div>
 crbug.com/854840 fast/table/backgr_border-table-quirks-collapsed-border.html [ Failure ]
 crbug.com/854840 fast/table/backgr_border-table-quirks.html [ Failure ]

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

@@ -66,7 +66,7 @@ void NGInlineBoxState::AccumulateUsedFonts(const ShapeResult* shape_result,
                                            FontBaseline baseline_type) {
   HashSet<const SimpleFontData*> fallback_fonts;
   shape_result->FallbackFonts(&fallback_fonts);
-  for (auto* const fallback_font : fallback_fonts) {
+  for (const SimpleFontData* const fallback_font : fallback_fonts) {
     NGLineHeightMetrics fallback_metrics(fallback_font->GetFontMetrics(),
                                          baseline_type);
     fallback_metrics.AddLeading(
@@ -90,7 +90,7 @@ LayoutObject*
 NGInlineLayoutStateStack::ContainingLayoutObjectForAbsolutePositionObjects()
     const {
   for (unsigned i = stack_.size(); i-- > 1;) {
-    const auto& box = stack_[i];
+    const NGInlineBoxState& box = stack_[i];
     DCHECK(box.style);
     if (box.style->CanContainAbsolutePositionObjects()) {
       DCHECK(box.item->GetLayoutObject());
@@ -111,7 +111,7 @@ NGInlineBoxState* NGInlineLayoutStateStack::OnBeginPlaceItems(
     box->fragment_start = 0;
   } else {
     // For the following lines, clear states that are not shared across lines.
-    for (auto& box : stack_) {
+    for (NGInlineBoxState& box : stack_) {
       box.fragment_start = 0;
       if (!line_height_quirk)
         box.metrics = box.text_metrics;
@@ -339,7 +339,7 @@ void NGInlineLayoutStateStack::PrepareForReorder(
     NGLineBoxFragmentBuilder::ChildList* line_box) {
   // Set indexes of BoxData to the children of the line box.
   unsigned box_data_index = 0;
-  for (const auto& box_data : box_data_list_) {
+  for (const BoxData& box_data : box_data_list_) {
     box_data_index++;
     for (unsigned i = box_data.fragment_start; i < box_data.fragment_end; i++) {
       NGLineBoxFragmentBuilder::Child& child = (*line_box)[i];
@@ -350,7 +350,7 @@ void NGInlineLayoutStateStack::PrepareForReorder(
 
   // When boxes are nested, placeholders have indexes to which box it should be
   // added. Copy them to BoxData.
-  for (auto& box_data : box_data_list_) {
+  for (BoxData& box_data : box_data_list_) {
     const NGLineBoxFragmentBuilder::Child& placeholder =
         (*line_box)[box_data.fragment_end];
     DCHECK(!placeholder.HasFragment());
@@ -362,11 +362,11 @@ void NGInlineLayoutStateStack::PrepareForReorder(
 void NGInlineLayoutStateStack::UpdateAfterReorder(
     NGLineBoxFragmentBuilder::ChildList* line_box) {
   // Compute start/end of boxes from the children of the line box.
-  for (auto& box_data : box_data_list_)
+  for (BoxData& box_data : box_data_list_)
     box_data.fragment_start = box_data.fragment_end = 0;
   for (unsigned i = 0; i < line_box->size(); i++) {
-    const auto& child = (*line_box)[i];
-    if (!child.HasFragment())
+    const NGLineBoxFragmentBuilder::Child& child = (*line_box)[i];
+    if (child.IsPlaceholder())
       continue;
     if (unsigned box_data_index = child.box_data_index) {
       BoxData& box_data = box_data_list_[box_data_index - 1];
@@ -377,7 +377,7 @@ void NGInlineLayoutStateStack::UpdateAfterReorder(
   }
 
   // Extend start/end of boxes when they are nested.
-  for (auto& box_data : box_data_list_) {
+  for (BoxData& box_data : box_data_list_) {
     if (box_data.box_data_index) {
       BoxData& parent_box_data = box_data_list_[box_data.box_data_index - 1];
       if (!parent_box_data.fragment_end) {
@@ -394,7 +394,7 @@ void NGInlineLayoutStateStack::UpdateAfterReorder(
 
 #if DCHECK_IS_ON()
   // Check all BoxData have ranges.
-  for (const auto& box_data : box_data_list_) {
+  for (const BoxData& box_data : box_data_list_) {
     DCHECK_NE(box_data.fragment_end, 0u);
     DCHECK_GT(box_data.fragment_end, box_data.fragment_start);
   }
@@ -406,7 +406,7 @@ LayoutUnit NGInlineLayoutStateStack::ComputeInlinePositions(
   // At this point, children are in the visual order, and they have their
   // origins at (0, 0). Accumulate inline offset from left to right.
   LayoutUnit position;
-  for (auto& child : *line_box) {
+  for (NGLineBoxFragmentBuilder::Child& child : *line_box) {
     child.offset.inline_offset += position;
     // Box margins/boders/paddings will be processed later.
     // TODO(kojii): we could optimize this if the reordering did not occur.
@@ -419,7 +419,7 @@ LayoutUnit NGInlineLayoutStateStack::ComputeInlinePositions(
     return position;
 
   // Adjust child offsets for margin/border/padding of inline boxes.
-  for (auto& box_data : box_data_list_) {
+  for (BoxData& box_data : box_data_list_) {
     unsigned start = box_data.fragment_start;
     unsigned end = box_data.fragment_end;
     DCHECK_GT(end, start);
@@ -446,7 +446,7 @@ LayoutUnit NGInlineLayoutStateStack::ComputeInlinePositions(
     LayoutUnit line_right;
   };
   Vector<LinePadding, 32> accumulated_padding(line_box->size());
-  for (auto& box_data : box_data_list_) {
+  for (BoxData& box_data : box_data_list_) {
     // Compute line-left and line-right edge of this box by accomodating
     // border/padding of this box and margin/border/padding of descendants
     // boxes, while accumulating its margin/border/padding.
@@ -477,7 +477,7 @@ void NGInlineLayoutStateStack::CreateBoxFragments(
     NGLineBoxFragmentBuilder::ChildList* line_box) {
   DCHECK(!box_data_list_.IsEmpty());
 
-  for (auto& box_data : box_data_list_) {
+  for (BoxData& box_data : box_data_list_) {
     unsigned start = box_data.fragment_start;
     unsigned end = box_data.fragment_end;
     DCHECK_GT(end, start);
@@ -545,7 +545,7 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
   // |pending_descendants|.
   LayoutUnit baseline_shift;
   if (!box->pending_descendants.IsEmpty()) {
-    for (auto& child : box->pending_descendants) {
+    for (NGPendingPositions& child : box->pending_descendants) {
       if (child.metrics.IsEmpty()) {
         // This can happen with boxes with no content in quirks mode
         child.metrics = NGLineHeightMetrics(LayoutUnit(), LayoutUnit());

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

@@ -188,7 +188,7 @@ void NGInlineLayoutAlgorithm::CreateLine(NGLineInfo* line_info,
   NGInlineBoxState* box =
       box_states_->OnBeginPlaceItems(&line_style, baseline_type_, quirks_mode_);
 
-  for (auto& item_result : *line_items) {
+  for (NGInlineItemResult& item_result : *line_items) {
     DCHECK(item_result.item);
     const NGInlineItem& item = *item_result.item;
     if (item.Type() == NGInlineItem::kText) {
@@ -442,7 +442,7 @@ bool NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
     const NGLineInfo& line_info,
     const NGLineHeightMetrics& line_box_metrics) {
   bool has_fragments = false;
-  for (auto& child : line_box_) {
+  for (NGLineBoxFragmentBuilder::Child& child : line_box_) {
     if (LayoutObject* box = child.out_of_flow_positioned_box) {
       // The static position is at the line-top. Ignore the block_offset.
       NGLogicalOffset static_offset(child.offset.inline_offset, LayoutUnit());
@@ -800,17 +800,18 @@ unsigned NGInlineLayoutAlgorithm::PositionLeadingItems(
 
   unsigned index = BreakToken() ? BreakToken()->ItemIndex() : 0;
   for (; index < items.size(); ++index) {
-    const auto& item = items[index];
+    const NGInlineItem& item = items[index];
 
     if (item.Type() == NGInlineItem::kFloating) {
       NGBlockNode node(ToLayoutBox(item.GetLayoutObject()));
       NGBoxStrut margins =
           ComputeMarginsForContainer(ConstraintSpace(), node.Style());
 
-      auto unpositioned_float = NGUnpositionedFloat::Create(
-          ConstraintSpace().AvailableSize(),
-          ConstraintSpace().PercentageResolutionSize(), bfc_line_offset,
-          bfc_line_offset, margins, node, /* break_token */ nullptr);
+      scoped_refptr<NGUnpositionedFloat> unpositioned_float =
+          NGUnpositionedFloat::Create(
+              ConstraintSpace().AvailableSize(),
+              ConstraintSpace().PercentageResolutionSize(), bfc_line_offset,
+              bfc_line_offset, margins, node, /* break_token */ nullptr);
       AddUnpositionedFloat(&unpositioned_floats_, &container_builder_,
                            std::move(unpositioned_float));
     } else if (is_empty_inline &&
@@ -851,7 +852,7 @@ void NGInlineLayoutAlgorithm::PositionPendingFloats(
   LayoutUnit origin_block_offset = bfc_offset.block_offset + content_size;
   LayoutUnit from_block_offset = bfc_offset.block_offset;
 
-  const auto positioned_floats =
+  const Vector<NGPositionedFloat> positioned_floats =
       PositionFloats(origin_block_offset, from_block_offset,
                      unpositioned_floats_, ConstraintSpace(), exclusion_space);
 
@@ -873,8 +874,8 @@ void NGInlineLayoutAlgorithm::BidiReorder() {
   Vector<UBiDiLevel, 32> levels;
   logical_items.ReserveInitialCapacity(line_box_.size());
   levels.ReserveInitialCapacity(line_box_.size());
-  for (auto& item : line_box_) {
-    if (!item.HasFragment() && !item.HasBidiLevel())
+  for (NGLineBoxFragmentBuilder::Child& item : line_box_) {
+    if (item.IsPlaceholder())
       continue;
     levels.push_back(item.bidi_level);
     logical_items.AddChild(std::move(item));

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

@@ -22,6 +22,7 @@ struct NGPositionedFloat;
 class CORE_EXPORT NGLineBoxFragmentBuilder final
     : public NGContainerFragmentBuilder {
   STACK_ALLOCATED();
+
  public:
   NGLineBoxFragmentBuilder(NGInlineNode,
                            scoped_refptr<const ComputedStyle>,
@@ -106,6 +107,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
       return HasInFlowFragment() || HasOutOfFlowFragment();
     }
     bool HasBidiLevel() const { return bidi_level != 0xff; }
+    bool IsPlaceholder() const { return !HasFragment() && !HasBidiLevel(); }
     const NGPhysicalFragment* PhysicalFragment() const;
   };