[LayoutNG] Remove trailing spaces of blocks in phase 1

This patch changes NGInlineItemsBuilder to remove trailing
spaces at the end of blocks.

It used to do so, but stopped in https://crrev.com/c/1022191
because doing so reduces opportunities to re-use NGInlineItem
and increases shaping when contents are added.

However, for fonts that has kerning before a space character,
this change increased re-shaping every block end. Given line
breaking is more often ran than adding contents dynamically,
this turned out to be not a good trade off. We could re-shape
only end-part in RestoreTrailingCollapsibleSpace to achieve
the originally intended optimizations.

This speeds up blink_perf tests on Windows by ~10%.

This patch also fixes a few cases where space collapsing is
incorrect when items are re-used.

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

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/20110323/list-style-position-005-expected.txt

@@ -13,9 +13,9 @@ layer at (0,0) size 800x262
           text run at (124,0) width 274: " be a green bar stacked on top of a blue bar."
       LayoutNGListItem {DIV} at (0,36) size 784x202 [color=#00FF00] [bgcolor=#00FF00]
         LayoutNGBlockFlow (anonymous) at (0,0) size 784x101
-          LayoutInline (anonymous) at (0,0) size 30x97
-            LayoutText (anonymous) at (-1,2) size 30x97
-              text run at (-1,2) width 30: "\x{2022}"
+          LayoutInline (anonymous) at (0,0) size 27x97
+            LayoutText (anonymous) at (-1,2) size 27x97
+              text run at (-1,2) width 27: "\x{2022}"
         LayoutNGBlockFlow {DIV} at (0,101) size 784x101 [bgcolor=#0000FF]
           LayoutText {#text} at (0,2) size 21x97
             text run at (0,2) width 21: " "

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/t1202-counter-13-b-expected.txt

@@ -73,10 +73,10 @@ layer at (0,0) size 800x100
               text run at (139,0) width 12: "K"
         LayoutText {#text} at (151,0) size 4x19
           text run at (151,0) width 4: " "
-        LayoutInline {SPAN} at (0,0) size 9x19
-          LayoutInline {<pseudo:before>} at (0,0) size 9x19
-            LayoutCounter (anonymous) at (155,0) size 9x19
-              text run at (155,0) width 9: "L"
+        LayoutInline {SPAN} at (0,0) size 10x19
+          LayoutInline {<pseudo:before>} at (0,0) size 10x19
+            LayoutCounter (anonymous) at (155,0) size 10x19
+              text run at (155,0) width 10: "L"
         LayoutText {#text} at (0,0) size 0x0
       LayoutNGBlockFlow {DIV} at (0,56) size 784x20
         LayoutText {#text} at (0,0) size 165x19

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/t1202-counter-14-b-expected.txt

@@ -73,10 +73,10 @@ layer at (0,0) size 800x100
               text run at (139,0) width 12: "K"
         LayoutText {#text} at (151,0) size 4x19
           text run at (151,0) width 4: " "
-        LayoutInline {SPAN} at (0,0) size 9x19
-          LayoutInline {<pseudo:before>} at (0,0) size 9x19
-            LayoutCounter (anonymous) at (155,0) size 9x19
-              text run at (155,0) width 9: "L"
+        LayoutInline {SPAN} at (0,0) size 10x19
+          LayoutInline {<pseudo:before>} at (0,0) size 10x19
+            LayoutCounter (anonymous) at (155,0) size 10x19
+              text run at (155,0) width 10: "L"
         LayoutText {#text} at (0,0) size 0x0
       LayoutNGBlockFlow {DIV} at (0,56) size 784x20
         LayoutText {#text} at (0,0) size 165x19

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/t1202-counters-13-b-expected.txt

@@ -73,10 +73,10 @@ layer at (0,0) size 800x100
               text run at (289,0) width 28: "A.K"
         LayoutText {#text} at (317,0) size 3x19
           text run at (317,0) width 3: " "
-        LayoutInline {SPAN} at (0,0) size 25x19
-          LayoutInline {<pseudo:before>} at (0,0) size 25x19
-            LayoutCounter (anonymous) at (320,0) size 25x19
-              text run at (320,0) width 25: "A.L"
+        LayoutInline {SPAN} at (0,0) size 26x19
+          LayoutInline {<pseudo:before>} at (0,0) size 26x19
+            LayoutCounter (anonymous) at (320,0) size 26x19
+              text run at (320,0) width 26: "A.L"
         LayoutText {#text} at (0,0) size 0x0
       LayoutNGBlockFlow {DIV} at (0,56) size 784x20
         LayoutText {#text} at (0,0) size 346x19

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/t1202-counters-14-b-expected.txt

@@ -73,10 +73,10 @@ layer at (0,0) size 800x100
               text run at (289,0) width 28: "A.K"
         LayoutText {#text} at (317,0) size 3x19
           text run at (317,0) width 3: " "
-        LayoutInline {SPAN} at (0,0) size 25x19
-          LayoutInline {<pseudo:before>} at (0,0) size 25x19
-            LayoutCounter (anonymous) at (320,0) size 25x19
-              text run at (320,0) width 25: "A.L"
+        LayoutInline {SPAN} at (0,0) size 26x19
+          LayoutInline {<pseudo:before>} at (0,0) size 26x19
+            LayoutCounter (anonymous) at (320,0) size 26x19
+              text run at (320,0) width 26: "A.L"
         LayoutText {#text} at (0,0) size 0x0
       LayoutNGBlockFlow {DIV} at (0,56) size 784x20
         LayoutText {#text} at (0,0) size 346x19

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/events/mouseclick-target-and-positioning-expected.txt

 This page tests whether a click event propagates with the correct target and positioning. See rdar://problem/4477126.
 
 click inside the red box:[]
+PASS: event target should be [object HTMLSpanElement] and is
+PASS: event.pageX should be 175 and is
+PASS: event.pageY should be 105 and is
+PASS: event.clientX should be 175 and is
+PASS: event.clientY should be 105 and is
+PASS: event.layerX should be 7 and is
+PASS: event.layerY should be 5 and is
+PASS: event.offsetX should be 7 and is
+PASS: event.offsetY should be 5 and is
+PASS: event.x should be 175 and is
+PASS: event.y should be 105 and is

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/images/imagemap-focus-ring-in-positioned-container-expected.txt

+layer at (0,0) size 800x600
+  LayoutView at (0,0) size 800x600
+layer at (0,0) size 800x36
+  LayoutNGBlockFlow {HTML} at (0,0) size 800x36
+    LayoutNGBlockFlow {BODY} at (8,8) size 784x20
+      LayoutText {#text} at (0,0) size 551x19
+        text run at (0,0) width 551: "Tests that we paint area outline properly when the image is inside positioned containers."
+layer at (20,50) size 0x0
+  LayoutNGBlockFlow (positioned) {DIV} at (20,50) size 0x0
+layer at (30,60) size 50x55
+  LayoutNGBlockFlow (positioned) {DIV} at (10,10) size 50x55
+    LayoutImage {IMG} at (0,0) size 50x50
+    LayoutText {#text} at (0,0) size 0x0
+    LayoutInline {MAP} at (0,0) size 0x19
+      LayoutInline {AREA} at (0,0) size 0x19
+    LayoutText {#text} at (0,0) size 0x0

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/images/imagemap-focus-ring-with-paint-root-offset-expected.txt

+layer at (0,0) size 800x600
+  LayoutView at (0,0) size 800x600
+layer at (0,0) size 800x36
+  LayoutNGBlockFlow {HTML} at (0,0) size 800x36
+    LayoutNGBlockFlow {BODY} at (8,8) size 784x20
+      LayoutText {#text} at (0,0) size 437x19
+        text run at (0,0) width 437: "Tests that we paint area outline properly when the paintroot is shifted."
+layer at (5,50) size 50x55
+  LayoutNGBlockFlow (positioned) {DIV} at (5,50) size 50x55
+    LayoutImage {IMG} at (0,0) size 50x50
+    LayoutText {#text} at (0,0) size 0x0
+    LayoutInline {MAP} at (0,0) size 0x19
+      LayoutInline {AREA} at (0,0) size 0x19
+    LayoutText {#text} at (0,0) size 0x0

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/virtual/gpu-rasterization/images/imagemap-focus-ring-in-positioned-container-expected.txt

+layer at (0,0) size 800x600
+  LayoutView at (0,0) size 800x600
+layer at (0,0) size 800x36
+  LayoutNGBlockFlow {HTML} at (0,0) size 800x36
+    LayoutNGBlockFlow {BODY} at (8,8) size 784x20
+      LayoutText {#text} at (0,0) size 551x19
+        text run at (0,0) width 551: "Tests that we paint area outline properly when the image is inside positioned containers."
+layer at (20,50) size 0x0
+  LayoutNGBlockFlow (positioned) {DIV} at (20,50) size 0x0
+layer at (30,60) size 50x55
+  LayoutNGBlockFlow (positioned) {DIV} at (10,10) size 50x55
+    LayoutImage {IMG} at (0,0) size 50x50
+    LayoutText {#text} at (0,0) size 0x0
+    LayoutInline {MAP} at (0,0) size 0x19
+      LayoutInline {AREA} at (0,0) size 0x19
+    LayoutText {#text} at (0,0) size 0x0

third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/virtual/gpu-rasterization/images/imagemap-focus-ring-with-paint-root-offset-expected.txt

+layer at (0,0) size 800x600
+  LayoutView at (0,0) size 800x600
+layer at (0,0) size 800x36
+  LayoutNGBlockFlow {HTML} at (0,0) size 800x36
+    LayoutNGBlockFlow {BODY} at (8,8) size 784x20
+      LayoutText {#text} at (0,0) size 437x19
+        text run at (0,0) width 437: "Tests that we paint area outline properly when the paintroot is shifted."
+layer at (5,50) size 50x55
+  LayoutNGBlockFlow (positioned) {DIV} at (5,50) size 50x55
+    LayoutImage {IMG} at (0,0) size 50x50
+    LayoutText {#text} at (0,0) size 0x0
+    LayoutInline {MAP} at (0,0) size 0x19
+      LayoutInline {AREA} at (0,0) size 0x19
+    LayoutText {#text} at (0,0) size 0x0

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

@@ -56,8 +56,7 @@ NGInlineItem::NGInlineItem(NGInlineItemType type,
                            unsigned start,
                            unsigned end,
                            const ComputedStyle* style,
-                           LayoutObject* layout_object,
-                           bool end_may_collapse)
+                           LayoutObject* layout_object)
     : start_offset_(start),
       end_offset_(end),
       style_(style),
@@ -72,7 +71,7 @@ NGInlineItem::NGInlineItem(NGInlineItemType type,
       should_create_box_fragment_(false),
       style_variant_(static_cast<unsigned>(NGStyleVariant::kStandard)),
       end_collapse_type_(kNotCollapsible),
-      end_may_collapse_(end_may_collapse),
+      is_end_collapsible_newline_(false),
       is_symbol_marker_(false) {
   DCHECK_GE(end, start);
   ComputeBoxProperties();
@@ -97,7 +96,7 @@ NGInlineItem::NGInlineItem(const NGInlineItem& other,
       should_create_box_fragment_(other.should_create_box_fragment_),
       style_variant_(other.style_variant_),
       end_collapse_type_(other.end_collapse_type_),
-      end_may_collapse_(other.end_may_collapse_),
+      is_end_collapsible_newline_(other.is_end_collapsible_newline_),
       is_symbol_marker_(other.is_symbol_marker_) {
   DCHECK_GE(end, start);
 }
@@ -329,4 +328,15 @@ bool NGInlineItem::HasEndEdge() const {
          !ToLayoutInline(GetLayoutObject())->Continuation();
 }
 
+void NGInlineItem::SetEndCollapseType(NGCollapseType type) {
+  DCHECK(Type() == NGInlineItem::kText || type == kOpaqueToCollapsing ||
+         (Type() == NGInlineItem::kControl && type == kCollapsible));
+  end_collapse_type_ = type;
+}
+
+void NGInlineItem::SetEndCollapseType(NGCollapseType type, bool is_newline) {
+  SetEndCollapseType(type);
+  is_end_collapsible_newline_ = is_newline;
+}
+
 }  // namespace blink

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

@@ -51,12 +51,12 @@ class CORE_EXPORT NGInlineItem {
   enum NGCollapseType {
     // No collapsible spaces.
     kNotCollapsible,
-    // A collapsible space run that does not contain segment breaks.
-    kCollapsibleSpace,
-    // A collapsible space run that contains segment breaks.
-    kCollapsibleNewline,
     // This item is opaque to whitespace collapsing.
-    kOpaqueToCollapsing
+    kOpaqueToCollapsing,
+    // This item ends with collapsible spaces.
+    kCollapsible,
+    // Collapsible spaces at the end of this item were collapsed.
+    kCollapsed,
   };
 
   // The constructor and destructor can't be implicit or inlined, because they
@@ -65,8 +65,7 @@ class CORE_EXPORT NGInlineItem {
                unsigned start,
                unsigned end,
                const ComputedStyle* style = nullptr,
-               LayoutObject* layout_object = nullptr,
-               bool end_may_collapse = false);
+               LayoutObject* layout_object = nullptr);
   ~NGInlineItem();
 
   // Copy constructor adjusting start/end and shape results.
@@ -123,12 +122,12 @@ class CORE_EXPORT NGInlineItem {
   NGCollapseType EndCollapseType() const {
     return static_cast<NGCollapseType>(end_collapse_type_);
   }
-  void SetEndCollapseType(NGCollapseType type) { end_collapse_type_ = type; }
+  void SetEndCollapseType(NGCollapseType type);
 
-  // Whether the item may be affected by whitespace collapsing. Unlike the
-  // EndCollapseType() method this returns true even if a trailing space has
-  // been removed.
-  bool EndMayCollapse() const { return end_may_collapse_; }
+  // Whether the end collapsible space run contains a newline.
+  // Valid only when kCollapsible or kCollapsed.
+  bool IsEndCollapsibleNewline() const { return is_end_collapsible_newline_; }
+  void SetEndCollapseType(NGCollapseType type, bool is_newline);
 
   static void Split(Vector<NGInlineItem>&, unsigned index, unsigned offset);
 
@@ -189,7 +188,7 @@ class CORE_EXPORT NGInlineItem {
   unsigned should_create_box_fragment_ : 1;
   unsigned style_variant_ : 2;
   unsigned end_collapse_type_ : 2;  // NGCollapseType
-  unsigned end_may_collapse_ : 1;
+  unsigned is_end_collapsible_newline_ : 1;
   unsigned is_symbol_marker_ : 1;
   friend class NGInlineNode;
 };

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

@@ -21,29 +21,9 @@ NGInlineItemsBuilderTemplate<
 
 template <typename OffsetMappingBuilder>
 String NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::ToString() {
-  // Segment Break Transformation Rules[1] defines to keep trailing new lines in
-  // Phase I, but to remove after line break, in Phase II[2]. Although the spec
-  // defines so, trailing collapsible spaces at the end of an inline formatting
-  // context will be removed in Phase II and that removing here makes no
-  // differences.
-  //
-  // However, doing so reduces the opportunities to re-use NGInlineItem a lot in
-  // appending scenario, which is quite common. In order to re-use NGInlineItem
-  // as much as posssible, trailing spaces are removed in Phase II, exactly as
-  // defined in the spec.
-  //
-  // [1] https://drafts.csswg.org/css-text-3/#line-break-transform
-  // [2] https://drafts.csswg.org/css-text-3/#white-space-phase-2
-  return text_.ToString();
-}
-
-template <>
-String NGInlineItemsBuilderTemplate<NGOffsetMappingBuilder>::ToString() {
-  // While trailing collapsible space is kept as above, NGOffsetMappingBuilder
-  // assumes NGLineBreaker does not remove it. For now, remove only for
-  // NGOffsetMappingBuilder.
-  // TODO(kojii): Consider NGOffsetMappingBuilder to support NGLineBreaker to
-  // remove trailing spaces.
+  // Segment Break Transformation Rules[1] defines to keep trailing new lines,
+  // but it will be removed in Phase II[2]. We prefer not to add trailing new
+  // lines and collapsible spaces in Phase I.
   RemoveTrailingCollapsibleSpaceIfExists();
 
   return text_.ToString();
@@ -133,10 +113,8 @@ void AppendItem(Vector<NGInlineItem>* items,
                 unsigned start,
                 unsigned end,
                 const ComputedStyle* style = nullptr,
-                LayoutObject* layout_object = nullptr,
-                bool end_may_collapse = false) {
-  items->push_back(
-      NGInlineItem(type, start, end, style, layout_object, end_may_collapse));
+                LayoutObject* layout_object = nullptr) {
+  items->push_back(NGInlineItem(type, start, end, style, layout_object));
 }
 
 inline bool ShouldIgnore(UChar c) {
@@ -194,10 +172,6 @@ NGInlineItem* LastItemToCollapseWith(Vector<NGInlineItem>* items) {
   return nullptr;
 }
 
-inline bool MayCollapseWithLast(const NGInlineItem& item) {
-  return item.EndMayCollapse();
-}
-
 }  // anonymous namespace
 
 template <typename OffsetMappingBuilder>
@@ -210,33 +184,56 @@ bool NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::Append(
   // TODO(layout-dev): This could likely be optimized further.
   // TODO(layout-dev): Handle cases where the old items are not consecutive.
   const ComputedStyle& new_style = layout_text->StyleRef();
-  const ComputedStyle& old_style = *items[0]->Style();
   bool collapse_spaces = new_style.CollapseWhiteSpace();
-  if (collapse_spaces != old_style.CollapseWhiteSpace())
-    return false;
-
-  NGInlineItem* last_item = LastItemToCollapseWith(items_);
-  if (collapse_spaces) {
-    if (!last_item || MayCollapseWithLast(*last_item)) {
-      // If the original string starts with a collapsible space, it may be
-      // collapsed.
-      if (original_string[items[0]->StartOffset()] == kSpaceCharacter)
-        return false;
-    } else {
-      // If the start of the original string was collapsed, it may be
-      // restored.
-      const String& source_text = layout_text->GetText();
-      if (source_text.length() && IsCollapsibleSpace(source_text[0]) &&
-          original_string[items[0]->StartOffset()] != kSpaceCharacter)
-        return false;
+  if (NGInlineItem* last_item = LastItemToCollapseWith(items_)) {
+    const NGInlineItem& old_item0 = *items[0];
+    if (collapse_spaces) {
+      DCHECK_GT(old_item0.Length(), 0u);
+      switch (last_item->EndCollapseType()) {
+        case NGInlineItem::kCollapsible:
+          // If the original string starts with a collapsible space, it may be
+          // collapsed.
+          if (original_string[old_item0.StartOffset()] == kSpaceCharacter)
+            return false;
+          break;
+        case NGInlineItem::kNotCollapsible: {
+          // If the start of the original string was collapsed, it may be
+          // restored.
+          const String& source_text = layout_text->GetText();
+          if (source_text.length() && IsCollapsibleSpace(source_text[0]) &&
+              original_string[old_item0.StartOffset()] != kSpaceCharacter)
+            return false;
+          break;
+        }
+        case NGInlineItem::kCollapsed:
+          RestoreTrailingCollapsibleSpace(last_item);
+          return false;
+        case NGInlineItem::kOpaqueToCollapsing:
+          NOTREACHED();
+          break;
+      }
     }
-  }
 
-  // On nowrap -> wrap boundary, a break opporunity may be inserted.
-  if (last_item && !last_item->Style()->AutoWrap() && new_style.AutoWrap())
-    return false;
+    // On nowrap -> wrap boundary, a break opporunity may be inserted.
+    DCHECK(last_item->Style());
+    if (!last_item->Style()->AutoWrap() && new_style.AutoWrap())
+      return false;
+
+  } else if (collapse_spaces) {
+    // If the original string starts with a collapsible space, it may be
+    // collapsed because it is now a leading collapsible space.
+    const NGInlineItem& old_item0 = *items[0];
+    DCHECK_GT(old_item0.Length(), 0u);
+    if (original_string[old_item0.StartOffset()] == kSpaceCharacter)
+      return false;
+  }
 
   for (const NGInlineItem* item : items) {
+    // Collapsed space item at the start will not be restored, and that not
+    // needed to add.
+    if (!text_.length() && !item->Length() && collapse_spaces)
+      continue;
+
     unsigned start = text_.length();
     text_.Append(original_string, item->StartOffset(), item->Length());
 
@@ -304,6 +301,8 @@ void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::Append(
   EWhiteSpace whitespace = style->WhiteSpace();
   bool is_svg_text = layout_object && layout_object->IsSVGInlineText();
 
+  RestoreTrailingCollapsibleSpaceIfRemoved();
+
   if (!ComputedStyle::CollapseWhiteSpace(whitespace))
     AppendPreserveWhitespace(string, style, layout_object);
   else if (ComputedStyle::PreserveNewline(whitespace) && !is_svg_text)
@@ -335,9 +334,10 @@ void NGInlineItemsBuilderTemplate<
   NGInlineItem::NGCollapseType end_collapse = NGInlineItem::kNotCollapsible;
   unsigned i = 0;
   UChar c = string[i];
+  bool space_run_has_newline = false;
   if (IsCollapsibleSpace(c)) {
     // Find the end of the collapsible space run.
-    bool space_run_has_newline = MoveToEndOfCollapsibleSpaces(string, &i, &c);
+    space_run_has_newline = MoveToEndOfCollapsibleSpaces(string, &i, &c);
 
     // LayoutBR does not set preserve_newline, but should be preserved.
     if (UNLIKELY(space_run_has_newline && string.length() == 1 &&
@@ -356,15 +356,13 @@ void NGInlineItemsBuilderTemplate<
       } else {
         // The last item ends with a collapsible space this run should collapse
         // to. Collapse the entire space run in this item.
-        DCHECK(item->EndCollapseType() == NGInlineItem::kCollapsibleSpace ||
-               item->EndCollapseType() == NGInlineItem::kCollapsibleNewline);
+        DCHECK(item->EndCollapseType() == NGInlineItem::kCollapsible);
         insert_space = false;
 
         // If the space run either in this item or in the last item contains a
         // newline, apply segment break rules. This may result in removal of
         // the space in the last item.
-        if ((space_run_has_newline ||
-             item->EndCollapseType() == NGInlineItem::kCollapsibleNewline) &&
+        if ((space_run_has_newline || item->IsEndCollapsibleNewline()) &&
             item->Type() == NGInlineItem::kText &&
             ShouldRemoveNewline(text_, item->EndOffset() - 1, item->Style(),
                                 StringView(string, i), style)) {
@@ -416,15 +414,15 @@ void NGInlineItemsBuilderTemplate<
     // If this space run is at the end of this item, keep whether the
     // collapsible space run has a newline or not in the item.
     if (i == string.length()) {
-      end_collapse = space_run_has_newline ? NGInlineItem::kCollapsibleNewline
-                                           : NGInlineItem::kCollapsibleSpace;
+      end_collapse = NGInlineItem::kCollapsible;
     }
   } else {
     // If the last item ended with a collapsible space run with segment breaks,
     // apply segment break rules. This may result in removal of the space in the
     // last item.
     if (NGInlineItem* item = LastItemToCollapseWith(items_)) {
-      if (item->EndCollapseType() == NGInlineItem::kCollapsibleNewline &&
+      if (item->EndCollapseType() == NGInlineItem::kCollapsible &&
+          item->IsEndCollapsibleNewline() &&
           ShouldRemoveNewline(text_, item->EndOffset() - 1, item->Style(),
                               string, style)) {
         RemoveTrailingCollapsibleSpace(item);
@@ -449,14 +447,16 @@ void NGInlineItemsBuilderTemplate<
       text_.Append(string, start_of_non_space, i - start_of_non_space);
       mapping_builder_.AppendIdentityMapping(i - start_of_non_space);
 
-      if (i == string.length())
+      if (i == string.length()) {
+        DCHECK_EQ(end_collapse, NGInlineItem::kNotCollapsible);
         break;
+      }
 
       // Process a collapsible space run. First, find the end of the run.
       DCHECK_EQ(c, string[i]);
       DCHECK(IsCollapsibleSpace(c));
       unsigned start_of_spaces = i;
-      bool space_run_has_newline = MoveToEndOfCollapsibleSpaces(string, &i, &c);
+      space_run_has_newline = MoveToEndOfCollapsibleSpaces(string, &i, &c);
 
       // Because leading spaces are handled before this loop, no need to check
       // cross-item collapsing.
@@ -481,8 +481,7 @@ void NGInlineItemsBuilderTemplate<
       // If this space run is at the end of this item, keep whether the
       // collapsible space run has a newline or not in the item.
       if (i == string.length()) {
-        end_collapse = space_run_has_newline ? NGInlineItem::kCollapsibleNewline
-                                             : NGInlineItem::kCollapsibleSpace;
+        end_collapse = NGInlineItem::kCollapsible;
         break;
       }
     }
@@ -490,9 +489,9 @@ void NGInlineItemsBuilderTemplate<
 
   if (text_.length() > start_offset) {
     AppendItem(items_, NGInlineItem::kText, start_offset, text_.length(), style,
-               layout_object, end_collapse != NGInlineItem::kNotCollapsible);
+               layout_object);
     NGInlineItem& item = items_->back();
-    item.SetEndCollapseType(end_collapse);
+    item.SetEndCollapseType(end_collapse, space_run_has_newline);
     is_empty_inline_ &= item.IsEmptyItem();
   }
 }
@@ -570,7 +569,7 @@ void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendForcedBreak(
   // are leading spaces and that they should be collapsed.
   // Pretend that this item ends with a collapsible space, so that following
   // collapsible spaces can be collapsed.
-  items_->back().SetEndCollapseType(NGInlineItem::kCollapsibleSpace);
+  items_->back().SetEndCollapseType(NGInlineItem::kCollapsible, false);
 
   // Then re-add bidi controls to restore the bidi context.
   if (!bidi_context_.IsEmpty()) {
@@ -621,6 +620,7 @@ void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendAtomicInline(
     LayoutObject* layout_object) {
   typename OffsetMappingBuilder::SourceNodeScope scope(&mapping_builder_,
                                                        layout_object);
+  RestoreTrailingCollapsibleSpaceIfRemoved();
   Append(NGInlineItem::kAtomicInline, kObjectReplacementCharacter, style,
          layout_object);
 }
@@ -659,7 +659,7 @@ template <typename OffsetMappingBuilder>
 void NGInlineItemsBuilderTemplate<
     OffsetMappingBuilder>::RemoveTrailingCollapsibleSpaceIfExists() {
   if (NGInlineItem* item = LastItemToCollapseWith(items_)) {
-    if (item->EndCollapseType() != NGInlineItem::kNotCollapsible)
+    if (item->EndCollapseType() == NGInlineItem::kCollapsible)
       RemoveTrailingCollapsibleSpace(item);
   }
 }
@@ -669,9 +669,8 @@ template <typename OffsetMappingBuilder>
 void NGInlineItemsBuilderTemplate<
     OffsetMappingBuilder>::RemoveTrailingCollapsibleSpace(NGInlineItem* item) {
   DCHECK(item);
+  DCHECK_EQ(item->EndCollapseType(), NGInlineItem::kCollapsible);
   DCHECK_GT(item->Length(), 0u);
-  DCHECK(item->EndCollapseType() == NGInlineItem::kCollapsibleSpace ||
-         item->EndCollapseType() == NGInlineItem::kCollapsibleNewline);
 
   // A forced break pretends that it's a collapsible space, see
   // |AppendForcedBreak()|. It should not be removed.
@@ -679,6 +678,7 @@ void NGInlineItemsBuilderTemplate<
     return;
   DCHECK_EQ(item->Type(), NGInlineItem::kText);
 
+  DCHECK_GT(item->EndOffset(), item->StartOffset());
   unsigned space_offset = item->EndOffset() - 1;
   DCHECK_EQ(text_[space_offset], kSpaceCharacter);
   text_.erase(space_offset);
@@ -695,7 +695,7 @@ void NGInlineItemsBuilderTemplate<
     item = &(*items_)[index];
   } else {
     item->SetEndOffset(item->EndOffset() - 1);
-    item->SetEndCollapseType(NGInlineItem::kNotCollapsible);
+    item->SetEndCollapseType(NGInlineItem::kCollapsed);
     item++;
   }
 
@@ -706,6 +706,42 @@ void NGInlineItemsBuilderTemplate<
   }
 }
 
+// Restore removed collapsible space at the end of items.
+template <typename OffsetMappingBuilder>
+void NGInlineItemsBuilderTemplate<
+    OffsetMappingBuilder>::RestoreTrailingCollapsibleSpaceIfRemoved() {
+  if (NGInlineItem* last_item = LastItemToCollapseWith(items_)) {
+    if (last_item->EndCollapseType() == NGInlineItem::kCollapsed)
+      RestoreTrailingCollapsibleSpace(last_item);
+  }
+}
+
+// Restore removed collapsible space at the end of the specified item.
+template <typename OffsetMappingBuilder>
+void NGInlineItemsBuilderTemplate<
+    OffsetMappingBuilder>::RestoreTrailingCollapsibleSpace(NGInlineItem* item) {
+  DCHECK(item);
+  DCHECK(item->EndCollapseType() == NGInlineItem::kCollapsed);
+
+  // TODO(kojii): Implement StringBuilder::insert().
+  if (text_.length() == item->EndOffset()) {
+    text_.Append(' ');
+  } else {
+    String current = text_.ToString();
+    text_.Clear();
+    text_.Append(StringView(current, 0, item->EndOffset()));
+    text_.Append(' ');
+    text_.Append(StringView(current, item->EndOffset()));
+  }
+
+  item->SetEndOffset(item->EndOffset() + 1);
+  item->SetEndCollapseType(NGInlineItem::kCollapsible);
+
+  for (item++; item != items_->end(); item++) {
+    item->SetOffset(item->StartOffset() + 1, item->EndOffset() + 1);
+  }
+}
+
 template <typename OffsetMappingBuilder>
 void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::EnterBidiContext(
     LayoutObject* node,

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

@@ -154,13 +154,12 @@ class NGInlineItemsBuilderTemplate {
   void RemoveTrailingCollapsibleSpaceIfExists();
   void RemoveTrailingCollapsibleSpace(NGInlineItem*);
 
+  void RestoreTrailingCollapsibleSpaceIfRemoved();
+  void RestoreTrailingCollapsibleSpace(NGInlineItem*);
+
   void Exit(LayoutObject*);
 };
 
-template <>
-CORE_EXPORT String
-NGInlineItemsBuilderTemplate<NGOffsetMappingBuilder>::ToString();
-
 template <>
 CORE_EXPORT bool NGInlineItemsBuilderTemplate<NGOffsetMappingBuilder>::Append(
     const String&,

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

@@ -294,16 +294,12 @@ const NGOffsetMapping* NGInlineNode::ComputeOffsetMappingIfNeeded() {
     NGInlineItemsBuilderForOffsetMapping builder(&items);
     CollectInlinesInternal(GetLayoutBlockFlow(), &builder, nullptr);
     String text = builder.ToString();
-
-    // The trailing space of the text for offset mapping may be removed. If not,
-    // share the string instance.
-    if (text == data->text_content)
-      text = data->text_content;
+    DCHECK_EQ(data->text_content, text);
 
     // TODO(xiaochengh): This doesn't compute offset mapping correctly when
     // text-transform CSS property changes text length.
     NGOffsetMappingBuilder& mapping_builder = builder.GetOffsetMappingBuilder();
-    mapping_builder.SetDestinationString(text);
+    mapping_builder.SetDestinationString(data->text_content);
     data->offset_mapping =
         std::make_unique<NGOffsetMapping>(mapping_builder.Build());
   }