Update ShapeResult::start_index_ when there are no runs

This patch updates |start_index_| when there are no runs,
because it is generally done when a new run is inserted.

This is a speculative fix. While I hit a DCHECK because of
uninitialized |start_index_| once, I cannot reproduce
consistently, nor in unit tests using the string I hit in
the debugger on http://switchsoku.com/soft/splatoon2/28757 .

Bug: 896456
Change-Id: I6b1f65e770ebe7e8d2156a9056d7e57666d61d31
Reviewed-on: https://chromium-review.googlesource.com/c/1304276Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603645}

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

@@ -1004,9 +1004,9 @@ scoped_refptr<ShapeResult> HarfBuzzShaper::Shape(
     }
   }
 
-  // Ensure we have at least one run for StartIndexForResult().
-  if (UNLIKELY(result->runs_.IsEmpty() && start))
-    result->InsertRunForIndex(start);
+  // Ensure |start_index_| is updated even when no runs were inserted.
+  if (UNLIKELY(result->runs_.IsEmpty()))
+    result->start_index_ = start;
 
 #if DCHECK_IS_ON()
   if (result)

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

@@ -533,13 +533,22 @@ TEST_F(HarfBuzzShaperTest, ShapeVerticalMixed) {
   EXPECT_EQ(result->Bounds(), composite_result->Bounds());
 }
 
-TEST_P(ShapeParameterTest, MissingGlyph) {
+class ShapeStringTest : public HarfBuzzShaperTest,
+                        public testing::WithParamInterface<const char16_t*> {};
+
+INSTANTIATE_TEST_CASE_P(HarfBuzzShaperTest,
+                        ShapeStringTest,
+                        testing::Values(
                             // U+FFF0 is not assigned as of Unicode 10.0.
-  String string(
-      u"\uFFF0"
-      u"Hello");
+                            u"\uFFF0",
+                            u"\uFFF0Hello",
+                            // U+00AD SOFT HYPHEN often does not have glyphs.
+                            u"\u00AD"));
+
+TEST_P(ShapeStringTest, MissingGlyph) {
+  String string(GetParam());
   HarfBuzzShaper shaper(string);
-  scoped_refptr<ShapeResult> result = ShapeWithParameter(&shaper);
+  scoped_refptr<ShapeResult> result = shaper.Shape(&font, TextDirection::kLtr);
   EXPECT_EQ(0u, result->StartIndexForResult());
   EXPECT_EQ(string.length(), result->EndIndexForResult());
 }

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

@@ -1219,18 +1219,6 @@ void ShapeResult::InsertRun(std::unique_ptr<ShapeResult::RunInfo> run) {
   UpdateStartIndex();
 }
 
-// Insert a |RunInfo| without glyphs. |StartIndexForResult()| needs a run to
-// compute the start character index. When all glyphs are missing, this function
-// synthesize a run without glyphs.
-void ShapeResult::InsertRunForIndex(unsigned start_character_index) {
-  DCHECK(runs_.IsEmpty());
-  runs_.push_back(std::make_unique<RunInfo>(
-      primary_font_.get(), !Rtl() ? HB_DIRECTION_LTR : HB_DIRECTION_RTL,
-      CanvasRotationInVertical::kRegular, HB_SCRIPT_UNKNOWN,
-      start_character_index, 0, num_characters_));
-  UpdateStartIndex();
-}
-
 ShapeResult::RunInfo* ShapeResult::InsertRunForTesting(
     unsigned start_index,
     unsigned num_characters,

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

@@ -389,7 +389,6 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
                  unsigned num_glyphs,
                  hb_buffer_t*);
   void InsertRun(std::unique_ptr<ShapeResult::RunInfo>);
-  void InsertRunForIndex(unsigned start_character_index);
   void ReorderRtlRuns(unsigned run_size_before);
   unsigned ComputeStartIndex() const;
   void UpdateStartIndex();