RenderText: Try fallback fonts of the Uniscribe font while shaping

ui/gfx/render_text_win.cc

Index: ui/gfx/render_text_win.cc
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index e76e6db39e8a55af31a8b1a0a1e0ad349bd86953..4a9848eaeb6eab6e2f3df0b63133ae4567f154ca 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -1032,8 +1032,10 @@ void RenderTextWin::LayoutTextRun(internal::TextRun* run) {
 
   // Try finding a fallback font using a meta file.
   // TODO(msw|asvitkine): Support RenderText's font_list()?
+  Font uniscribe_font;
   if (GetUniscribeFallbackFont(original_font, run_text, run_length,
-                               &current_font)) {
+                               &uniscribe_font)) {
+    current_font = uniscribe_font;
     missing_count = CountCharsWithMissingGlyphs(run,
         ShapeTextRunWithFont(run, current_font));
     if (missing_count == 0) {
@@ -1050,8 +1052,25 @@ void RenderTextWin::LayoutTextRun(internal::TextRun* run) {
   std::vector<std::string> fonts =
       GetFallbackFontFamilies(original_font.GetFontName());
   for (size_t i = 1; i < fonts.size(); ++i) {
+    current_font = Font(fonts[i], original_font.GetFontSize());

[msw, 2014/10/20 20:12:03]

The entire defect may just be that |current_font| was not updated in this loop.
Any good shaping would then use the stale value for
|successful_substitute_fonts_| or |best_partial_font|.

Can you try just adding this line and updating the ShapeTextRunWithFont call
immediately below, without trying additional fallback fonts of the Uniscribe
fallback font? (remove the loop in lines 1068-1082 below and potentially revert
the change above too). See if that fixes the issue... do you need me to test?

[Alexei Svitkine (slow), 2014/10/20 20:21:20]

From my recollection of the original motivation for the previous logic, I
suspect it won't be enough - though happy to hear otherwise.

IIRC, issue is that the good font is from the fallback list of the uniscribe
font, not of the original font. So updating current_font isn't enough I think.

[Alexei Svitkine (slow), 2014/10/20 20:25:06]

Although yeah, it looks like the code without the above line would just result
in font linking not working ... so maybe this will work too. I guess the
question is whether it will cover all the cases or not. I'd prefer we keep logic
that matches the original working behavior for the patch that's merged to stable
- and we can investigate if the second linked font query is needed separately on
TOT.

     missing_count = CountCharsWithMissingGlyphs(run,
-        ShapeTextRunWithFont(run, Font(fonts[i], original_font.GetFontSize())));
+        ShapeTextRunWithFont(run, current_font));
+    if (missing_count == 0) {
+      successful_substitute_fonts_[original_font.GetFontName()] = current_font;
+      return;
+    }
+    if (missing_count < best_partial_font_missing_char_count) {
+      best_partial_font_missing_char_count = missing_count;
+      best_partial_font = current_font;
+    }
+  }
+
+  // Try fonts in the fallback list of the Uniscribe font.
+  fonts = GetFallbackFontFamilies(uniscribe_font.GetFontName());
+  for (size_t i = 1; i < fonts.size(); ++i) {
+    current_font = Font(fonts[i], original_font.GetFontSize());
+    missing_count = CountCharsWithMissingGlyphs(run,
+      ShapeTextRunWithFont(run, current_font));

[Alexei Svitkine (slow), 2014/10/20 20:14:04]

Nit: Indent 2 more.

     if (missing_count == 0) {
       successful_substitute_fonts_[original_font.GetFontName()] = current_font;
       return;