RenderTextHarfBuzz: Check CreateSkiaTypeface return value against NULL

BUG=398712

Review URL: https://codereview.chromium.org/435583003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287250 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 76 insertions, 14 deletions

diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index bc848c3..f5cb746 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -175,6 +175,7 @@ struct Line {
 };
 
 // Creates an SkTypeface from a font |family| name and a |gfx::Font::FontStyle|.
+// May return NULL.
 skia::RefPtr<SkTypeface> CreateSkiaTypeface(const std::string& family,
                                             int style);
 
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index 5c2ecb3..a1aced1 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -454,6 +454,12 @@ void TextRunHarfBuzz::GetClusterAt(size_t pos,
   DCHECK(chars);
   DCHECK(glyphs);
 
+  if (glyph_count == 0) {
+    *chars = range;
+    *glyphs = Range();
+    return;
+  }
+
   if (is_rtl) {
     GetClusterAtImpl(pos, range, glyph_to_char.rbegin(), glyph_to_char.rend(),
         true, chars, glyphs);
@@ -491,6 +497,8 @@ Range TextRunHarfBuzz::GetGraphemeBounds(
     base::i18n::BreakIterator* grapheme_iterator,
     size_t text_index) {
   DCHECK_LT(text_index, range.end());
+  if (glyph_count == 0)
+    return Range(preceding_run_widths, preceding_run_widths + width);
 
   Range chars;
   Range glyphs;
@@ -1008,25 +1016,35 @@ void RenderTextHarfBuzz::ShapeRun(internal::TextRunHarfBuzz* run) {
   const std::string primary_font_name = primary_font.GetFontName();
   run->font_size = primary_font.GetFontSize();
 
+  size_t best_font_missing = std::numeric_limits<size_t>::max();
+  std::string best_font;
+  std::string current_font;
+
   // Try shaping with |primary_font|.
-  ShapeRunWithFont(run, primary_font_name);
-  size_t best_font_missing = run->CountMissingGlyphs();
-  if (best_font_missing == 0)
-    return;
-  std::string best_font = primary_font_name;
+  if (ShapeRunWithFont(run, primary_font_name)) {
+    current_font = primary_font_name;
+    size_t current_missing = run->CountMissingGlyphs();
+    if (current_missing == 0)
+      return;
+    if (current_missing < best_font_missing) {
+      best_font_missing = current_missing;
+      best_font = current_font;
+    }
+  }
 
 #if defined(OS_WIN)
   Font uniscribe_font;
   const base::char16* run_text = &(GetLayoutText()[run->range.start()]);
   if (GetUniscribeFallbackFont(primary_font, run_text, run->range.length(),
-                               &uniscribe_font)) {
-    ShapeRunWithFont(run, uniscribe_font.GetFontName());
+                               &uniscribe_font) &&
+      ShapeRunWithFont(run, uniscribe_font.GetFontName())) {
+    current_font = uniscribe_font.GetFontName();
     size_t current_missing = run->CountMissingGlyphs();
     if (current_missing == 0)
       return;
     if (current_missing < best_font_missing) {
       best_font_missing = current_missing;
-      best_font = uniscribe_font.GetFontName();
+      best_font = current_font;
     }
   }
 #endif
@@ -1035,23 +1053,35 @@ void RenderTextHarfBuzz::ShapeRun(internal::TextRunHarfBuzz* run) {
   // |primary_font|.
   std::vector<std::string> fonts = GetFallbackFontFamilies(primary_font_name);
   for (size_t i = 1; i < fonts.size(); ++i) {
-    ShapeRunWithFont(run, fonts[i]);
+    if (!ShapeRunWithFont(run, fonts[i]))
+      continue;
+    current_font = fonts[i];
     size_t current_missing = run->CountMissingGlyphs();
     if (current_missing == 0)
       return;
     if (current_missing < best_font_missing) {
       best_font_missing = current_missing;
-      best_font = fonts[i];
+      best_font = current_font;
     }
   }
 
-  ShapeRunWithFont(run, best_font);
+  if (!best_font.empty() &&
+      (best_font == current_font || ShapeRunWithFont(run, best_font))) {
+    return;
+  }
+
+  run->glyph_count = 0;
+  run->width = 0;
 }
 
-void RenderTextHarfBuzz::ShapeRunWithFont(internal::TextRunHarfBuzz* run,
+bool RenderTextHarfBuzz::ShapeRunWithFont(internal::TextRunHarfBuzz* run,
                                           const std::string& font_family) {
   const base::string16& text = GetLayoutText();
-  run->skia_face = internal::CreateSkiaTypeface(font_family, run->font_style);
+  skia::RefPtr<SkTypeface> skia_face =
+      internal::CreateSkiaTypeface(font_family, run->font_style);
+  if (skia_face == NULL)
+    return false;
+  run->skia_face = skia_face;
   hb_font_t* harfbuzz_font = CreateHarfBuzzFont(run->skia_face.get(),
                                                 run->font_size);
 
@@ -1094,6 +1124,7 @@ void RenderTextHarfBuzz::ShapeRunWithFont(internal::TextRunHarfBuzz* run,
 
   hb_buffer_destroy(buffer);
   hb_font_destroy(harfbuzz_font);
+  return true;
 }
 
 }  // namespace gfx
diff --git a/ui/gfx/render_text_harfbuzz.h b/ui/gfx/render_text_harfbuzz.h
index 7c304c0..b028a37a 100644
--- a/ui/gfx/render_text_harfbuzz.h
+++ b/ui/gfx/render_text_harfbuzz.h
@@ -107,6 +107,7 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_BreakRunsByUnicodeBlocks);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_SubglyphGraphemeCases);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_SubglyphGraphemePartition);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_NonExistentFont);
 
   // Return the run index that contains the argument; or the length of the
   // |runs_| vector if argument exceeds the text length or width.
