Fix incorrect usage of wcslen on a StringPiece

The call to wcslen on a StringPiece is incorrect. There is no guarantee that
the string is NUL terminated.

As stated on crbug/624905:
 "Problem is that gfx::RenderTextHarfBuzz::ShapeRun is asking for fallback for
  an empty string but claims it's of length 1. This confuses DirectWrite and it
  crashes. Can easily work around the crash by checking for empty string, but
  still need to investigate why gfx::RenderTextHarfBuzz::ShapeRun is getting
  confused."

The reason of the code was to avoid a broken case with ShapeRuns.
The empty string is no longer accepted in any GetFallbackFont(...)

Bug: 624905

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

ui/gfx/font_fallback_win.cc

@@ -464,11 +464,16 @@ bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
   // renderer should instead use the font proxy.
   DCHECK(base::MessageLoopCurrentForUI::IsSet());
 
+  // The text passed must be at least length 1.
+  if (text.empty())
+    return false;
+
   // 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.data()), text.length());
-  size_t text_length = std::min(wcslen(text.data()), text.length());
+  constexpr base::char16 kNulCharacter = '\0';
+  if (text.find(kNulCharacter) != base::StringPiece16::npos)
+    return false;
 
   Microsoft::WRL::ComPtr<IDWriteFactory> factory;
   gfx::win::CreateDWriteFactory(factory.GetAddressOf());
@@ -500,7 +505,7 @@ bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
       base::i18n::IsRTL() ? DWRITE_READING_DIRECTION_RIGHT_TO_LEFT
                           : DWRITE_READING_DIRECTION_LEFT_TO_RIGHT;
   if (FAILED(gfx::win::TextAnalysisSource::Create(
-          &text_analysis, base::string16(text.data(), text_length),
+          &text_analysis, base::string16(text.data(), text.length()),
           locale.c_str(), number_substitution.Get(), reading_direction))) {
     return false;
   }
@@ -509,7 +514,7 @@ bool GetFallbackFont(const Font& font, base::StringPiece16 text, Font* result) {
   if (font.GetStyle() & Font::ITALIC)
     font_style = DWRITE_FONT_STYLE_ITALIC;
   if (FAILED(fallback->MapCharacters(
-          text_analysis.Get(), 0, text_length, nullptr, original_name.c_str(),
+          text_analysis.Get(), 0, text.length(), nullptr, original_name.c_str(),
           static_cast<DWRITE_FONT_WEIGHT>(font.GetWeight()), font_style,
           DWRITE_FONT_STRETCH_NORMAL, &mapped_length, &mapped_font, &scale))) {
     return false;

ui/gfx/font_fallback_win_unittest.cc

@@ -200,18 +200,40 @@ TEST_F(FontFallbackWinTest, ParseFontFamilyString) {
 TEST_F(FontFallbackWinTest, EmptyStringUniscribeFallback) {
   Font base_font;
   Font fallback_font;
-  bool result =
-      internal::GetUniscribeFallbackFont(base_font, L"", &fallback_font);
+  bool result = internal::GetUniscribeFallbackFont(
+      base_font, base::StringPiece16(), &fallback_font);
   EXPECT_FALSE(result);
 }
 
 TEST_F(FontFallbackWinTest, EmptyStringFallback) {
   Font base_font;
   Font fallback_font;
-  bool result = GetFallbackFont(base_font, L"", &fallback_font);
+  bool result =
+      GetFallbackFont(base_font, base::StringPiece16(), &fallback_font);
   EXPECT_FALSE(result);
 }
 
+TEST_F(FontFallbackWinTest, NulTerminatedStringPiece) {
+  Font base_font;
+  Font fallback_font;
+  // Multiple ending NUL characters.
+  const wchar_t kTest1[] = {0x0540, 0x0541, 0, 0, 0};
+  EXPECT_FALSE(GetFallbackFont(base_font,
+                               base::StringPiece16(kTest1, ARRAYSIZE(kTest1)),
+                               &fallback_font));
+  // No ending NUL character.
+  const wchar_t kTest2[] = {0x0540, 0x0541};
+  EXPECT_TRUE(GetFallbackFont(base_font,
+                              base::StringPiece16(kTest2, ARRAYSIZE(kTest2)),
+                              &fallback_font));
+
+  // NUL only characters.
+  const wchar_t kTest3[] = {0, 0, 0};
+  EXPECT_FALSE(GetFallbackFont(base_font,
+                               base::StringPiece16(kTest3, ARRAYSIZE(kTest3)),
+                               &fallback_font));
+}
+
 // This test ensures the font fallback work correctly. It will ensures that
 //   1) The script supports the text
 //   2) The input font does not already support the text