[LayoutNG] Fix may_break_inside in NGLineBreaker

This patch fixes |may_break_inside| in |NGLineBreaker|.

Issue 1002442 revealed an error case in |ShapingLineBreaker|.
In that case, it returns a result whose size is larger than
the available space.

Before this patch, |NGLineBreaker| assumes that the line
overflows if the result size is larger than the available
size, and sets |may_break_inside| to |false|, which
prevents breaking in |HandleOverflow()|. This can cause
rather a large layout error.

crrev.com/c/1797954 tried to fix the error in
|ShapingLineBreaker|, but it looks like some tests is relying
on the behavior and it was reverted.

This patch instead makes |NGLineBreaker| more robust, by
using the actual overflow state from |ShapingLineBreaker| to
compute |may_break_inside|. The error in |ShapingLineBreaker|
is not fixed yet, but with this fix, the worst result is to
not able to fit a word that can fit by less than 1px or so.

Further work for better correctness will be tracked in
issue 1003742.

Bug: 1002442, 1003742
Change-Id: I13a3172abc8a0d2f5f78b2dd0b57b85de593fdff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1803019Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696621}

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

@@ -579,7 +579,6 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
             !override_break_anywhere_));
     position_ += item_result->inline_size;
     DCHECK_EQ(break_result == kSuccess, position_ <= available_width);
-    item_result->may_break_inside = break_result == kSuccess;
     MoveToNextOf(*item_result);
 
     if (break_result == kSuccess ||
@@ -677,12 +676,12 @@ NGLineBreaker::BreakResult NGLineBreaker::BreakText(
   unsigned try_count = 0;
 #endif
   LayoutUnit inline_size;
+  ShapingLineBreaker::Result result;
   while (true) {
 #if DCHECK_IS_ON()
     ++try_count;
     DCHECK_LE(try_count, 2u);
 #endif
-    ShapingLineBreaker::Result result;
     scoped_refptr<const ShapeResultView> shape_result = breaker.ShapeLine(
         item_result->start_offset, available_width.ClampNegativeToZero(),
         options, &result);
@@ -752,6 +751,13 @@ NGLineBreaker::BreakResult NGLineBreaker::BreakText(
       item_result->can_break_after = true;
     trailing_whitespace_ = WhitespaceState::kUnknown;
   }
+
+  // This result is not breakable any further if overflow. This information is
+  // useful to optimize |HandleOverflow()|.
+  item_result->may_break_inside = !result.is_overflow;
+
+  // TODO(crbug.com/1003742): We should use |result.is_overflow| here. For now,
+  // use |inline_size| because some tests rely on this behavior.
   return inline_size <= available_width ? kSuccess : kOverflow;
 }
 

third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker.cc

@@ -216,6 +216,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
   unsigned range_end = result_->EndIndex();
   DCHECK_GE(start, range_start);
   DCHECK_LT(start, range_end);
+  result_out->is_overflow = false;
   result_out->is_hyphenated = false;
   const String& text = GetText();
 
@@ -254,8 +255,8 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
   // Find the next break opportunity after the candidate_break.
   BreakOpportunity break_opportunity =
       PreviousBreakOpportunity(candidate_break, start);
-  bool is_overflow = break_opportunity.offset <= start;
-  if (is_overflow) {
+  result_out->is_overflow = break_opportunity.offset <= start;
+  if (result_out->is_overflow) {
     if (options & kNoResultIfOverflow)
       return nullptr;
     // No need to scan past range_end for a break oppertunity.
@@ -327,7 +328,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
 
       // If previously determined to let it overflow, reshape the line end.
       DCHECK_LE(break_opportunity.offset, range_end);
-      if (UNLIKELY(is_overflow)) {
+      if (UNLIKELY(result_out->is_overflow)) {
         line_end_result = Shape(last_safe, break_opportunity.offset);
         break;
       }
@@ -353,6 +354,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
       // This line will overflow, but there are multiple choices to break,
       // because none can fit. The one after candidate_break is better for
       // ligatures, but the one before is better for kernings.
+      result_out->is_overflow = true;
       break_opportunity = PreviousBreakOpportunity(candidate_break, start);
       if (break_opportunity.offset <= start) {
         break_opportunity = NextBreakOpportunity(
@@ -363,7 +365,6 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
         }
       }
       // Loop once more to compute last_safe for the new break opportunity.
-      is_overflow = true;
     }
   }
   // It is critical to move forward, or callers may end up in an infinite loop.

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

@@ -56,6 +56,11 @@ class PLATFORM_EXPORT ShapingLineBreaker final {
     // Indicates the resulting break offset.
     unsigned break_offset;
 
+    // True if there were no break opportunities that can fit. When this is
+    // false, the result width should be smaller than or equal to the available
+    // space.
+    bool is_overflow;
+
     // True if the break is hyphenated, either by automatic hyphenation or
     // soft-hyphen characters.
     // The hyphen glyph is not included in the |ShapeResult|, and that appending

third_party/blink/web_tests/external/wpt/css/css-text/line-breaking/line-breaking-020.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Text Test: Simple line breaking test</title>
+<link rel="author" title="Koji Ishii" href="mailto:kojiishi@gmail.com">
+<link rel="help" href="https://crbug.com/1002442">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+body {
+  font-family: Arial, Helvetica, sans-serif;
+  font-size: 12px;
+}
+p {
+  width: 460px;
+}
+</style>
+<body>
+<p>
+abschließend geklärt. Allerdings scheint eine gewisse <b>genetische Veranlagung</b> eine
+Zahl der Typ-1-Diabetiker gerade unter Kleinkindern stetig wächst. Daher rücken auch <b>Virusinfektionen</b> in den Fokus
+</p>
+<script>
+test(() => {
+  for (let e of document.getElementsByTagName('b')) {
+    let bounds = e.getBoundingClientRect();
+    assert_less_than_equal(bounds.x + bounds.width, 460);
+  }
+});
+</script>
+</body>