@@ -126,7 +127,7 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
 
   // Shape the glyphs needed for the text |run|.
   void ShapeRun(internal::TextRunHarfBuzz* run);
-  void ShapeRunWithFont(internal::TextRunHarfBuzz* run,
+  bool ShapeRunWithFont(internal::TextRunHarfBuzz* run,
                         const std::string& font);
 
   // Text runs in logical order.
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index c2b8975..751942e 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -2262,4 +2262,33 @@ TEST_F(RenderTextTest, HarfBuzz_GlyphBounds) {
   }
 }
 
+// Ensure that shaping with a non-existent font does not cause a crash.
+TEST_F(RenderTextTest, HarfBuzz_NonExistentFont) {
+  RenderTextHarfBuzz render_text;
+  render_text.SetText(ASCIIToUTF16("test"));
+  render_text.EnsureLayout();
+  ASSERT_EQ(1U, render_text.runs_.size());
+  internal::TextRunHarfBuzz* run = render_text.runs_[0];
+  render_text.ShapeRunWithFont(run, "TheFontThatDoesntExist");
+}
+
+// Ensure an empty run returns sane values to queries.
+TEST_F(RenderTextTest, HarfBuzz_EmptyRun) {
+  internal::TextRunHarfBuzz run;
+  const base::string16 kString = ASCIIToUTF16("abcdefgh");
+  scoped_ptr<base::i18n::BreakIterator> iter(new base::i18n::BreakIterator(
+      kString, base::i18n::BreakIterator::BREAK_CHARACTER));
+  ASSERT_TRUE(iter->Init());
+
+  run.range = Range(3, 8);
+  run.glyph_count = 0;
+  EXPECT_EQ(Range(0, 0), run.CharRangeToGlyphRange(Range(4, 5)));
+  EXPECT_EQ(Range(0, 0), run.GetGraphemeBounds(iter.get(), 4));
+  Range chars;
+  Range glyphs;
+  run.GetClusterAt(4, &chars, &glyphs);
+  EXPECT_EQ(Range(3, 8), chars);
+  EXPECT_EQ(Range(0, 0), glyphs);
+}
+
 }  // namespace gfx