Bug 1245811 - part 2 (based on patch by Andrew Comminos) - Let gfxFcPlatformFontList return multiple families for a given name once fontconfig substitutions have been applied. r=karlt a=ritu
authorJonathan Kew <jkew@mozilla.com>
Sat, 09 Apr 2016 11:09:08 +0100
changeset 310565 c9962404eb5dfc4c8fed45a1fc964072c1a783e6
parent 310564 976db6fcfed77d4ac4626ab8cbb6a25213232711
child 310566 7d39fe1e571c9bd656d66e05732ebd11419c5e77
push id9403
push userjkew@mozilla.com
push dateWed, 13 Apr 2016 20:48:31 +0000
treeherdermozilla-aurora@c9962404eb5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskarlt, ritu
bugs1245811
milestone47.0a2
Bug 1245811 - part 2 (based on patch by Andrew Comminos) - Let gfxFcPlatformFontList return multiple families for a given name once fontconfig substitutions have been applied. r=karlt a=ritu
gfx/thebes/gfxFcPlatformFontList.cpp
gfx/thebes/gfxFcPlatformFontList.h
--- a/gfx/thebes/gfxFcPlatformFontList.cpp
+++ b/gfx/thebes/gfxFcPlatformFontList.cpp
@@ -1205,17 +1205,17 @@ gfxFcPlatformFontList::FindAndAddFamilie
         isDeprecatedGeneric = true;
     }
 
     // fontconfig generics? use fontconfig to determine the family for lang
     if (isDeprecatedGeneric ||
         mozilla::FontFamilyName::Convert(familyName).IsGeneric()) {
         PrefFontList* prefFonts = FindGenericFamilies(familyName, language);
         if (prefFonts && !prefFonts->IsEmpty()) {
-            aOutput->AppendElement((*prefFonts)[0]);
+            aOutput->AppendElements(*prefFonts);
             return true;
         }
         return false;
     }
 
     // fontconfig allows conditional substitutions in such a way that it's
     // difficult to distinguish an explicit substitution from other suggested
     // choices. To sniff out explicit substitutions, compare the substitutions
@@ -1226,64 +1226,65 @@ gfxFcPlatformFontList::FindAndAddFamilie
     //
     //   serif ==> DejaVu Serif, ...
     //   Helvetica, serif ==> Helvetica, TeX Gyre Heros, Nimbus Sans L, DejaVu Serif
     //
     // In this case fontconfig is including Tex Gyre Heros and
     // Nimbus Sans L as alternatives for Helvetica.
 
     // Because the FcConfigSubstitute call is quite expensive, we cache the
-    // actual font family found via this process. So check the cache first:
+    // actual font families found via this process. So check the cache first:
     NS_ConvertUTF16toUTF8 familyToFind(familyName);
-    gfxFontFamily* cached = mFcSubstituteCache.GetWeak(familyToFind);
-    if (cached) {
-        aOutput->AppendElement(cached);
+    AutoTArray<gfxFontFamily*,10> cachedFamilies;
+    if (mFcSubstituteCache.Get(familyToFind, &cachedFamilies)) {
+        if (cachedFamilies.IsEmpty()) {
+            return false;
+        }
+        aOutput->AppendElements(cachedFamilies);
         return true;
     }
 
+    // It wasn't in the cache, so we need to ask fontconfig...
     const FcChar8* kSentinelName = ToFcChar8Ptr("-moz-sentinel");
     FcChar8* sentinelFirstFamily = nullptr;
     nsAutoRef<FcPattern> sentinelSubst(FcPatternCreate());
     FcPatternAddString(sentinelSubst, FC_FAMILY, kSentinelName);
     FcConfigSubstitute(nullptr, sentinelSubst, FcMatchPattern);
     FcPatternGetString(sentinelSubst, FC_FAMILY, 0, &sentinelFirstFamily);
 
     // substitutions for font, -moz-sentinel pattern
     nsAutoRef<FcPattern> fontWithSentinel(FcPatternCreate());
     FcPatternAddString(fontWithSentinel, FC_FAMILY,
                        ToFcChar8Ptr(familyToFind.get()));
     FcPatternAddString(fontWithSentinel, FC_FAMILY, kSentinelName);
     FcConfigSubstitute(nullptr, fontWithSentinel, FcMatchPattern);
 
-    // iterate through substitutions until hitting the sentinel
+    // Add all font family matches until reaching the sentinel.
     FcChar8* substName = nullptr;
     for (int i = 0;
          FcPatternGetString(fontWithSentinel, FC_FAMILY,
                             i, &substName) == FcResultMatch;
          i++)
     {
         NS_ConvertUTF8toUTF16 subst(ToCharPtr(substName));
         if (sentinelFirstFamily &&
             FcStrCmp(substName, sentinelFirstFamily) == 0) {
             break;
         }
-        // We can't use gfxPlatformFontList::FindFamily() here, even though
-        // we only expect a single result, because that would call back into
-        // this method, resulting in infinite recursion.
-        AutoTArray<gfxFontFamily*,1> foundFamilies;
-        if (gfxPlatformFontList::FindAndAddFamilies(subst, &foundFamilies)) {
-            // We've figured out what family the given name maps to, after any
-            // fontconfig subsitutions. Cache it to speed up future lookups.
-            mFcSubstituteCache.Put(familyToFind, foundFamilies[0]);
-            aOutput->AppendElement(foundFamilies[0]);
-            return true;
-        }
+        gfxPlatformFontList::FindAndAddFamilies(subst, &cachedFamilies);
     }
 
-    return false;
+    // Cache the resulting list, so we don't have to do this again.
+    mFcSubstituteCache.Put(familyToFind, cachedFamilies);
+
+    if (cachedFamilies.IsEmpty()) {
+        return false;
+    }
+    aOutput->AppendElements(cachedFamilies);
+    return true;
 }
 
 bool
 gfxFcPlatformFontList::GetStandardFamilyName(const nsAString& aFontName,
                                              nsAString& aFamilyName)
 {
     aFamilyName.Truncate();
 
--- a/gfx/thebes/gfxFcPlatformFontList.h
+++ b/gfx/thebes/gfxFcPlatformFontList.h
@@ -276,18 +276,24 @@ protected:
                     nsCountedRef<FcPattern>,
                     FcPattern*> mLocalNames;
 
     // caching generic/lang ==> font family list
     nsBaseHashtable<nsCStringHashKey,
                     nsAutoPtr<PrefFontList>,
                     PrefFontList*> mGenericMappings;
 
-    // caching family lookups as found by FindFamily after resolving substitutions
-    nsRefPtrHashtable<nsCStringHashKey, gfxFontFamily> mFcSubstituteCache;
+    // Caching family lookups as found by FindAndAddFamilies after resolving
+    // substitutions. The gfxFontFamily objects cached here are owned by the
+    // gfxFcPlatformFontList via its mFamilies table; note that if the main
+    // font list is rebuilt (e.g. due to a fontconfig configuration change),
+    // these pointers will be invalidated. InitFontList() flushes the cache
+    // in this case.
+    nsDataHashtable<nsCStringHashKey,
+                    nsTArray<gfxFontFamily*>> mFcSubstituteCache;
 
     nsCOMPtr<nsITimer> mCheckFontUpdatesTimer;
     nsCountedRef<FcConfig> mLastConfig;
 
     // By default, font prefs under Linux are set to simply lookup
     // via fontconfig the appropriate font for serif/sans-serif/monospace.
     // Rather than check each time a font pref is used, check them all at startup
     // and set a boolean to flag the case that non-default user font prefs exist