Make NGInlineNode::SetTextWithOffset() to handle bidi text correctly

This patch intorudces |NGInlineItemsBuilder::DidFinishCollectInlines()| to
populate collected inline data to |NGInlineNodeData| at one place to ensure
valid |NGInlineNodeData|, e.g. valid value of |is_bidi_enabled_|.

Background:
Before this patch, we processed RTL text as LTR text when text is modified
with |LayoutText::SetTextWithOffset()| by Text.{append,replace}Data() and
chunked append at loading.

Bug: 1038254
Change-Id: I645f78f853e89704120d50b9db3234806f65c1ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010590
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733296}

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

@@ -47,6 +47,20 @@ class LayoutNGTextTest : public PageTestBase {
   }
 };
 
+TEST_F(LayoutNGTextTest, SetTextWithOffsetAppendBidi) {
+  if (!RuntimeEnabledFeatures::LayoutNGEnabled())
+    return;
+
+  SetBodyInnerHTML(u"<div dir=rtl id=target>\u05D0\u05D1\u05BC\u05D2</div>");
+  Text& text = To<Text>(*GetElementById("target")->firstChild());
+  text.appendData(u"\u05D0\u05D1\u05BC\u05D2");
+
+  EXPECT_EQ(String(u"*{'\u05D0\u05D1\u05BC\u05D2\u05D0\u05D1\u05BC\u05D2', "
+                   u"ShapeResult=0+8}\n")
+                .Utf8(),
+            GetItemsAsString(*text.GetLayoutObject()));
+}
+
 TEST_F(LayoutNGTextTest, SetTextWithOffsetAppendControl) {
   if (!RuntimeEnabledFeatures::LayoutNGEnabled())
     return;
@@ -136,8 +150,11 @@ TEST_F(LayoutNGTextTest, SetTextWithOffsetDeleteRTL) {
   Text& text = To<Text>(*GetElementById("target")->firstChild());
   text.deleteData(2, 2, ASSERT_NO_EXCEPTION);  // remove "23"
 
-  EXPECT_EQ("*{'0 4', ShapeResult=0+3}\n",
-            GetItemsAsString(*text.GetLayoutObject()));
+  EXPECT_EQ(
+      "*{'0', ShapeResult=0+1}\n"
+      "*{' ', ShapeResult=1+1}\n"
+      "*{'4', ShapeResult=2+1}\n",
+      GetItemsAsString(*text.GetLayoutObject()));
 }
 
 // http://crbug.com/1000685
@@ -149,8 +166,12 @@ TEST_F(LayoutNGTextTest, SetTextWithOffsetDeleteRTL2) {
   Text& text = To<Text>(*GetElementById("target")->firstChild());
   text.deleteData(0, 1, ASSERT_NO_EXCEPTION);  // remove "0"
 
-  EXPECT_EQ("*{'(xy)5', ShapeResult=0+5}\n",
-            GetItemsAsString(*text.GetLayoutObject()));
+  EXPECT_EQ(
+      "*{'(', ShapeResult=0+1}\n"
+      "*{'xy', ShapeResult=1+2}\n"
+      "*{')', ShapeResult=3+1}\n"
+      "*{'5', ShapeResult=4+1}\n",
+      GetItemsAsString(*text.GetLayoutObject()));
 }
 
 // http://crbug.com/1039143

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

@@ -1230,6 +1230,27 @@ void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::Exit(
   }
 }
 
