Reland "Improve performance of PreviousBreakOpportunity"

This reverts commit 20990963.

Reason for revert: The revert did not fix crbug.com/883963.
Requested to re-run bisect.

Original change's description:
> Revert "Improve performance of PreviousBreakOpportunity"
> 
> This reverts commit 99d504fd.
> 
> Reason for revert:
> 
> crbug.com/883963 bisected to this CL. I'm not sure how this
> affects print preview, reverting to see the effect.
> 
> This revert may affect LayoutNG performance, but should not
> affect the currently shipping line breaker.
> 
> Original change's description:
> > Improve performance of PreviousBreakOpportunity
> >
> > In the current layout engine, PreviousBreakOpportunity is used
> > only when mid-word break (break-all or break-word), but it is
> > much more heavily used in LayoutNG.
> >
> > LazyLineBreakIterator is designed for forward only.
> > PreviousBreakOpportunity is implemented by repeatedly calling
> > NextBreakablePosition, but since NextBreakablePosition look
> > for the next break opportunity until the end of the string,
> > when a very long word without break opportunity is given,
> > PreviousBreakOpportunity is O(n!).
> >
> > This patch changes it to O(n) by limiting the end position
> > NextBreakablePosition can look for.
> >
> > blink/perf_tests/layout/word-break-break-word.html consumes
> > 78% of the total time in LayoutNG. The average run is:
> >   Current engine:    469ms
> >   LayoutNG:       26,644ms
> >   This patch:      2,250ms
> >
> > It's still 4-5 times slower, more improvements will be in
> > following patches.
> >
> > Bug: 636993
> > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
> > Change-Id: I814e2c45c8030aa682c7f5e3a3b785b3c0733c84
> > Reviewed-on: https://chromium-review.googlesource.com/1095894
> > Commit-Queue: Koji Ishii <kojii@chromium.org>
> > Reviewed-by: Emil A Eklund <eae@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#567133}
> 
> TBR=eae@chromium.org,kojii@chromium.org
> 
> Bug: 636993, 883963
> Change-Id: I2c453a011208b014e342d7b44646eb90b5b7bbd2
> Reviewed-on: https://chromium-review.googlesource.com/c/1270396
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Reviewed-by: Koji Ishii <kojii@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597876}

TBR=eae@chromium.org,kojii@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 636993, 883963
Change-Id: I8e9881deca736c5b574dc287d04f9c2fffe76680
Reviewed-on: https://chromium-review.googlesource.com/c/1275985Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598754}

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

@@ -298,8 +298,8 @@ template <typename CharacterType,
           BreakSpaceType break_space>
 inline int LazyLineBreakIterator::NextBreakablePosition(
     int pos,
-    const CharacterType* str) const {
-  int len = static_cast<int>(string_.length());
+    const CharacterType* str,
+    int len) const {
   DCHECK_GE(pos, 0);
   DCHECK_GE(static_cast<unsigned>(pos), start_offset_);
   int next_break = -1;
@@ -379,30 +379,35 @@ inline int LazyLineBreakIterator::NextBreakablePosition(
 template <typename CharacterType, LineBreakType lineBreakType>
 inline int LazyLineBreakIterator::NextBreakablePosition(
     int pos,
-    const CharacterType* str) const {
+    const CharacterType* str,
+    int len) const {
   switch (break_space_) {
     case BreakSpaceType::kBeforeEverySpace:
       return NextBreakablePosition<CharacterType, lineBreakType,
-                                   BreakSpaceType::kBeforeEverySpace>(pos, str);
+                                   BreakSpaceType::kBeforeEverySpace>(pos, str,
+                                                                      len);
     case BreakSpaceType::kBeforeSpaceRun:
       return NextBreakablePosition<CharacterType, lineBreakType,
-                                   BreakSpaceType::kBeforeSpaceRun>(pos, str);
+                                   BreakSpaceType::kBeforeSpaceRun>(pos, str,
+                                                                    len);
   }
   NOTREACHED();
   return NextBreakablePosition<CharacterType, lineBreakType,
-                               BreakSpaceType::kBeforeEverySpace>(pos, str);
+                               BreakSpaceType::kBeforeEverySpace>(pos, str,
+                                                                  len);
 }
 
 template <LineBreakType lineBreakType>
-inline int LazyLineBreakIterator::NextBreakablePosition(int pos) const {
+inline int LazyLineBreakIterator::NextBreakablePosition(int pos,
+                                                        int len) const {
   if (UNLIKELY(string_.IsNull()))
     return 0;
   if (string_.Is8Bit()) {
-    return NextBreakablePosition<LChar, lineBreakType>(pos,
-                                                       string_.Characters8());
+    return NextBreakablePosition<LChar, lineBreakType>(
+        pos, string_.Characters8(), len);
   }
-  return NextBreakablePosition<UChar, lineBreakType>(pos,
-                                                     string_.Characters16());
+  return NextBreakablePosition<UChar, lineBreakType>(
+      pos, string_.Characters16(), len);
 }
 
 int LazyLineBreakIterator::NextBreakablePositionBreakCharacter(int pos) const {
@@ -415,16 +420,16 @@ int LazyLineBreakIterator::NextBreakablePositionBreakCharacter(int pos) const {
   return next != kTextBreakDone ? next + start_offset_ : string_.length();
 }
 
-int LazyLineBreakIterator::NextBreakablePosition(
-    int pos,
-    LineBreakType line_break_type) const {
+int LazyLineBreakIterator::NextBreakablePosition(int pos,
+                                                 LineBreakType line_break_type,
+                                                 int len) const {
   switch (line_break_type) {
     case LineBreakType::kNormal:
-      return NextBreakablePosition<LineBreakType::kNormal>(pos);
+      return NextBreakablePosition<LineBreakType::kNormal>(pos, len);
     case LineBreakType::kBreakAll:
-      return NextBreakablePosition<LineBreakType::kBreakAll>(pos);
+      return NextBreakablePosition<LineBreakType::kBreakAll>(pos, len);
     case LineBreakType::kKeepAll:
-      return NextBreakablePosition<LineBreakType::kKeepAll>(pos);
+      return NextBreakablePosition<LineBreakType::kKeepAll>(pos, len);
     case LineBreakType::kBreakCharacter:
       return NextBreakablePositionBreakCharacter(pos);
   }
@@ -432,9 +437,15 @@ int LazyLineBreakIterator::NextBreakablePosition(
   return NextBreakablePosition(pos, LineBreakType::kNormal);
 }
 
+int LazyLineBreakIterator::NextBreakablePosition(
+    int pos,
+    LineBreakType line_break_type) const {
+  return NextBreakablePosition(pos, line_break_type,
+                               static_cast<int>(string_.length()));
+}
+
 unsigned LazyLineBreakIterator::NextBreakOpportunity(unsigned offset) const {
-  int next_break = -1;
-  IsBreakable(offset, next_break);
+  int next_break = NextBreakablePosition(offset, break_type_);
   DCHECK_GE(next_break, 0);
   return next_break;
 }
@@ -442,10 +453,7 @@ unsigned LazyLineBreakIterator::NextBreakOpportunity(unsigned offset) const {
 unsigned LazyLineBreakIterator::NextBreakOpportunity(unsigned offset,
                                                      unsigned len) const {
   DCHECK_LE(len, string_.length());
-  // TODO(kojii): |len| is not utilized for perf benefit as
-  // |NextBreakablePosition| does not accept it due to a revert in
-  // crbug.com/883963.
-  int next_break = NextBreakablePosition(offset, break_type_);
+  int next_break = NextBreakablePosition(offset, break_type_, len);
   DCHECK_GE(next_break, 0);
   return next_break;
 }
@@ -453,9 +461,20 @@ unsigned LazyLineBreakIterator::NextBreakOpportunity(unsigned offset,
 unsigned LazyLineBreakIterator::PreviousBreakOpportunity(unsigned offset,
                                                          unsigned min) const {
   unsigned pos = std::min(offset, string_.length());
-  for (; pos > min; pos--) {
-    if (IsBreakable(pos))
-      return pos;
+  // +2 to ensure at least one code point is included.
+  unsigned end = std::min(pos + 2, string_.length());
+  while (pos > min) {
+    int next_break = NextBreakablePosition(pos, break_type_, end);
+    DCHECK_GE(next_break, 0);
+    if (static_cast<unsigned>(next_break) == pos)
+      return next_break;
+
+    // There's no break opportunities at |pos| or after.
+    end = pos;
+    if (string_.Is8Bit())
+      --pos;
+    else
+      U16_BACK_1(string_.Characters16(), 0, pos);
   }
   return min;
 }

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

@@ -242,7 +242,8 @@ class PLATFORM_EXPORT LazyLineBreakIterator final {
     // Limit length to pos + 1.
     // TODO(layout-dev): We should probably try to break out an actual
     // IsBreakable method from NextBreakablePosition and get rid of this hack.
-    int next_breakable = NextBreakablePosition(pos, break_type_);
+    int len = std::min(pos + 1, static_cast<int>(string_.length()));
+    int next_breakable = NextBreakablePosition(pos, break_type_, len);
     return pos == next_breakable;
   }
 
@@ -305,12 +306,13 @@ class PLATFORM_EXPORT LazyLineBreakIterator final {
   }
 
   template <typename CharacterType, LineBreakType, BreakSpaceType>
-  int NextBreakablePosition(int pos, const CharacterType* str) const;
+  int NextBreakablePosition(int pos, const CharacterType* str, int len) const;
   template <typename CharacterType, LineBreakType>
-  int NextBreakablePosition(int pos, const CharacterType* str) const;
+  int NextBreakablePosition(int pos, const CharacterType* str, int len) const;
   template <LineBreakType>
-  int NextBreakablePosition(int pos) const;
+  int NextBreakablePosition(int pos, int len) const;
   int NextBreakablePositionBreakCharacter(int pos) const;
+  int NextBreakablePosition(int pos, LineBreakType, int len) const;
   int NextBreakablePosition(int pos, LineBreakType) const;
 
   static const unsigned kPriorContextCapacity = 2;