[LayoutNG] Fix newlines with bidi contexts in relayout (for real this time)

When we encounter a preserved line break in relayout, we can't reuse its
items if:
- It was in a bidi context in a previous layout
- It's in a bidi context in the current (re)layout

We attempted to ensure that in crrev.com/c/1626498, but failed because
a <span dir=ltr> creates bidi contexts without necessarily setting the
NGInlineNodeData::is_bidi_enabled_ flag.

This patch fixes the issue by adding a flag to LayoutText to indicate if
the text has any bidi control items, so that when it's true and the text
contains newlines, we don't reuse its old inline items.

Bug: 966751
Change-Id: I63d95c9d1ddcffda62a5227d0d6166b2e9ce221a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649218
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667289}

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

@@ -128,6 +128,7 @@ LayoutText::LayoutText(Node* node, scoped_refptr<StringImpl> str)
       has_tab_(false),
       lines_dirty_(false),
       valid_ng_items_(false),
+      has_bidi_control_items_(false),
       contains_reversed_text_(false),
       known_to_have_no_overflow_and_no_fallback_fonts_(false),
       contains_only_whitespace_or_nbsp_(
@@ -2413,6 +2414,7 @@ void LayoutText::SetInlineItems(NGInlineItem* begin, NGInlineItem* end) {
 }
 
 void LayoutText::ClearInlineItems() {
+  has_bidi_control_items_ = false;
   valid_ng_items_ = false;
   if (base::span<NGInlineItem>* items = GetNGInlineItems())
     *items = base::span<NGInlineItem>();

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

@@ -320,6 +320,10 @@ class CORE_EXPORT LayoutText : public LayoutObject {
   // it was inserted/changed but also it was moved.
   void InvalidateInlineItems() { valid_ng_items_ = false; }
 
+  bool HasBidiControlInlineItems() const { return has_bidi_control_items_; }
+  void SetHasBidiControlInlineItems() { has_bidi_control_items_ = true; }
+  void ClearHasBidiControlInlineItems() { has_bidi_control_items_ = false; }
+
  protected:
   virtual const base::span<NGInlineItem>* GetNGInlineItems() const {
     return nullptr;
@@ -421,6 +425,10 @@ class CORE_EXPORT LayoutText : public LayoutObject {
   // Functionally the inverse equivalent of lines_dirty_ for LayoutNG.
   unsigned valid_ng_items_ : 1;
 
+  // Used by LayoutNGText. Whether there is any BidiControl type NGInlineItem
+  // associated with this object. Set after layout when associating items.
+  unsigned has_bidi_control_items_ : 1;
+
   unsigned contains_reversed_text_ : 1;
   mutable unsigned known_to_have_no_overflow_and_no_fallback_fonts_ : 1;
   unsigned contains_only_whitespace_or_nbsp_ : 2;

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

@@ -362,7 +362,7 @@ bool NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendTextReusing(
     // We exit and then re-enter all bidi contexts around a forced break. So, We
     // must go through the full pipeline to ensure that we exit and enter the
     // correct bidi contexts the re-layout.
-    if (bidi_context_.size() || original_data.IsBidiEnabled()) {
+    if (bidi_context_.size() || layout_text->HasBidiControlInlineItems()) {
       if (layout_text->GetText().Contains(kNewlineCharacter))
         return false;
     }

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

@@ -861,12 +861,18 @@ void NGInlineNode::AssociateItemsWithInlines(NGInlineNodeData* data) {
       // Items split from a LayoutObject should be consecutive.
       DCHECK(associated_objects.insert(object).is_new_entry);
 #endif
+      layout_text->ClearHasBidiControlInlineItems();
+      bool has_bidi_control = false;
       NGInlineItem* begin = item;
       for (++item; item != items.end(); ++item) {
         if (item->GetLayoutObject() != object)
           break;
+        if (item->Type() == NGInlineItem::kBidiControl)
+          has_bidi_control = true;
       }
       layout_text->SetInlineItems(begin, item);
+      if (has_bidi_control)
+        layout_text->SetHasBidiControlInlineItems();
       continue;
     }
     ++item;

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

@@ -1431,6 +1431,18 @@ TEST_F(NGInlineNodeTest, PreservedNewlineWithRemovedBidiAndRelayout) {
   EXPECT_EQ("foo\nbar", GetText());
 }
 
+TEST_F(NGInlineNodeTest, PreservedNewlineWithRemovedLtrDirAndRelayout) {
+  SetupHtml("container",
+            "<pre id=container>foo<span dir=ltr>\nbar</span></pre>");
+  EXPECT_EQ(String(u"foo\u2066\u2069\n\u2066bar\u2069"), GetText());
+
+  GetDocument().QuerySelector("span")->removeAttribute(html_names::kDirAttr);
+  UpdateAllLifecyclePhasesForTest();
+
+  // The bidi control characters around '\n' should not preserve
+  EXPECT_EQ("foo\nbar", GetText());
+}
+
 // https://crbug.com/969089
 TEST_F(NGInlineNodeTest, InsertedWBRWithLineBreakInRelayout) {
   SetupHtml("container", "<div id=container><span>foo</span>\nbar</div>");