[LayoutNG] Fix bidi reordering of lines with empty inline boxes

This patch fixes bidi reordering sometimes confused by empty
inline boxes.

These items do not have resolved bidi levels. Before this
patch, |NGInlineLayoutAlgorithm| reordered ignoring such
items. This patch changes it to assign bidi levels to such
items by copying from adjacent items and let reordering to
handle them.

Bug: 1010662, 1010641, 998872
Change-Id: I5d074143970b83b64bddbb83cd03b17551e3f521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837435Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702526}

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

@@ -404,8 +404,6 @@ unsigned NGInlineLayoutStateStack::UpdateBoxDataFragmentRange(
   // Find the first line box item that should create a box fragment.
   for (; index < line_box->size(); index++) {
     NGLineBoxFragmentBuilder::Child* start = &(*line_box)[index];
-    if (start->IsPlaceholder())
-      continue;
     const unsigned box_data_index = start->box_data_index;
     if (!box_data_index)
       continue;
@@ -423,8 +421,6 @@ unsigned NGInlineLayoutStateStack::UpdateBoxDataFragmentRange(
     const unsigned start_index = index;
     for (index++; index < line_box->size(); index++) {
       NGLineBoxFragmentBuilder::Child* end = &(*line_box)[index];
-      if (end->IsPlaceholder())
-        continue;
 
       // If we found another box that maybe included in this box, update it
       // first. Updating will change |end->box_data_index| so that we can

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

@@ -295,7 +295,7 @@ void NGInlineLayoutAlgorithm::CreateLine(
 
   if (UNLIKELY(Node().IsBidiEnabled())) {
     box_states_->PrepareForReorder(&line_box_);
-    BidiReorder();
+    BidiReorder(line_info->BaseDirection());
     box_states_->UpdateAfterReorder(&line_box_);
   }
   LayoutUnit inline_size = box_states_->ComputeInlinePositions(&line_box_);
@@ -1058,37 +1058,75 @@ NGPositionedFloat NGInlineLayoutAlgorithm::PositionFloat(
       &unpositioned_float, ConstraintSpace(), Style(), exclusion_space);
 }
 
-void NGInlineLayoutAlgorithm::BidiReorder() {
+void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction) {
+  if (line_box_.IsEmpty())
+    return;
+
   // TODO(kojii): UAX#9 L1 is not supported yet. Supporting L1 may change
   // embedding levels of parts of runs, which requires to split items.
   // http://unicode.org/reports/tr9/#L1
   // BidiResolver does not support L1 crbug.com/316409.
 
+  // A sentinel value for items that are opaque to bidi reordering. Should be
+  // larger than the maximum resolved level.
+  constexpr UBiDiLevel kOpaqueBidiLevel = 0xff;
+  DCHECK_GT(kOpaqueBidiLevel, UBIDI_MAX_EXPLICIT_LEVEL + 1);
+
   // Create a list of chunk indices in the visual order.
   // ICU |ubidi_getVisualMap()| works for a run of characters. Since we can
   // handle the direction of each run, we use |ubidi_reorderVisual()| to reorder
   // runs instead of characters.
-  NGLineBoxFragmentBuilder::ChildList logical_items;
   Vector<UBiDiLevel, 32> levels;
-  logical_items.ReserveInitialCapacity(line_box_.size());
   levels.ReserveInitialCapacity(line_box_.size());
+  bool has_opaque_items = false;
   for (NGLineBoxFragmentBuilder::Child& item : line_box_) {
-    if (item.IsPlaceholder())
+    if (item.IsOpaqueToBidiReordering()) {
+      levels.push_back(kOpaqueBidiLevel);
+      has_opaque_items = true;
       continue;
+    }
+    DCHECK_NE(item.bidi_level, kOpaqueBidiLevel);
     levels.push_back(item.bidi_level);
-    logical_items.AddChild(std::move(item));
-    DCHECK(!item.HasInFlowFragment());
   }
 
+  // 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) {
+      if (level == kOpaqueBidiLevel)
+        level = last_level;
+      else
+        last_level = level;
+    }
+  }
+
+  // Compute visual indices from resolved levels.
   Vector<int32_t, 32> indices_in_visual_order(levels.size());
   NGBidiParagraph::IndicesInVisualOrder(levels, &indices_in_visual_order);
 
   // Reorder to the visual order.
-  line_box_.resize(0);
+  NGLineBoxFragmentBuilder::ChildList visual_items;
+  visual_items.ReserveInitialCapacity(line_box_.size());
   for (unsigned logical_index : indices_in_visual_order) {
-    line_box_.AddChild(std::move(logical_items[logical_index]));
-    DCHECK(!logical_items[logical_index].HasInFlowFragment());
+    visual_items.AddChild(std::move(line_box_[logical_index]));
+    DCHECK(!line_box_[logical_index].HasInFlowFragment());
   }
+  DCHECK_EQ(line_box_.size(), visual_items.size());
+  line_box_ = std::move(visual_items);
 }
 
 }  // namespace blink

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

@@ -74,7 +74,7 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
                                    const NGInlineItemResult&,
                                    NGInlineBoxState*);
 
