Bug 623711 - erratic font selection with multiple @font-face rules and bad font resources; r=jdaggett

gfx/thebes/gfxFont.cpp

@@ -2040,21 +2040,26 @@ gfxFontGroup::FindPlatformFont(const nsAString& aName,
     PRBool needsBold;
     gfxFontEntry *fe = nsnull;
 
-    // first, look up in the user font set
+    // First, look up in the user font set...
+    // If the fontSet matches the family, we must not look for a platform
+    // font of the same name, even if we fail to actually get a fontEntry
+    // here; we'll fall back to the next name in the CSS font-family list.
+    PRBool foundFamily = PR_FALSE;
     gfxUserFontSet *fs = fontGroup->GetUserFontSet();
     if (fs) {
-        // if the fontSet matches the family, but the font has not yet finished
+        // If the fontSet matches the family, but the font has not yet finished
         // loading (nor has its load timeout fired), the fontGroup should wait
-        // for the download, and not actually draw its text yet
+        // for the download, and not actually draw its text yet.
         PRBool waitForUserFont = PR_FALSE;
-        fe = fs->FindFontEntry(aName, *fontStyle, needsBold, waitForUserFont);
+        fe = fs->FindFontEntry(aName, *fontStyle, foundFamily,
+                               needsBold, waitForUserFont);
         if (!fe && waitForUserFont) {
             fontGroup->mSkipDrawing = PR_TRUE;
         }
     }
 
-    // nothing in the user font set ==> check system fonts
-    if (!fe) {
+    // Not known in the user font set ==> check system fonts
+    if (!foundFamily) {
         fe = gfxPlatformFontList::PlatformFontList()->
             FindFontForFamily(aName, fontStyle, needsBold);
     }
@@ -2247,22 +2252,24 @@ gfxFontGroup::ForEachFontInternal(const nsAString& aFamilies,
             if (aResolveFontName) {
                 ResolveData data(fc, gf, closure);
                 PRBool aborted = PR_FALSE, needsBold;
-                nsresult rv;
+                nsresult rv = NS_OK;
+                PRBool foundFamily = PR_FALSE;
                 PRBool waitForUserFont = PR_FALSE;
                 if (mUserFontSet &&
-                    mUserFontSet->FindFontEntry(family, mStyle, needsBold,
-                                                waitForUserFont))
+                    mUserFontSet->FindFontEntry(family, mStyle, foundFamily,
+                                                needsBold, waitForUserFont))
                 {
                     gfxFontGroup::FontResolverProc(family, &data);
-                    rv = NS_OK;
                 } else {
                     if (waitForUserFont) {
                         mSkipDrawing = PR_TRUE;
                     }
-                    gfxPlatform *pf = gfxPlatform::GetPlatform();
-                    rv = pf->ResolveFontName(family,
-                                             gfxFontGroup::FontResolverProc,
-                                             &data, aborted);
+                    if (!foundFamily) {
+                        gfxPlatform *pf = gfxPlatform::GetPlatform();
+                        rv = pf->ResolveFontName(family,
+                                                 gfxFontGroup::FontResolverProc,
+                                                 &data, aborted);
+                    }
                 }
                 if (NS_FAILED(rv) || aborted)
                     return PR_FALSE;

gfx/thebes/gfxPangoFonts.cpp

@@ -1184,7 +1184,7 @@ private:
 static const nsTArray< nsCountedRef<FcPattern> >*
 FindFontPatterns(gfxUserFontSet *mUserFontSet,
                 const nsACString &aFamily, PRUint8 aStyle, PRUint16 aWeight,
-                PRBool& aWaitForUserFont)
+                PRBool& aFoundFamily, PRBool& aWaitForUserFont)
 {
     // Convert to UTF16
     NS_ConvertUTF8toUTF16 utf16Family(aFamily);
@@ -1199,15 +1199,15 @@ FindFontPatterns(gfxUserFontSet *mUserFontSet,
     style.weight = aWeight;
 
     gfxUserFcFontEntry *fontEntry = static_cast<gfxUserFcFontEntry*>
-        (mUserFontSet->FindFontEntry(utf16Family, style, needsBold,
-                                     aWaitForUserFont));
+        (mUserFontSet->FindFontEntry(utf16Family, style, aFoundFamily,
+                                     needsBold, aWaitForUserFont));
 
     // Accept synthetic oblique for italic and oblique.
     if (!fontEntry && aStyle != FONT_STYLE_NORMAL) {
         style.style = FONT_STYLE_NORMAL;
         fontEntry = static_cast<gfxUserFcFontEntry*>
-            (mUserFontSet->FindFontEntry(utf16Family, style, needsBold,
-                                         aWaitForUserFont));
+            (mUserFontSet->FindFontEntry(utf16Family, style, aFoundFamily,
+                                         needsBold, aWaitForUserFont));
     }
 
     if (!fontEntry)
@@ -1365,13 +1365,15 @@ gfxFcFontSet::SortPreferredFonts(PRBool &aWaitForUserFont)
                 PRUint16 thebesWeight =
                     gfxFontconfigUtils::GetThebesWeight(mSortPattern);
 
-                PRBool waitForUserFont;
+                PRBool foundFamily, waitForUserFont;
                 familyFonts = FindFontPatterns(mUserFontSet, cssFamily,
                                                thebesStyle, thebesWeight,
-                                               waitForUserFont);
+                                               foundFamily, waitForUserFont);
                 if (waitForUserFont) {
                     aWaitForUserFont = PR_TRUE;
                 }
+                NS_ASSERTION(foundFamily,
+                             "expected to find a user font, but it's missing!");
             }
         }
 

gfx/thebes/gfxUserFontSet.cpp

@@ -163,6 +163,7 @@ gfxUserFontSet::AddFontFace(const nsAString& aFamilyName,
 gfxFontEntry*
 gfxUserFontSet::FindFontEntry(const nsAString& aName, 
                               const gfxFontStyle& aFontStyle, 
+                              PRBool& aFoundFamily,
                               PRBool& aNeedsBold,
                               PRBool& aWaitForUserFont)
 {
@@ -171,9 +172,11 @@ gfxUserFontSet::FindFontEntry(const nsAString& aName,
 
     // no user font defined for this name
     if (!family) {
+        aFoundFamily = PR_FALSE;
         return nsnull;
     }
 
+    aFoundFamily = PR_TRUE;
     gfxFontEntry* fe = family->FindFontForStyle(aFontStyle, aNeedsBold);
 
     // if not a proxy, font has already been loaded
@@ -682,25 +685,14 @@ gfxUserFontSet::LoadNext(gfxProxyFontEntry *aProxyEntry)
         aProxyEntry->mSrcIndex++;
     }
 
-    // all src's failed, remove this face
+    // all src's failed; mark this entry as unusable (so fallback will occur)
     LOG(("userfonts (%p) failed all src for (%s)\n", 
-               this, NS_ConvertUTF16toUTF8(aProxyEntry->mFamily->Name()).get()));            
-
-    gfxMixedFontFamily *family = static_cast<gfxMixedFontFamily*>(aProxyEntry->mFamily);
-
-    family->RemoveFontEntry(aProxyEntry);
-
-    // no more faces?  remove the entire family
-    if (family->mAvailableFonts.Length() == 0) {
-        LOG(("userfonts (%p) failed all faces, remove family (%s)\n", 
-             this, NS_ConvertUTF16toUTF8(family->Name()).get()));            
-        RemoveFamily(family->Name());
-    }
+        this, NS_ConvertUTF16toUTF8(aProxyEntry->mFamily->Name()).get()));            
+    aProxyEntry->mLoadingState = gfxProxyFontEntry::LOADING_FAILED;
 
     return STATUS_END_OF_LIST;
 }
 
-
 void
 gfxUserFontSet::IncrementGeneration()
 {

gfx/thebes/gfxUserFontSet.h

@@ -188,8 +188,9 @@ public:
     }
 
     // lookup a font entry for a given style, returns null if not loaded
-    gfxFontEntry *FindFontEntry(const nsAString& aName, 
-                                const gfxFontStyle& aFontStyle, 
+    gfxFontEntry *FindFontEntry(const nsAString& aName,
+                                const gfxFontStyle& aFontStyle,
+                                PRBool& aFoundFamily,
                                 PRBool& aNeedsBold,
                                 PRBool& aWaitForUserFont);
                                 
@@ -256,8 +257,9 @@ public:
         LOADING_STARTED,     // loading has started; hide fallback font
         LOADING_ALMOST_DONE, // timeout happened but we're nearly done,
                              // so keep hiding fallback font
-        LOADING_SLOWLY       // timeout happened and we're not nearly done,
+        LOADING_SLOWLY,      // timeout happened and we're not nearly done,
                              // so use the fallback font
+        LOADING_FAILED       // failed to load any source: use fallback
     };
     LoadingState             mLoadingState;