Remove unnecessary font loading when loading font fallbacks.

third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp

Index: third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp b/third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp
index 54f6d8f8f8625536725a81b927d07997942a8a38..3d432c45afef97150e303eaf8734c614bcc5e3a2 100644
--- a/third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp
+++ b/third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp
@@ -68,11 +68,15 @@ static inline bool isFontPresent(const UChar* fontName, SkFontMgr* fontManager)
     return matchesRequestedFamily;
 }
 
+struct FontMapping {
+    const UChar* familyName;
+    bool attemptedLoad;

[kojii, 2016/08/04 06:02:03]

How about adding:
  const UChar** candidateFamilyNames;
so that we only need to loop once, but still can lazily resolve the first
available font?

[Ilya Kulshin, 2016/08/06 03:20:50]

Done.

+};
 // A simple mapping from UScriptCode to family name. This is a sparse array,
 // which works well since the range of UScriptCode values is small.
-typedef const UChar* ScriptToFontMap[USCRIPT_CODE_LIMIT];
+typedef FontMapping ScriptToFontMap[USCRIPT_CODE_LIMIT];
 
-void initializeScriptMonospaceFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontManager)
+void initializeScriptMonospaceFontMapping(UScriptCode script, FontMapping& scriptFontMapping, SkFontMgr* fontManager)
 {
     struct FontMap {
         UScriptCode script;
@@ -84,25 +88,17 @@ void initializeScriptMonospaceFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr*
         { USCRIPT_ARABIC, L"courier new" },
     };
 
-    for (size_t i = 0; i < WTF_ARRAY_LENGTH(fontMap); ++i)
-        scriptFontMap[fontMap[i].script] = fontMap[i].family;
+    for (const auto& fontFamily : fontMap) {
+        if (fontFamily.script != script)
+            continue;
+        scriptFontMapping.familyName = fontFamily.family;
+        break;
+    }
+    scriptFontMapping.attemptedLoad = true;
 }
 
