Change FallbackFont signature to use StringPiece16

This CL is changing the FontFallback signature to take an
StringPiece instead of a raw pointer and a length.

The CL doesn't include behavior changes.

R=asvitkine@chromium.org
Bug: 944227

Change-Id: I1302e5dd94ce9cf111ac8fb70b55f32d11aafbe3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594344
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658582}

ui/gfx/font_fallback.h

@@ -25,8 +25,7 @@ GFX_EXPORT std::vector<Font> GetFallbackFonts(const Font& font);
 // initial |font|. Returns the resulting font via out param |result|. Returns
 // |true| if a fallback font was found.
 bool GFX_EXPORT GetFallbackFont(const Font& font,
-                                const base::char16* text,
-                                int text_length,
+                                base::StringPiece16 text,
                                 Font* result);
 
 #endif  // ANDROID || OS_MACOSX || OS_WIN

ui/gfx/font_fallback_android.cc

@@ -23,13 +23,10 @@ std::vector<Font> GetFallbackFonts(const Font& font) {
   return std::vector<Font>();
 }
 
-bool GetFallbackFont(const Font& font,
-                     const base::char16* text,
-                     int text_length,
-                     Font* result) {
+bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
   TRACE_EVENT0("fonts", "gfx::GetFallbackFont");
 
-  if (text_length <= 0)
+  if (text.empty())
     return false;
 
   sk_sp<SkFontMgr> font_mgr(SkFontMgr::RefDefault());
@@ -47,27 +44,27 @@ bool GetFallbackFont(const Font& font,
 
   std::set<SkFontID> tested_typeface;
   SkString skia_family_name;
-  size_t fewest_missing_glyphs = text_length + 1;
+  size_t fewest_missing_glyphs = text.length() + 1;
 
-  int offset = 0;
-  while (offset < text_length) {
+  size_t offset = 0;
+  while (offset < text.length()) {
     UChar32 code_point;
-    U16_NEXT(text, offset, text_length, code_point);
+    U16_NEXT(text.data(), offset, text.length(), code_point);
 
     sk_sp<SkTypeface> typeface(font_mgr->matchFamilyStyleCharacter(
         font.GetFontName().c_str(), skia_style,
         locale.empty() ? nullptr : bcp47_locales, locale.empty() ? 0 : 1,
-        text[offset]));
+        code_point));
     // If the typeface is not found or was already tested, skip it.
     if (!typeface || !tested_typeface.insert(typeface->uniqueID()).second)
       continue;
 
     // Validate that every character has a known glyph in the font.
     size_t missing_glyphs = 0;
-    int i = 0;
-    while (i < text_length) {
+    size_t i = 0;
+    while (i < text.length()) {
       UChar32 c;
-      U16_NEXT(text, i, text_length, c);
+      U16_NEXT(text.data(), i, text.length(), c);
       if (typeface->unicharToGlyph(c) == 0)
         ++missing_glyphs;
     }

ui/gfx/font_fallback_android_unittest.cc

@@ -23,8 +23,8 @@ static const wchar_t* kFallbackFontTests[] = {
 TEST(FontFallbackAndroidTest, EmptyStringFallback) {
   Font base_font;
   Font fallback_font;
-  bool result = GetFallbackFont(base_font, base::ASCIIToUTF16("").c_str(), 0,
-                                &fallback_font);
+  bool result =
+      GetFallbackFont(base_font, base::StringPiece16(), &fallback_font);
   EXPECT_FALSE(result);
 }
 
@@ -34,8 +34,7 @@ TEST(FontFallbackAndroidTest, FontFallback) {
     Font fallback_font;
     base::string16 text = base::WideToUTF16(test);
 
-    if (!GetFallbackFont(base_font, text.c_str(), text.size(),
-                         &fallback_font)) {
+    if (!GetFallbackFont(base_font, text, &fallback_font)) {
       ADD_FAILURE() << "Font fallback failed: '" << text << "'";
     }
   }

ui/gfx/font_fallback_mac.mm

@@ -72,15 +72,12 @@ std::vector<Font> GetFallbackFonts(const Font& font) {
   return fallback_fonts;
 }
 
-bool GetFallbackFont(const Font& font,
-                     const base::char16* text,
-                     int text_length,
-                     Font* result) {
-  base::ScopedCFTypeRef<CFStringRef> cf_string(
-      CFStringCreateWithCharacters(kCFAllocatorDefault, text, text_length));
+bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
+  base::ScopedCFTypeRef<CFStringRef> cf_string(CFStringCreateWithCharacters(
+      kCFAllocatorDefault, text.data(), text.length()));
   CTFontRef ct_font = base::mac::NSToCFCast(font.GetNativeFont());
   base::ScopedCFTypeRef<CTFontRef> ct_result(
-      CTFontCreateForString(ct_font, cf_string, {0, text_length}));
+      CTFontCreateForString(ct_font, cf_string, {0, text.length()}));
   if (FontsEqual(ct_font, ct_result))
     return false;
 

ui/gfx/font_fallback_mac_unittest.cc

@@ -30,10 +30,10 @@ TEST(FontFallbackMacTest, GetFallbackFont) {
   const base::string16 emoji = base::UTF8ToUTF16("😋");
 
   gfx::Font fallback;
-  EXPECT_FALSE(GetFallbackFont(arial, ascii.data(), ascii.size(), &fallback));
-  EXPECT_TRUE(GetFallbackFont(arial, hebrew.data(), hebrew.size(), &fallback));
+  EXPECT_FALSE(GetFallbackFont(arial, ascii, &fallback));
+  EXPECT_TRUE(GetFallbackFont(arial, hebrew, &fallback));
   EXPECT_EQ("Lucida Grande", fallback.GetFontName());
-  EXPECT_TRUE(GetFallbackFont(arial, emoji.data(), emoji.size(), &fallback));
+  EXPECT_TRUE(GetFallbackFont(arial, emoji, &fallback));
   EXPECT_EQ("Apple Color Emoji", fallback.GetFontName());
 }
 

ui/gfx/font_fallback_win.cc

@@ -336,13 +336,12 @@ namespace internal {
 //   EMR_DELETEOBJECT ID:2
 //   EMR_EOF
 bool GetUniscribeFallbackFont(const Font& font,
-                              const wchar_t* text,
-                              int text_length,
+                              base::StringPiece16 text,
                               Font* result) {
   static HDC hdc = CreateCompatibleDC(NULL);
 
   // The length passed to |ScriptStringAnalyse| must be at least 1.
-  if (text_length <= 0)
+  if (text.empty())
     return false;
 
   // Use a meta file to intercept the fallback font chosen by Uniscribe.
@@ -368,7 +367,7 @@ bool GetUniscribeFallbackFont(const Font& font,
   // Run the script analysis.
   SCRIPT_STRING_ANALYSIS script_analysis;
   HRESULT hresult =
-      ScriptStringAnalyse(meta_file_dc, text, text_length, 0, -1,
+      ScriptStringAnalyse(meta_file_dc, text.data(), text.length(), 0, -1,
                           SSA_METAFILE | SSA_FALLBACK | SSA_GLYPHS | SSA_LINK,
                           0, NULL, NULL, NULL, NULL, NULL, &script_analysis);
 
@@ -378,7 +377,7 @@ bool GetUniscribeFallbackFont(const Font& font,
   }
 
   MetaFileEnumState state;
-  state.expected_text = base::StringPiece16(text, text_length);
+  state.expected_text = text;
 
   HENHMETAFILE meta_file = CloseEnhMetaFile(meta_file_dc);
   if (SUCCEEDED(hresult)) {
@@ -457,10 +456,7 @@ std::vector<Font> GetFallbackFonts(const Font& font) {
   return *font_link->GetLinkedFonts(Font(font_family, 10));
 }
 
-bool GetFallbackFont(const Font& font,
-                     const base::char16* text,
-                     int text_length,
-                     Font* result) {
+bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
   TRACE_EVENT0("fonts", "gfx::GetFallbackFont");
   // Creating a DirectWrite font fallback can be expensive. It's ok in the
   // browser process because we can use the shared system fallback, but in the
@@ -471,8 +467,8 @@ bool GetFallbackFont(const Font& font,
   // Check that we have at least as much text as was claimed. If we have less
   // text than expected then DirectWrite will become confused and crash. This
   // shouldn't happen, but crbug.com/624905 shows that it happens sometimes.
-  DCHECK_GE(wcslen(text), static_cast<size_t>(text_length));
-  text_length = std::min(wcslen(text), static_cast<size_t>(text_length));
+  DCHECK_GE(wcslen(text.data()), text.length());
+  size_t text_length = std::min(wcslen(text.data()), text.length());
 
   Microsoft::WRL::ComPtr<IDWriteFactory> factory;
   gfx::win::CreateDWriteFactory(factory.GetAddressOf());
@@ -480,7 +476,7 @@ bool GetFallbackFont(const Font& font,
   factory.CopyTo(factory2.GetAddressOf());
   if (!factory2) {
     // IDWriteFactory2 is not available before Win8.1
-    return internal::GetUniscribeFallbackFont(font, text, text_length, result);
+    return internal::GetUniscribeFallbackFont(font, text, result);
   }
 
   Microsoft::WRL::ComPtr<IDWriteFontFallback> fallback;
@@ -504,8 +500,8 @@ bool GetFallbackFont(const Font& font,
       base::i18n::IsRTL() ? DWRITE_READING_DIRECTION_RIGHT_TO_LEFT
                           : DWRITE_READING_DIRECTION_LEFT_TO_RIGHT;
   if (FAILED(gfx::win::TextAnalysisSource::Create(
-          text_analysis.GetAddressOf(), text, locale.c_str(),
-          number_substitution.Get(), reading_direction))) {
+          &text_analysis, base::string16(text.data(), text_length),
+          locale.c_str(), number_substitution.Get(), reading_direction))) {
     return false;
   }
   base::string16 original_name = base::UTF8ToUTF16(font.GetFontName());

ui/gfx/font_fallback_win.h

@@ -25,18 +25,17 @@ namespace internal {
 // Sets |filename| and |font_name| respectively. If a field is not present
 // or could not be parsed, the corresponding parameter will be cleared.
 void GFX_EXPORT ParseFontLinkEntry(const std::string& entry,
-                                  std::string* filename,
-                                  std::string* font_name);
+                                   std::string* filename,
+                                   std::string* font_name);
 
 // Parses a font |family| in the format "FamilyFoo & FamilyBar (TrueType)".
 // Splits by '&' and strips off the trailing parenthesized expression.
 void GFX_EXPORT ParseFontFamilyString(const std::string& family,
-                                     std::vector<std::string>* font_names);
+                                      std::vector<std::string>* font_names);
 
 // Exposed for unittest.
 bool GFX_EXPORT GetUniscribeFallbackFont(const Font& font,
-                                         const wchar_t* text,
-                                         int text_length,
+                                         base::StringPiece16 text,
                                          Font* result);
 }  // namespace internal
 

ui/gfx/font_fallback_win_unittest.cc

@@ -91,11 +91,9 @@ class GetFallbackFontTest
  protected:
   bool GetFallbackFont(const Font& font, Font* result) {
     if (test_option_.fallback_font == FallbackFontFn::DEFAULT) {
-      return gfx::GetFallbackFont(font, test_case_.text.c_str(),
-                                  test_case_.text.size(), result);
+      return gfx::GetFallbackFont(font, test_case_.text, result);
     } else if (test_option_.fallback_font == FallbackFontFn::UNISCRIBE) {
-      return internal::GetUniscribeFallbackFont(font, test_case_.text.c_str(),
-                                                test_case_.text.size(), result);
+      return internal::GetUniscribeFallbackFont(font, test_case_.text, result);
     }
     return false;
   }
@@ -203,14 +201,14 @@ TEST_F(FontFallbackWinTest, EmptyStringUniscribeFallback) {
   Font base_font;
   Font fallback_font;
   bool result =
-      internal::GetUniscribeFallbackFont(base_font, L"", 0, &fallback_font);
+      internal::GetUniscribeFallbackFont(base_font, L"", &fallback_font);
   EXPECT_FALSE(result);
 }
 
 TEST_F(FontFallbackWinTest, EmptyStringFallback) {
   Font base_font;
   Font fallback_font;
-  bool result = GetFallbackFont(base_font, L"", 0, &fallback_font);
+  bool result = GetFallbackFont(base_font, L"", &fallback_font);
   EXPECT_FALSE(result);
 }
 

ui/gfx/render_text_harfbuzz.cc

@@ -1806,9 +1806,9 @@ void RenderTextHarfBuzz::ShapeRuns(
     SCOPED_UMA_HISTOGRAM_LONG_TIMER("RenderTextHarfBuzz.GetFallbackFontTime");
     TRACE_EVENT1("ui", "RenderTextHarfBuzz::GetFallbackFont", "script",
                  TRACE_STR_COPY(uscript_getShortName(font_params.script)));
-    const base::char16* run_text = &(text[runs.front()->range.start()]);
-    fallback_found = GetFallbackFont(
-        primary_font, run_text, runs.front()->range.length(), &fallback_font);
+    const base::StringPiece16 run_text(&text[runs.front()->range.start()],
+                                       runs.front()->range.length());
+    fallback_found = GetFallbackFont(primary_font, run_text, &fallback_font);
   }
   if (fallback_found) {
     preferred_fallback_family = fallback_font.GetFontName();

ui/gfx/render_text_unittest.cc

@@ -4129,6 +4129,11 @@ TEST_F(RenderTextTest, EmojiFlagGlyphCount) {
 #if defined(OS_MACOSX)
   // On Mac, the flags should be found, so two glyphs result.
   EXPECT_EQ(2u, run_list->runs()[0]->shape.glyph_count);
+#elif defined(OS_ANDROID)
+  // It seems that some versions of android support the flags. Older versions
+  // don't support it.
+  EXPECT_TRUE(2u == run_list->runs()[0]->shape.glyph_count ||
+              4u == run_list->runs()[0]->shape.glyph_count);
 #else
   // Elsewhere, the flags are not found, so each surrogate pair gets a
   // placeholder glyph. Eventually, all platforms should have 2 glyphs.