Remove CachedRunSegmenter

This patch removes CachedRunSegmenter, introduced in
CL:1053346, because CL:1083914 supercedes this improvement
by LayoutNG not invoking RunSegmenter multiple times.

Other recent changes such as pre-allocating are preserved
as they are still valid.

Bug: 636993, 645035
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I83e74a58b79b856df1f046b7fa90b485609cd85a
Reviewed-on: https://chromium-review.googlesource.com/1087191Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568085}

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

@@ -357,9 +357,7 @@ void NGInlineNode::SegmentScriptRuns(NGInlineNodeData* data) {
   RunSegmenter segmenter(text_content.Characters16(), text_content.length(),
                          FontOrientation::kHorizontal);
   RunSegmenter::RunSegmenterRange range = RunSegmenter::NullRange();
-  for (unsigned item_index = 0, segmenter_start = 0;
-       segmenter.ConsumePast(segmenter_start, &range);
-       segmenter_start = range.end) {
+  for (unsigned item_index = 0; segmenter.Consume(&range);) {
     DCHECK_EQ(items[item_index].start_offset_, range.start);
     item_index = NGInlineItem::PopulateItemsFromRun(items, item_index, range);
   }

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

@@ -919,28 +919,6 @@ void HarfBuzzShaper::ShapeSegment(
   }
 }
 
-// Get a RunSegmenter for the specified range and FontOrientation.
-//
-// Because RunSegmenter needs pre-context but is foward-only, it needs to start
-// from the beginning for each range. By caching the last-used RunSegmenter,
-// when caller shapes multiple ranges of a string incrementally, we can re-use
-// existing instance and avoid scanning from the beginning.
-RunSegmenter* HarfBuzzShaper::CachedRunSegmenter(
-    unsigned start,
-    unsigned end,
-    FontOrientation orientation) const {
-  if (!run_segmenter_.has_value() || run_segmenter_->HasProgressedPast(start) ||
-      !run_segmenter_->Supports(orientation)) {
-    // RunSegmenter is not created yet, or existing instance cannot be reused
-    // for the new range. Create a new instance.
-    //
-    // Run segmentation needs to operate on the entire string, regardless of the
-    // shaping window (defined by the start and end parameters).
-    run_segmenter_.emplace(text_, text_length_, orientation);
-  }
-  return &run_segmenter_.value();
-}
-
 scoped_refptr<ShapeResult> HarfBuzzShaper::Shape(
     const Font* font,
     TextDirection direction,
@@ -967,14 +945,12 @@ scoped_refptr<ShapeResult> HarfBuzzShaper::Shape(
   if (pre_segmented) {
     ShapeSegment(&range_data, *pre_segmented, result.get());
   } else {
-    // TODO(kojii): With pre_segmented, CachedRunSegmenter is probably no longer
-    // needed. Confirm and cleanup.
-    RunSegmenter* run_segmenter = CachedRunSegmenter(
-        start, end, font->GetFontDescription().Orientation());
+    // Run segmentation needs to operate on the entire string, regardless of the
+    // shaping window (defined by the start and end parameters).
+    RunSegmenter run_segmenter(text_, text_length_,
+                               font->GetFontDescription().Orientation());
     RunSegmenter::RunSegmenterRange segment_range = RunSegmenter::NullRange();
-    for (unsigned run_segmenter_start = start;
-         run_segmenter->ConsumePast(run_segmenter_start, &segment_range);
-         run_segmenter_start = segment_range.end) {
+    while (run_segmenter.Consume(&segment_range)) {
       // Only shape segments overlapping with the range indicated by start and
       // end. Not only those strictly within.
       if (start < segment_range.end && end > segment_range.start)

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

@@ -31,7 +31,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_FONTS_SHAPING_HARF_BUZZ_SHAPER_H_
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_FONTS_SHAPING_HARF_BUZZ_SHAPER_H_
 
-#include "base/optional.h"
 #include "third_party/blink/renderer/platform/fonts/shaping/run_segmenter.h"
 #include "third_party/blink/renderer/platform/fonts/shaping/shape_result.h"
 #include "third_party/blink/renderer/platform/wtf/allocator.h"
@@ -46,8 +45,6 @@ struct ReshapeQueueItem;
 struct RangeData;
 struct BufferSlice;
 
-enum class FontOrientation;
-
 class PLATFORM_EXPORT HarfBuzzShaper final {
  public:
   HarfBuzzShaper(const UChar*, unsigned length);
@@ -108,15 +105,8 @@ class PLATFORM_EXPORT HarfBuzzShaper final {
                     const BufferSlice&,
                     ShapeResult*) const;
 
-  RunSegmenter* CachedRunSegmenter(unsigned start,
-                                   unsigned end,
-                                   FontOrientation) const;
-
   const UChar* text_;
   unsigned text_length_;
-
-  // Cache an instnace of |RunSegmenter|. See |CachedRunSegmenter()|.
-  mutable base::Optional<RunSegmenter> run_segmenter_;
 };
 
 }  // namespace blink

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

@@ -74,34 +74,9 @@ void RunSegmenter::ConsumeSymbolsIteratorPastLastSplit() {
   }
 }
 
-bool RunSegmenter::Supports(FontOrientation orientation) const {
-  bool needs_orientation_iterator =
-      orientation == FontOrientation::kVerticalMixed;
-  return (needs_orientation_iterator == !!orientation_iterator_.get());
-}
-
-bool RunSegmenter::HasProgressedPast(unsigned start) const {
-  return candidate_range_.start > start;
-}
-
-bool RunSegmenter::ConsumePast(unsigned start, RunSegmenterRange* next_range) {
-  DCHECK(!HasProgressedPast(start));
-  while (true) {
-    // Check the candidate range first, before checking |at_end_| because
-    // |Consume()| sets |at_end_| even when the current range is valid.
-    if (candidate_range_.start <= start && start < candidate_range_.end) {
-      *next_range = candidate_range_;
-      return true;
-    }
-
-    if (!Consume())
-      return false;
-  }
-}
-
 // Consume the input until the next range. Returns false if no more ranges are
 // available.
-bool RunSegmenter::Consume() {
+bool RunSegmenter::Consume(RunSegmenterRange* next_range) {
   if (at_end_)
     return false;
 
@@ -126,6 +101,7 @@ bool RunSegmenter::Consume() {
 
   candidate_range_.start = candidate_range_.end;
   candidate_range_.end = last_split_;
+  *next_range = candidate_range_;
 
   at_end_ = last_split_ == buffer_size_;
   return true;

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

@@ -39,20 +39,7 @@ class PLATFORM_EXPORT RunSegmenter {
   // Initialize a RunSegmenter.
   RunSegmenter(const UChar* buffer, unsigned buffer_size, FontOrientation);
 
-  // Returns true if the internal state is past |start|. RunSegmenter is forward
-  // incremental that once it's past a point, it cannot be used for text before
-  // it. On the ohter hand, if it's not, caller can use the existing instance.
-  bool HasProgressedPast(unsigned start) const;
-
-  // Returns true if this instance supports the specified FontOrientation.
-  bool Supports(FontOrientation) const;
-
-  // Advance to the range that includes |start| and return the range. Returns
-  // false if there are no such ranges.
-  //
-  // Callers need to update |start| of the next call to the previous range end
-  // when they need next range.
-  bool ConsumePast(unsigned start, RunSegmenterRange*);
+  bool Consume(RunSegmenterRange*);
 
   static RunSegmenterRange NullRange() {
     return {0, 0, USCRIPT_INVALID_CODE, OrientationIterator::kOrientationKeep,
@@ -63,7 +50,6 @@ class PLATFORM_EXPORT RunSegmenter {
   void ConsumeOrientationIteratorPastLastSplit();
   void ConsumeScriptIteratorPastLastSplit();
   void ConsumeSymbolsIteratorPastLastSplit();
-  bool Consume();
 
   unsigned buffer_size_;
   RunSegmenterRange candidate_range_;

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

@@ -69,9 +69,7 @@ class RunSegmenterTest : public testing::Test {
                   const Vector<SegmenterExpectedRun>& expect) {
     RunSegmenter::RunSegmenterRange segmenter_range;
     unsigned long run_count = 0;
-    for (unsigned run_segmenter_start = 0;
-         run_segmenter->ConsumePast(run_segmenter_start, &segmenter_range);
-         run_segmenter_start = segmenter_range.end) {
+    while (run_segmenter->Consume(&segmenter_range)) {
       ASSERT_LT(run_count, expect.size());
       ASSERT_EQ(expect[run_count].start, segmenter_range.start);
       ASSERT_EQ(expect[run_count].limit, segmenter_range.end);
@@ -92,7 +90,7 @@ TEST_F(RunSegmenterTest, Empty) {
       0, 0, USCRIPT_INVALID_CODE, OrientationIterator::kOrientationKeep};
   RunSegmenter run_segmenter(empty.Characters16(), empty.length(),
                              FontOrientation::kVerticalMixed);
-  DCHECK(!run_segmenter.ConsumePast(0, &segmenter_range));
+  DCHECK(!run_segmenter.Consume(&segmenter_range));
   ASSERT_EQ(segmenter_range.start, 0u);
   ASSERT_EQ(segmenter_range.end, 0u);
   ASSERT_EQ(segmenter_range.script, USCRIPT_INVALID_CODE);
@@ -243,29 +241,4 @@ TEST_F(RunSegmenterTest, EmojiSubdivisionFlags) {
         FontFallbackPriority::kEmojiEmoji}});
 }
 
-// Test ConsumePast with |start| advances to the run that includes |start|.
-TEST_F(RunSegmenterTest, PastFirstRun) {
-  String text(u"αβγあいうabc");
-  RunSegmenter run_segmenter(text.Characters16(), text.length(),
-                             FontOrientation::kHorizontal);
-  RunSegmenter::RunSegmenterRange segmenter_range;
-  EXPECT_TRUE(run_segmenter.ConsumePast(7, &segmenter_range));
-  EXPECT_EQ(segmenter_range.start, 6u);
-  EXPECT_EQ(segmenter_range.end, 9u);
-  EXPECT_EQ(segmenter_range.script, USCRIPT_LATIN);
-  EXPECT_EQ(segmenter_range.render_orientation,
-            OrientationIterator::kOrientationKeep);
-  EXPECT_EQ(segmenter_range.font_fallback_priority,
-            FontFallbackPriority::kText);
-}
-
-// Test ConsumePast with |start| larger than buffer size returns false.
-TEST_F(RunSegmenterTest, PastBufferLength) {
-  String text(u"abc");
-  RunSegmenter run_segmenter(text.Characters16(), text.length(),
-                             FontOrientation::kHorizontal);
-  RunSegmenter::RunSegmenterRange segmenter_range;
-  EXPECT_FALSE(run_segmenter.ConsumePast(4, &segmenter_range));
-}
-
 }  // namespace blink