Call |HandleTrailingSpaces| even if |shape_result| is nullptr

r796465 crrev.com/c/2344431 changed |HandleOverflow| to call
|HandleTrailingSpaces| only if |shape_result| is not nullptr.

crbug.com/1139513 found that this change caused M86 to hang.

M87 and later has a new trailing space code in r807457
crrev.com/c/2412307 and that the symptom is not observed.
However, this indicates that we may hang if we go to the
codepath in any other conditions.

This patch:
1. Reverts the behavior to M85/86.
2. Change the function to take the pointer of |shape_result|
   instead of ref to indicate it can be nullptr.

|HandleTrailingSpaces| needs |shape_result| only under certain
condition, and the current assumption is that the combination
will not occur.

Change-Id: Ide9f48e14657c8e3c893fd506fee8412216df25f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489730Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819722}

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

@@ -638,7 +638,7 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
 
   // If we're trailing, only trailing spaces can be included in this line.
   if (UNLIKELY(state_ == LineBreakState::kTrailing)) {
-    HandleTrailingSpaces(item, shape_result, line_info);
+    HandleTrailingSpaces(item, &shape_result, line_info);
     return;
   }
 
@@ -670,7 +670,7 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
     // rewind, but in most cases it will rewind.
     const String& text = Text();
     if (auto_wrap_ && IsBreakableSpace(text[offset_])) {
-      HandleTrailingSpaces(item, shape_result, line_info);
+      HandleTrailingSpaces(item, &shape_result, line_info);
       if (state_ != LineBreakState::kDone) {
         state_ = LineBreakState::kContinue;
         return;
@@ -718,7 +718,7 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
       // items follow, only trailable spaces if any. This is very common that
       // shortcut to handling trailing spaces.
       if (item_result->EndOffset() < item.EndOffset())
-        return HandleTrailingSpaces(item, shape_result, line_info);
+        return HandleTrailingSpaces(item, &shape_result, line_info);
 
       // The break point found at the end of this text item. Continue looking
       // next items, because the next item maybe trailable, or can prohibit
@@ -1155,29 +1155,26 @@ void NGLineBreaker::UpdateShapeResult(const NGLineInfo& line_info,
   item_result->inline_size = item_result->shape_result->SnappedWidth();
 }
 
-void NGLineBreaker::HandleTrailingSpacesIfNeeded(NGLineInfo* line_info) {
+void NGLineBreaker::HandleTrailingSpaces(NGLineInfo* line_info) {
   const Vector<NGInlineItem>& items = Items();
-  if (item_index_ >= items.size())
-    return;
-  const NGInlineItem& item = items[item_index_];
-  if (const ShapeResult* shape_result = item.TextShapeResult())
-    HandleTrailingSpaces(item, *shape_result, line_info);
+  if (item_index_ < items.size())
+    HandleTrailingSpaces(items[item_index_], line_info);
 }
 
 inline void NGLineBreaker::HandleTrailingSpaces(const NGInlineItem& item,
                                                 NGLineInfo* line_info) {
   const ShapeResult* shape_result = item.TextShapeResult();
-  DCHECK(shape_result);
-  HandleTrailingSpaces(item, *shape_result, line_info);
+  // Call |HandleTrailingSpaces| even if |item| does not have |ShapeResult|, so
+  // that we skip spaces.
+  HandleTrailingSpaces(item, shape_result, line_info);
 }
 
 void NGLineBreaker::HandleTrailingSpaces(const NGInlineItem& item,
-                                         const ShapeResult& shape_result,
+                                         const ShapeResult* shape_result,
                                          NGLineInfo* line_info) {
   DCHECK(item.Type() == NGInlineItem::kText ||
          (item.Type() == NGInlineItem::kControl &&
           Text()[item.StartOffset()] == kTabulationCharacter));
-  DCHECK(&shape_result);
   bool is_control_tab = item.Type() == NGInlineItem::kControl &&
                         Text()[item.StartOffset()] == kTabulationCharacter;
   DCHECK(item.Type() == NGInlineItem::kText || is_control_tab);
@@ -1228,10 +1225,11 @@ void NGLineBreaker::HandleTrailingSpaces(const NGInlineItem& item,
     // Probably we can (koji). We would need to review usage of these
     // item results, and change them to use "non_hangable_run_end"
     // instead.
+    DCHECK(shape_result);
     NGInlineItemResult* item_result = AddItem(item, end, line_info);
     item_result->non_hangable_run_end = offset_;
     item_result->has_only_trailing_spaces = true;
-    item_result->shape_result = ShapeResultView::Create(&shape_result);
+    item_result->shape_result = ShapeResultView::Create(shape_result);
     if (item_result->StartOffset() == item.StartOffset() &&
         item_result->EndOffset() == item.EndOffset()) {
       item_result->inline_size = item_result->shape_result
@@ -2089,7 +2087,7 @@ void NGLineBreaker::RewindOverflow(unsigned new_end, NGLineInfo* line_info) {
           // and break there.
           // TODO: optimize more?
           Rewind(index, line_info);
-          HandleTrailingSpacesIfNeeded(line_info);
+          HandleTrailingSpaces(line_info);
 #if DCHECK_IS_ON()
           item_results.back().CheckConsistency(false);
 #endif

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

@@ -157,10 +157,10 @@ class CORE_EXPORT NGLineBreaker {
                                        unsigned start,
                                        unsigned end);
 
-  void HandleTrailingSpacesIfNeeded(NGLineInfo*);
+  void HandleTrailingSpaces(NGLineInfo*);
   void HandleTrailingSpaces(const NGInlineItem&, NGLineInfo*);
   void HandleTrailingSpaces(const NGInlineItem&,
-                            const ShapeResult&,
+                            const ShapeResult*,
                             NGLineInfo*);
   void RemoveTrailingCollapsibleSpace(NGLineInfo*);
   LayoutUnit TrailingCollapsibleSpaceWidth(NGLineInfo*);

third_party/blink/web_tests/external/wpt/css/css-text/crashtests/trailing-space-with-cr-crash.html

+<!DOCTYPE html>
+<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-property">
+<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
+<style>
+div {
+  width: 100px;
+  white-space: pre-wrap;
+  word-break: break-word;
+  border: 1px solid blue;
+}
+.atomic {
+  display: inline-block;
+  width: 99px;
+  height: 1em;
+  background: orange;
+}
+</style>
+<div><span class="atomic"></span>&#x0D; <span class="atomic"></span></div>