+template <typename OffsetMappingBuilder>
+bool NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::MayBeBidiEnabled()
+    const {
+  return !text_.Is8Bit() || HasBidiControls();
+}
+
+template <typename OffsetMappingBuilder>
+void NGInlineItemsBuilderTemplate<
+    OffsetMappingBuilder>::DidFinishCollectInlines(NGInlineNodeData* data) {
+  data->text_content = ToString();
+
+  // Set |is_bidi_enabled_| for all UTF-16 strings for now, because at this
+  // point the string may or may not contain RTL characters.
+  // |SegmentText()| will analyze the text and reset |is_bidi_enabled_| if it
+  // doesn't contain any RTL characters.
+  data->is_bidi_enabled_ = MayBeBidiEnabled();
+  data->is_empty_inline_ = IsEmptyInline();
+  data->is_block_level_ = IsBlockLevel();
+  data->changes_may_affect_earlier_lines_ = ChangesMayAffectEarlierLines();
+}
+
 template <typename OffsetMappingBuilder>
 void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::SetIsSymbolMarker(
     bool b) {

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

@@ -134,6 +134,9 @@ class NGInlineItemsBuilderTemplate {
   void EnterInline(LayoutInline*);
   void ExitInline(LayoutObject*);
 
+  // Set collected inline items data to |data|.
+  void DidFinishCollectInlines(NGInlineNodeData* data);
+
   OffsetMappingBuilder& GetOffsetMappingBuilder() { return mapping_builder_; }
 
   void SetIsSymbolMarker(bool b);
@@ -223,6 +226,8 @@ class NGInlineItemsBuilderTemplate {
 
   void Exit(LayoutObject*);
 
+  bool MayBeBidiEnabled() const;
+
   bool ShouldInsertBreakOpportunityAfterLeadingPreservedSpaces(
       const String&,
       const ComputedStyle&,

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

@@ -427,13 +427,6 @@ void TruncateOrPadText(String* text, unsigned length) {
   }
 }
 
-template <typename OffsetMappingBuilder>
-bool MayBeBidiEnabled(
-    const String& text_content,
-    const NGInlineItemsBuilderTemplate<OffsetMappingBuilder>& builder) {
-  return !text_content.Is8Bit() || builder.HasBidiControls();
-}
-
 }  // namespace
 
 NGInlineNode::NGInlineNode(LayoutBlockFlow* block)
@@ -778,7 +771,7 @@ bool NGInlineNode::SetTextWithOffset(LayoutText* layout_text,
   // inline items.
   layout_text->ClearInlineItems();
   CollectInlinesInternal(node.GetLayoutBlockFlow(), &builder, previous_data);
-  data->text_content = builder.ToString();
+  builder.DidFinishCollectInlines(data);
   // Relocates |ShapeResult| in |previous_data| after |offset|+|length|
   editor.Run();
   node.SegmentText(data);
@@ -886,17 +879,7 @@ void NGInlineNode::CollectInlines(NGInlineNodeData* data,
   data->items.ReserveCapacity(EstimateInlineItemsCount(*block));
   NGInlineItemsBuilder builder(&data->items, dirty_lines);
   CollectInlinesInternal(block, &builder, previous_data);
-  data->text_content = builder.ToString();
-
-  // Set |is_bidi_enabled_| for all UTF-16 strings for now, because at this
-  // point the string may or may not contain RTL characters.
-  // |SegmentText()| will analyze the text and reset |is_bidi_enabled_| if it
-  // doesn't contain any RTL characters.
-  data->is_bidi_enabled_ = MayBeBidiEnabled(data->text_content, builder);
-  data->is_empty_inline_ = builder.IsEmptyInline();
-  data->is_block_level_ = builder.IsBlockLevel();
-  data->changes_may_affect_earlier_lines_ =
-      builder.ChangesMayAffectEarlierLines();
+  builder.DidFinishCollectInlines(data);
 }
 
 void NGInlineNode::SegmentText(NGInlineNodeData* data) {

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

@@ -11,6 +11,9 @@
 
 namespace blink {
 
+template <typename OffsetMappingBuilder>
+class NGInlineItemsBuilderTemplate;
+
 // Data which is required for inline nodes.
 struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData {
  public:
@@ -40,6 +43,9 @@ struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData {
   friend class NGInlineNodeForTest;
   friend class NGOffsetMappingTest;
 
+  template <typename OffsetMappingBuilder>
+  friend class NGInlineItemsBuilderTemplate;
+
   // Items to use for the first line, when the node has :first-line rules.
   //
   // Items have different ComputedStyle, and may also have different