-  void BidiReorder();
+  void BidiReorder(TextDirection base_direction);
 
   void PlaceControlItem(const NGInlineItem&,
                         const NGLineInfo&,

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

@@ -166,6 +166,20 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
     }
     bool HasBidiLevel() const { return bidi_level != 0xff; }
     bool IsPlaceholder() const { return !HasFragment() && !HasBidiLevel(); }
+    bool IsOpaqueToBidiReordering() const {
+      if (IsPlaceholder())
+        return true;
+      // Skip all inline boxes. Fragments for inline boxes maybe created earlier
+      // if they have no children.
+      if (layout_result) {
+        const LayoutObject* layout_object =
+            layout_result->PhysicalFragment().GetLayoutObject();
+        DCHECK(layout_object);
+        if (layout_object->IsLayoutInline())
+          return true;
+      }
+      return false;
+    }
     const NGPhysicalFragment* PhysicalFragment() const {
       if (layout_result)
         return &layout_result->PhysicalFragment();

third_party/blink/web_tests/TestExpectations

@@ -708,7 +708,6 @@ crbug.com/591099 fast/events/touch/compositor-touch-hit-rects-list-translate.htm
 crbug.com/591099 fast/events/touch/compositor-touch-hit-rects.html [ Failure ]
 crbug.com/889721 fast/inline/outline-continuations.html [ Failure ]
 crbug.com/591099 fast/text/font-format-support-color-cff2-vertical.html [ Failure ]
-crbug.com/998872 fast/text/international/bidi-ignored-for-first-child-inline.html [ Failure ]
 crbug.com/591099 fast/text/whitespace/018.html [ Failure ]
 crbug.com/591099 fast/writing-mode/auto-sizing-orthogonal-flows.html [ Failure ]
 crbug.com/591099 fast/writing-mode/percentage-height-orthogonal-writing-modes.html [ Failure ]

third_party/blink/web_tests/external/wpt/css/CSS2/text/bidi-span-001-ref.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<body>
+  <div>אבג</div>
+  <div>אבג</div>
+  <div>אבג</div>
+  <div>אבג</div>
+  <div>אבג</div>
+</body>

third_party/blink/web_tests/external/wpt/css/CSS2/text/bidi-span-001.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Test: Inline boxes should not affect bidi reordering</title>
+<link rel="match" href="bidi-span-001-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#direction">
+<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
+<body>
+  <div>א<span></span>בג</div>
+  <div>א<span style="background: white"></span>בג</div>
+  <div>א<span>ב</span>ג</div>
+  <div>א<span style="background: white">ב</span>ג</div>
+  <div>אבג</div>
+</body>

third_party/blink/web_tests/external/wpt/css/CSS2/text/bidi-span-002-ref.html

+<!DOCTYPE html>
+<body>()</body>

third_party/blink/web_tests/external/wpt/css/CSS2/text/bidi-span-002.html

+<!DOCTYPE html>
+<title>CSS Test: Inline boxes should not affect bidi reordering</title>
+<link rel="match" href="bidi-span-002-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#direction">
+<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
+<style>
+body {
+  text-align: left;
+  direction: rtl;
+}
+
+.c1:before {
+  content: '(';
+}
+
+.c1:after {
+  content: ')';
+}
+
+.c2:after {
+  content: '';
+}
+</style>
+<body><span class="c1"><span class="c2"></span></span></body>