-void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontManager)
+void initializeScriptFontMapping(UScriptCode script, FontMapping& scriptFontMapping, SkFontMgr* fontManager)
 {
-    struct FontMap {
-        UScriptCode script;
-        const UChar* family;
-    };
-
-    static const FontMap fontMap[] = {
-        { USCRIPT_LATIN, L"Times New Roman" },
-        { USCRIPT_GREEK, L"Times New Roman" },
-        { USCRIPT_CYRILLIC, L"Times New Roman" },
-        // For USCRIPT_COMMON, we map blocks to scripts when
-        // that makes sense.
-    };
-
     struct ScriptToFontFamilies {
         UScriptCode script;
         const UChar** families;
@@ -131,6 +127,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
     static const UChar* copticFonts[] = { L"Segoe UI Symbol", 0 };
     static const UChar* cuneiformFonts[] = { L"Segoe UI Historic", 0 };
     static const UChar* cypriotFonts[] = { L"Segoe UI Historic", 0 };
+    static const UChar* cyrillicFonts[] = { L"Times New Roman", 0 };
     static const UChar* deseretFonts[] = { L"Segoe UI Symbol", 0 };
     static const UChar* devanagariFonts[] = { L"Nirmala UI", L"Mangal", 0 };
     static const UChar* egyptianHieroglyphsFonts[] = { L"Segoe UI Historic",
@@ -143,6 +140,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         L"Segoe UI Symbol", 0 };
     static const UChar* gothicFonts[] = { L"Segoe UI Historic",
         L"Segoe UI Symbol", 0 };
+    static const UChar* greekFonts[] = { L"Times New Roman", 0 };
     static const UChar* gujaratiFonts[] = { L"Nirmala UI", L"Shruti", 0 };
     static const UChar* gurmukhiFonts[] = { L"Nirmala UI", L"Raavi", 0 };
     static const UChar* hangulFonts[] = { L"Malgun Gothic", L"Gulim", 0 };
@@ -163,6 +161,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         L"Khmer OS", L"MoolBoran", L"DaunPenh", 0 };
     static const UChar* laoFonts[] = { L"Leelawadee UI", L"Lao UI",
         L"DokChampa", L"Saysettha OT", L"Phetsarath OT", L"Code2000", 0 };
+    static const UChar* latinFonts[] = { L"Times New Roman", 0 };
     static const UChar* lisuFonts[] = { L"Segoe UI", 0 };
     static const UChar* lycianFonts[] = { L"Segoe UI Historic", 0 };
     static const UChar* lydianFonts[] = { L"Segoe UI Historic", 0 };
@@ -226,6 +225,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         { USCRIPT_COPTIC, copticFonts },
         { USCRIPT_CUNEIFORM, cuneiformFonts },
         { USCRIPT_CYPRIOT, cypriotFonts },
+        { USCRIPT_CYRILLIC, cyrillicFonts },
         { USCRIPT_DESERET, deseretFonts },
         { USCRIPT_DEVANAGARI, devanagariFonts },
         { USCRIPT_EGYPTIAN_HIEROGLYPHS, egyptianHieroglyphsFonts },
@@ -233,6 +233,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         { USCRIPT_GEORGIAN, georgianFonts },
         { USCRIPT_GLAGOLITIC, glagoliticFonts },
         { USCRIPT_GOTHIC, gothicFonts },
+        { USCRIPT_GREEK, greekFonts },
         { USCRIPT_GUJARATI, gujaratiFonts },
         { USCRIPT_GURMUKHI, gurmukhiFonts },
         { USCRIPT_HANGUL, hangulFonts },
@@ -248,6 +249,7 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         { USCRIPT_KHAROSHTHI, kharoshthiFonts },
         { USCRIPT_KHMER, khmerFonts },
         { USCRIPT_LAO, laoFonts },
+        { USCRIPT_LATIN, latinFonts },
         { USCRIPT_LISU, lisuFonts },
         { USCRIPT_LYCIAN, lycianFonts },
         { USCRIPT_LYDIAN, lydianFonts },
@@ -285,30 +287,31 @@ void initializeScriptFontMap(ScriptToFontMap& scriptFontMap, SkFontMgr* fontMana
         { USCRIPT_YI, yiFonts }
     };
 
-    for (size_t i = 0; i < WTF_ARRAY_LENGTH(fontMap); ++i)
-        scriptFontMap[fontMap[i].script] = fontMap[i].family;
-
-    // FIXME: Instead of scanning the hard-coded list, we have to
-    // use EnumFont* to 'inspect' fonts to pick up fonts covering scripts
-    // when it's possible (e.g. using OS/2 table). If we do that, this
-    // had better be pulled out of here.
-    for (size_t i = 0; i < WTF_ARRAY_LENGTH(scriptToFontFamilies); ++i) {
-        UScriptCode script = scriptToFontFamilies[i].script;
-        scriptFontMap[script] = 0;
-        const UChar** familyPtr = scriptToFontFamilies[i].families;
-        while (*familyPtr) {
+    for (const auto& fontFamily : scriptToFontFamilies) {
+        if (fontFamily.script != script)
+            continue;
+        for (const UChar** familyPtr = fontFamily.families; *familyPtr; ++familyPtr) {
             if (isFontPresent(*familyPtr, fontManager)) {
-                scriptFontMap[script] = *familyPtr;
+                scriptFontMapping.familyName = *familyPtr;
                 break;
             }
-            ++familyPtr;
         }
+        break;
     }
-
-    // Initialize the locale-dependent mapping from system locale.
-    UScriptCode hanScript = LayoutLocale::getSystem().scriptForHan();
-    if (const UChar* localeFamily = scriptFontMap[hanScript])
-        scriptFontMap[USCRIPT_HAN] = localeFamily;
+    if (script == USCRIPT_HAN) {
+        // Initialize the locale-dependent mapping from system locale.
+        UScriptCode hanScript = LayoutLocale::getSystem().scriptForHan();
+
+        // If the system specifies a different script to use for the Han locale,
+        // override the Han script mapping to use the font for that other script.
+        if (script != hanScript) {

[kojii, 2016/08/04 06:02:03]

With the CL a few days ago, scriptForHan() will never return USCRIPT_HAN, so you
can replace this with DCHECK(). There's some unknown I need to understand better
for Korean, but this is the current assumption.

[Ilya Kulshin, 2016/08/06 03:20:50]

Acknowledged.

+            FontMapping hanMapping = { 0 };
+            initializeScriptFontMapping(hanScript, hanMapping, fontManager);

[kojii, 2016/08/04 06:02:03]

Assuming this function is given "scriptFontMap" to resolve the loop issue
mentioned above, we need to do this only when
!scriptFontMap[hanScript].attemptedLoad.

But since this code runs only when !scriptFontMap[USCRIPT_HAN].attemptedLoad, I
can't imagine the case when !scriptFontMap[hanScript].attemptedLoad. Maybe
DCHECK should be fine.

[Ilya Kulshin, 2016/08/06 03:20:50]

Acknowledged.

+            if (hanMapping.familyName)
+                scriptFontMapping.familyName = hanMapping.familyName;
+        }
+    }
+    scriptFontMapping.attemptedLoad = true;
 }
 
 // There are a lot of characters in USCRIPT_COMMON that can be covered
@@ -437,18 +440,20 @@ const UChar* getFontFamilyForScript(UScriptCode script,
 {
     static ScriptToFontMap scriptFontMap;
     static ScriptToFontMap scriptMonospaceFontMap;
-    static bool initialized = false;
-    if (!initialized) {
-        initializeScriptFontMap(scriptFontMap, fontManager);
-        initializeScriptMonospaceFontMap(scriptMonospaceFontMap, fontManager);
-        initialized = true;
-    }
+
     if (script == USCRIPT_INVALID_CODE)
         return 0;
     ASSERT(script < USCRIPT_CODE_LIMIT);
-    if (generic == FontDescription::MonospaceFamily && scriptMonospaceFontMap[script])
-        return scriptMonospaceFontMap[script];
-    return scriptFontMap[script];
+    if (generic == FontDescription::MonospaceFamily) {
+        if (!scriptMonospaceFontMap[script].attemptedLoad) {
+            initializeScriptMonospaceFontMapping(script, scriptMonospaceFontMap[script], fontManager);
+        }
+        if (scriptMonospaceFontMap[script].familyName)
+            return scriptMonospaceFontMap[script].familyName;
+    }
+    if (!scriptFontMap[script].attemptedLoad)
+        initializeScriptFontMapping(script, scriptFontMap[script], fontManager);
+    return scriptFontMap[script].familyName;
 }
 
 // FIXME: