[LayoutNG] Fix letter-/word-spacing for following lines

This patch fixes |NGLineBreaker| to cache 'letter-/word-
spacing' values when it is constructed with a break token.

This seems to be an old bug. r527007 (crrev.com/c/846585)
initialized |current_style_| without calling
|SetCurrentStyle|. Then r590190 (crrev.com/c/1217708) skipped
caching locale and letter-/word-spacing in such case. The
issue has been there since then, but it seems r701601
(crrev.com/c/1826063) turned it to more frequent crashes.

Bug: 963521
Change-Id: I4abe207505917f351c8074a5e7e3cb139c9515d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1851625
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705051}

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

@@ -186,12 +186,14 @@ NGLineBreaker::NGLineBreaker(NGInlineNode node,
   break_iterator_.SetBreakSpace(BreakSpaceType::kBeforeSpaceRun);
 
   if (break_token) {
-    current_style_ = break_token->Style();
     item_index_ = break_token->ItemIndex();
     offset_ = break_token->TextOffset();
     break_iterator_.SetStartOffset(offset_);
     is_after_forced_break_ = break_token->IsForcedBreak();
     items_data_.AssertOffset(item_index_, offset_);
+    // TODO(crbug.com/1013040): |break_token->Style()| should not be nullptr.
+    if (const ComputedStyle* line_initial_style = break_token->Style())
+      SetCurrentStyle(*line_initial_style);
   }
 }
 
@@ -287,11 +289,13 @@ void NGLineBreaker::PrepareNextLine(NGLineInfo* line_info) {
     line_info->SetTextIndent(MinimumValueForLength(length, maximum_value));
   }
 
-  // Set the initial style of this line from the break token. Example:
+  // Set the initial style of this line from the line style, if the style from
+  // the end of previous line is not available. Example:
   //   <p>...<span>....</span></p>
   // When the line wraps in <span>, the 2nd line needs to start with the style
   // of the <span>.
-  SetCurrentStyle(current_style_ ? *current_style_ : line_info->LineStyle());
+  if (!current_style_)
+    SetCurrentStyle(line_info->LineStyle());
   ComputeBaseDirection();
   line_info->SetBaseDirection(base_direction_);
 
@@ -1826,6 +1830,23 @@ const ComputedStyle& NGLineBreaker::ComputeCurrentStyle(
 }
 
 void NGLineBreaker::SetCurrentStyle(const ComputedStyle& style) {
+  if (&style == current_style_.get()) {
+#if DCHECK_IS_ON()
+    // Check that cache fields are already setup correctly.
+    DCHECK_EQ(auto_wrap_, style.AutoWrap());
+    if (auto_wrap_) {
+      DCHECK_EQ(enable_soft_hyphen_, style.GetHyphens() != Hyphens::kNone);
+      DCHECK_EQ(break_iterator_.Locale(), style.LocaleForLineBreakIterator());
+    }
+    ShapeResultSpacing<String> spacing(spacing_.Text());
+    spacing.SetSpacing(style.GetFontDescription());
+    DCHECK_EQ(spacing.LetterSpacing(), spacing_.LetterSpacing());
+    DCHECK_EQ(spacing.WordSpacing(), spacing_.WordSpacing());
+#endif
+    return;
+  }
+  current_style_ = &style;
+
   auto_wrap_ = style.AutoWrap();
 
   if (auto_wrap_) {
@@ -1861,17 +1882,10 @@ void NGLineBreaker::SetCurrentStyle(const ComputedStyle& style) {
 
     if (style.WhiteSpace() == EWhiteSpace::kBreakSpaces)
       break_iterator_.SetBreakSpace(BreakSpaceType::kAfterEverySpace);
-  }
-
-  // The above calls are cheap & necessary. But the following are expensive
-  // and do not need to be reset every time if the style doesn't change,
-  // so avoid them if possible.
-  if (&style == current_style_.get())
-    return;
 
-  current_style_ = &style;
-  if (auto_wrap_)
     break_iterator_.SetLocale(style.LocaleForLineBreakIterator());
+  }
+
   spacing_.SetSpacing(style.GetFontDescription());
 }
 

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

@@ -306,6 +306,26 @@ TEST_F(NGLineBreakerTest, WrapLastWord) {
   EXPECT_EQ("AAA BB CC", ToString(lines[1], node));
 }
 
+TEST_F(NGLineBreakerTest, WrapLetterSpacing) {
+  NGInlineNode node = CreateInlineNode(R"HTML(
+    <!DOCTYPE html>
+    <style>
+    #container {
+      font: 10px/1 Times;
+      letter-spacing: 10px;
+      width: 0px;
+    }
+    </style>
+    <div id=container>Star Wars</div>
+  )HTML");
+
+  Vector<NGInlineItemResults> lines;
+  lines = BreakLines(node, LayoutUnit(100));
+  EXPECT_EQ(2u, lines.size());
+  EXPECT_EQ("Star", ToString(lines[0], node));
+  EXPECT_EQ("Wars", ToString(lines[1], node));
+}
+
 TEST_F(NGLineBreakerTest, BoundaryInWord) {
   LoadAhem();
   NGInlineNode node = CreateInlineNode(R"HTML(

third_party/blink/renderer/platform/fonts/shaping/shape_result_spacing.h

@@ -34,7 +34,8 @@ class PLATFORM_EXPORT ShapeResultSpacing final {
         is_after_expansion_(false) {}
 
   const TextContainerType& Text() const { return text_; }
-  float LetterSpacing() const { return letter_spacing_; }
+  float LetterSpacing() const { return has_spacing_ ? letter_spacing_ : .0f; }
+  float WordSpacing() const { return has_spacing_ ? word_spacing_ : .0f; }
   bool HasSpacing() const { return has_spacing_; }
   bool HasExpansion() const { return expansion_opportunity_count_; }
 

third_party/blink/renderer/platform/text/text_break_iterator.h

@@ -216,6 +216,7 @@ class PLATFORM_EXPORT LazyLineBreakIterator final {
     ReleaseIterator();
   }
 
+  const AtomicString& Locale() const { return locale_; }
   void SetLocale(const AtomicString& locale) {
     if (locale == locale_)
       return;