Bug 375757. Cache gfxFont objects. r=pavlov

gfx/thebes/public/gfxAtsuiFonts.h

@@ -50,6 +50,7 @@ class gfxAtsuiFontGroup;
 class gfxAtsuiFont : public gfxFont {
 public:
     gfxAtsuiFont(ATSUFontID fontID,
+                 const nsAString& name,
                  const gfxFontStyle *fontStyle);
     virtual ~gfxAtsuiFont();
 

gfx/thebes/public/gfxFont.h

@@ -44,12 +44,16 @@
 #include "nsString.h"
 #include "gfxPoint.h"
 #include "nsTArray.h"
+#include "nsTHashtable.h"
+#include "nsHashKeys.h"
 #include "gfxSkipChars.h"
 #include "gfxRect.h"
+#include "nsExpirationTracker.h"
 
 class gfxContext;
 class gfxTextRun;
 class nsIAtom;
+class gfxFont;
 class gfxFontGroup;
 typedef struct _cairo cairo_t;
 
@@ -74,6 +78,7 @@ struct THEBES_API gfxFontStyle {
                  const nsACString& aLangGroup,
                  float aSizeAdjust, PRPackedBool aSystemFont,
                  PRPackedBool aFamilyNameQuirks);
+    gfxFontStyle(const gfxFontStyle& aStyle);
 
     // The style of font (normal, italic, oblique)
     PRUint8 style : 7;
@@ -113,6 +118,12 @@ struct THEBES_API gfxFontStyle {
     // needs to be done.
     float sizeAdjust;
 
+    PLDHashNumber Hash() const {
+        return ((style + (systemFont << 7) + (familyNameQuirks << 8) +
+            (weight << 9)) + PRUint32(size*1000) + PRUint32(sizeAdjust*1000)) ^
+            HashString(langGroup);
+    }
+
     void ComputeWeightAndOffset(PRInt8 *outBaseWeight,
                                 PRInt8 *outOffset) const;
 
@@ -120,27 +131,144 @@ struct THEBES_API gfxFontStyle {
         return (size == other.size) &&
             (style == other.style) &&
             (systemFont == other.systemFont) &&
-            (variant == other.variant) &&
             (familyNameQuirks == other.familyNameQuirks) &&
             (weight == other.weight) &&
-            (decorations == other.decorations) &&
             (langGroup.Equals(other.langGroup)) &&
             (sizeAdjust == other.sizeAdjust);
     }
 };
 
+/**
+ * Font cache design:
+ * 
+ * The mFonts hashtable contains most fonts, indexed by (name, style).
+ * It does not add a reference to the fonts it contains.
+ * When a font's refcount decreases to zero, instead of deleting it we
+ * add it to our expiration tracker.
+ * The expiration tracker tracks fonts with zero refcount. After a certain
+ * period of time, such fonts expire and are deleted.
+ *
+ * We're using 3 generations with a ten-second generation interval, so
+ * zero-refcount fonts will be deleted 20-30 seconds after their refcount
+ * goes to zero, if timer events fire in a timely manner.
+ */
+class THEBES_API gfxFontCache : public nsExpirationTracker<gfxFont,3> {
+public:
+    enum { TIMEOUT_SECONDS = 1 }; // XXX change this to 10 later
+    gfxFontCache()
+        : nsExpirationTracker<gfxFont,3>(TIMEOUT_SECONDS*1000) { mFonts.Init(); }
+    ~gfxFontCache() {
+        // Expire everything that has a zero refcount, so we don't leak them.
+        AgeAllGenerations();
+        // All fonts should be gone. Otherwise we will crash releasing them
+        // later, since this cache no longer exists
+        NS_ASSERTION(mFonts.Count() == 0,
+                     "Fonts still alive while shutting down gfxFontCache");
+        // Note that we have to delete everything through the expiration
+        // tracker, since there might be fonts not in the hashtable but in
+        // the tracker.
+    }
 
+    /*
+     * Get the global gfxFontCache.  You must call Init() before
+     * calling this method --- the result will not be null.
+     */
+    static gfxFontCache* GetCache() {
+        return gGlobalCache;
+    }
+
+    static nsresult Init();
+    // It's OK to call this even if Init() has not been called.
+    static void Shutdown();
+
+    // Look up a font in the cache. Returns an addrefed pointer, or null
+    // if there's nothing matching in the cache
+    already_AddRefed<gfxFont> Lookup(const nsAString &aName,
+                                     const gfxFontStyle *aFontGroup);
+    // We created a new font (presumably because Lookup returned null);
+    // put it in the cache. The font's refcount should be nonzero. It is
+    // allowable to add a new font even if there is one already in the
+    // cache with the same key; we'll forget about the old one.
+    void AddNew(gfxFont *aFont);
+
+    // The font's refcount has gone to zero; give ownership of it to
+    // the cache. We delete it if it's not acquired again after a certain
+    // amount of time.
+    void NotifyReleased(gfxFont *aFont);
+
+    // This gets called when the timeout has expired on a zero-refcount
+    // font; we just delete it.
+    virtual void NotifyExpired(gfxFont *aFont);
+
+protected:
+    void DestroyFont(gfxFont *aFont);
+
+    static gfxFontCache *gGlobalCache;
+
+    struct Key {
+        const nsAString&    mString;
+        const gfxFontStyle* mStyle;
+        Key(const nsAString& aString, const gfxFontStyle* aStyle)
+            : mString(aString), mStyle(aStyle) {}
+    };
+
+    class HashEntry : public PLDHashEntryHdr {
+    public:
+        typedef const Key& KeyType;
+        typedef const Key* KeyTypePointer;
+
+        // When constructing a new entry in the hashtable, we'll leave this
+        // blank. The caller of Put() will fill this in.
+        HashEntry(KeyTypePointer aStr) : mFont(nsnull) { }
+        HashEntry(const HashEntry& toCopy) : mFont(toCopy.mFont) { }
+        ~HashEntry() { }
+
+        PRBool KeyEquals(const KeyTypePointer aKey) const;
+        static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
+        static PLDHashNumber HashKey(const KeyTypePointer aKey) {
+            return HashString(aKey->mString) ^ aKey->mStyle->Hash();
+        }
+        enum { ALLOW_MEMMOVE = PR_TRUE };
+
+        gfxFont* mFont;
+    };
+
+    nsTHashtable<HashEntry> mFonts;
+};
 
 /* a SPECIFIC single font family */
 class THEBES_API gfxFont {
-    THEBES_INLINE_DECL_REFCOUNTING(gfxFont)
+public:
+    nsrefcnt AddRef(void) {
+        NS_PRECONDITION(PRInt32(mRefCnt) >= 0, "illegal refcnt");
+        ++mRefCnt;
+        NS_LOG_ADDREF(this, mRefCnt, "gfxFont", sizeof(*this));
+        return mRefCnt;
+    }
+    nsrefcnt Release(void) {
+        NS_PRECONDITION(0 != mRefCnt, "dup release");
+        --mRefCnt;
+        NS_LOG_RELEASE(this, mRefCnt, "gfxFont");
+        if (mRefCnt == 0) {
+            // Don't delete just yet; return the object to the cache for
+            // possibly recycling within some time limit
+            gfxFontCache::GetCache()->NotifyReleased(this);
+            return 0;
+        }
+        return mRefCnt;
+    }
+
+    PRInt32 GetRefCount() { return mRefCnt; }
+
+protected:
+    nsAutoRefCnt mRefCnt;
 
 public:
     gfxFont(const nsAString &aName, const gfxFontStyle *aFontGroup);
     virtual ~gfxFont() {}
 
     const nsString& GetName() const { return mName; }
-    const gfxFontStyle *GetStyle() const { return mStyle; }
+    const gfxFontStyle *GetStyle() const { return &mStyle; }
 
     virtual nsString GetUniqueName() = 0;
 
@@ -283,14 +411,17 @@ public:
                                    PRUint32 aStart, PRUint32 aLength)
     { return PR_FALSE; }
 
+    // Expiration tracking
+    nsExpirationState *GetExpirationState() { return &mExpirationState; }
+
 protected:
     // The family name of the font
-    nsString mName;
+    nsString          mName;
+    nsExpirationState mExpirationState;
+    gfxFontStyle      mStyle;
 
     // This is called by the default Draw() implementation above.
     virtual void SetupCairoFont(cairo_t *aCR) = 0;
-
-    const gfxFontStyle *mStyle;
 };
 
 class THEBES_API gfxTextRunFactory {

gfx/thebes/public/gfxPangoFonts.h

@@ -123,17 +123,6 @@ public:
                               NS_STATIC_CAST(gfxFont*, mFonts[i]));
     }
 
-    gfxPangoFont *GetCachedFont(const nsAString& aName) const {
-        nsRefPtr<gfxPangoFont> font;
-        if (mFontCache.Get(aName, &font))
-            return font;
-        return nsnull;
-    }
-
-    void PutCachedFont(const nsAString& aName, gfxPangoFont *aFont) {
-        mFontCache.Put(aName, aFont);
-    }
-
 protected:
     friend class FontSelector;
 
@@ -167,7 +156,6 @@ protected:
                                 void *closure);
 
 private:
-    nsDataHashtable<nsStringHashKey, nsRefPtr<gfxPangoFont> > mFontCache;
     nsTArray<gfxFontStyle> mAdditionalStyles;
 };
 

gfx/thebes/public/gfxWindowsFonts.h

@@ -497,17 +497,6 @@ public:
         return PR_FALSE;
     }
 
-    gfxWindowsFont *GetCachedFont(const nsAString& aName) const {
-        nsRefPtr<gfxWindowsFont> font;
-        if (mFontCache.Get(aName, &font))
-            return font;
-        return nsnull;
-    }
-
-    void PutCachedFont(const nsAString& aName, gfxWindowsFont *aFont) {
-        mFontCache.Put(aName, aFont);
-    }
-
 protected:
     static PRBool MakeFont(const nsAString& fontName,
                            const nsACString& genericName,
@@ -519,11 +508,7 @@ protected:
     void InitTextRunUniscribe(gfxContext *aContext, gfxTextRun *aRun, const PRUnichar *aString, PRUint32 aLength);
 
 private:
-    friend class gfxWindowsTextRun;
-
     nsCString mGenericFamily;
-
-    nsDataHashtable<nsStringHashKey, nsRefPtr<gfxWindowsFont> > mFontCache;
 };
 
 #endif /* GFX_WINDOWSFONTS_H */

gfx/thebes/src/gfxAtsuiFonts.cpp

@@ -72,8 +72,9 @@ OSStatus ATSClearGlyphVector(void *glyphVectorPtr);
 #endif
 
 gfxAtsuiFont::gfxAtsuiFont(ATSUFontID fontID,
+                           const nsAString& name,
                            const gfxFontStyle *fontStyle)
-    : gfxFont(EmptyString(), fontStyle),
+    : gfxFont(name, fontStyle),
       mFontStyle(fontStyle), mATSUFontID(fontID), mATSUStyle(nsnull),
       mAdjustedSize(0)
 {
@@ -112,7 +113,7 @@ gfxAtsuiFont::InitMetrics(ATSUFontID aFontID, ATSFontRef aFontRef)
     };
 
     gfxFloat size =
-        PR_MAX(((mAdjustedSize != 0) ? mAdjustedSize : mStyle->size), 1.0f);
+        PR_MAX(((mAdjustedSize != 0) ? mAdjustedSize : GetStyle()->size), 1.0f);
 
     //fprintf (stderr, "string: '%s', size: %f\n", NS_ConvertUTF16toUTF8(aString).get(), size);
 
@@ -153,10 +154,10 @@ gfxAtsuiFont::InitMetrics(ATSUFontID aFontID, ATSFontRef aFontRef)
         mMetrics.xHeight = GetCharHeight('x');
 
     if (mAdjustedSize == 0) {
-        if (mStyle->sizeAdjust != 0) {
+        if (GetStyle()->sizeAdjust != 0) {
             gfxFloat aspect = mMetrics.xHeight / size;
             mAdjustedSize =
-                PR_MAX(ROUND(size * (mStyle->sizeAdjust / aspect)), 1.0f);
+                PR_MAX(ROUND(size * (GetStyle()->sizeAdjust / aspect)), 1.0f);
             InitMetrics(aFontID, aFontRef);
             return;
         }
@@ -212,7 +213,7 @@ gfxAtsuiFont::InitMetrics(ATSUFontID aFontID, ATSFontRef aFontRef)
 nsString
 gfxAtsuiFont::GetUniqueName()
 {
-    return gfxQuartzFontCache::SharedFontCache()->GetPostscriptNameForFontID(mATSUFontID);
+    return mName;
 }
 
 float
@@ -273,6 +274,33 @@ gfxAtsuiFont::GetMetrics()
     return mMetrics;
 }
 
+/**
+ * Look up the font in the gfxFont cache. If we don't find it, create one.
+ * In either case, add a ref, append it to the aFonts array, and return it ---
+ * except for OOM in which case we do nothing and return null.
+ */
+static gfxAtsuiFont *
+GetOrMakeFont(ATSUFontID aFontID, const gfxFontStyle *aStyle,
+              nsTArray<nsRefPtr<gfxFont> > *aFonts)
+{
+    const nsAString& name =
+        gfxQuartzFontCache::SharedFontCache()->GetPostscriptNameForFontID(aFontID);
+    nsRefPtr<gfxFont> font = gfxFontCache::GetCache()->Lookup(name, aStyle);
+    if (!font) {
+        font = new gfxAtsuiFont(aFontID, name, aStyle);
+        if (!font)
+            return nsnull;
+        gfxFontCache::GetCache()->AddNew(font);
+    }
+    // Add it to aFonts without unncessary refcount adjustment
+    nsRefPtr<gfxFont> *destination = aFonts->AppendElement();
+    if (!destination)
+        return nsnull;
+    destination->swap(font);
+    gfxFont *f = *destination;
+    return static_cast<gfxAtsuiFont *>(f);
+}
+
 gfxAtsuiFontGroup::gfxAtsuiFontGroup(const nsAString& families,
                                      const gfxFontStyle *aStyle)
     : gfxFontGroup(families, aStyle)
@@ -291,7 +319,7 @@ gfxAtsuiFontGroup::gfxAtsuiFontGroup(const nsAString& families,
         // a specific langGroup.  Let's just pick the default OSX
         // user font.
         ATSUFontID fontID = gfxQuartzFontCache::SharedFontCache()->GetDefaultATSUFontID (aStyle);
-        mFonts.AppendElement(new gfxAtsuiFont(fontID, aStyle));
+        GetOrMakeFont(fontID, aStyle, &mFonts);
     }
 
     // Create the fallback structure
@@ -331,7 +359,7 @@ gfxAtsuiFontGroup::FindATSUFont(const nsAString& aName,
 
     if (fontID != kATSUInvalidFontID) {
         //printf ("FindATSUFont! %s %d -> %d\n", NS_ConvertUTF16toUTF8(aName).get(), fontStyle->weight, (int)fontID);
-        fontGroup->mFonts.AppendElement(new gfxAtsuiFont(fontID, fontStyle));
+        GetOrMakeFont(fontID, fontStyle, &fontGroup->mFonts);
     }
 
     return PR_TRUE;
@@ -464,10 +492,7 @@ gfxAtsuiFontGroup::FindFontFor(ATSUFontID fid)
             return font;
     }
 
-    font = new gfxAtsuiFont(fid, GetStyle());
+    return GetOrMakeFont(fid, GetStyle(), &mFonts);
-    mFonts.AppendElement(font);
-
-    return font;
 }
 
 /**

gfx/thebes/src/gfxFont.cpp

@@ -39,6 +39,7 @@
 #include "nsIPref.h"
 #include "nsServiceManagerUtils.h"
 #include "nsReadableUtils.h"
+#include "nsExpirationTracker.h"
 
 #include "gfxFont.h"
 #include "gfxPlatform.h"
@@ -53,8 +54,97 @@
 
 #include "nsCRT.h"
 
+gfxFontCache *gfxFontCache::gGlobalCache = nsnull;
+
+nsresult
+gfxFontCache::Init()
+{
+    NS_ASSERTION(!gGlobalCache, "Where did this come from?");
+    gGlobalCache = new gfxFontCache();
+    return gGlobalCache ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
+}
+
+void
+gfxFontCache::Shutdown()
+{
+    delete gGlobalCache;
+    gGlobalCache = nsnull;
+}
+
+PRBool
+gfxFontCache::HashEntry::KeyEquals(const KeyTypePointer aKey) const
+{
+    return aKey->mString.Equals(mFont->GetName()) &&
+           aKey->mStyle->Equals(*mFont->GetStyle());
+}
+
+already_AddRefed<gfxFont>
+gfxFontCache::Lookup(const nsAString &aName,
+                     const gfxFontStyle *aStyle)
+{
+    Key key(aName, aStyle);
+    HashEntry *entry = mFonts.GetEntry(key);
+    if (!entry)
+        return nsnull;
+
+    gfxFont *font = entry->mFont;
+    NS_ADDREF(font);
+    if (font->GetExpirationState()->IsTracked()) {
+        RemoveObject(font);
+    }
+    return font;
+}
+
+void
+gfxFontCache::AddNew(gfxFont *aFont)
+{
+    Key key(aFont->GetName(), aFont->GetStyle());
+    HashEntry *entry = mFonts.PutEntry(key);
+    if (!entry)
+        return;
+    if (entry->mFont) {
+        // This is weird. Someone's asking us to overwrite an existing font.
+        // Oh well, make it happen ... just ensure that we're not tracking
+        // the old font
+        if (entry->mFont->GetExpirationState()->IsTracked()) {
+            RemoveObject(entry->mFont);
+        }
+    }
+    entry->mFont = aFont;
+}
+
+void
+gfxFontCache::NotifyReleased(gfxFont *aFont)
+{
+    nsresult rv = AddObject(aFont);
+    if (NS_FAILED(rv)) {
+        // We couldn't track it for some reason. Kill it now.
+        DestroyFont(aFont);
+    }
+    // Note that we might have fonts that aren't in the hashtable, perhaps because
+    // of OOM adding to the hashtable or because someone did an AddNew where
+    // we already had a font. These fonts are added to the expiration tracker
+    // anyway, even though Lookup can't resurrect them. Eventually they will
+    // expire and be deleted.
+}
+
+void
+gfxFontCache::NotifyExpired(gfxFont *aFont)
+{
+    RemoveObject(aFont);
+    DestroyFont(aFont);
+}
+
+void
+gfxFontCache::DestroyFont(gfxFont *aFont)
+{
+    Key key(aFont->GetName(), aFont->GetStyle());
+    mFonts.RemoveEntry(key);
+    delete aFont;
+}
+
 gfxFont::gfxFont(const nsAString &aName, const gfxFontStyle *aFontStyle) :
-    mName(aName), mStyle(aFontStyle)
+    mName(aName), mStyle(*aFontStyle)
 {
 }
 
@@ -505,6 +595,14 @@ gfxFontStyle::gfxFontStyle(PRUint8 aStyle, PRUint8 aVariant,
     }
 }
 
+gfxFontStyle::gfxFontStyle(const gfxFontStyle& aStyle) :
+    style(aStyle.style), systemFont(aStyle.systemFont), variant(aStyle.variant),
+    familyNameQuirks(aStyle.familyNameQuirks), weight(aStyle.weight),
+    decorations(aStyle.decorations), size(aStyle.size),
+    langGroup(aStyle.langGroup), sizeAdjust(aStyle.sizeAdjust)
+{
+}
+
 void
 gfxFontStyle::ComputeWeightAndOffset(PRInt8 *outBaseWeight, PRInt8 *outOffset) const
 {

gfx/thebes/src/gfxPangoFonts.cpp

@@ -127,6 +127,26 @@ gfxPangoFontGroup::FontCallback (const nsAString& fontName,
     return PR_TRUE;
 }
 
+/**
+ * Look up the font in the gfxFont cache. If we don't find it, create one.
+ * In either case, add a ref, append it to the aFonts array, and return it ---
+ * except for OOM in which case we do nothing and return null.
+ */
+static already_AddRefed<gfxPangoFont>
+GetOrMakeFont(const nsAString& aName, const gfxFontStyle *aStyle)
+{
+    nsRefPtr<gfxFont> font = gfxFontCache::GetCache()->Lookup(aName, aStyle);
+    if (!font) {
+        font = new gfxPangoFont(aName, aStyle);
+        if (!font)
+            return nsnull;
+        gfxFontCache::GetCache()->AddNew(font);
+    }
+    gfxFont *f = nsnull;
+    font.swap(f);
+    return static_cast<gfxPangoFont *>(f);
+}
+
 gfxPangoFontGroup::gfxPangoFontGroup (const nsAString& families,
                                       const gfxFontStyle *aStyle)
     : gfxFontGroup(families, aStyle)
@@ -135,8 +155,6 @@ gfxPangoFontGroup::gfxPangoFontGroup (const nsAString& families,
 
     nsStringArray familyArray;
 
-    mFontCache.Init(15);
-
     ForEachFont (FontCallback, &familyArray);
 
     FindGenericFontFromStyle (FontCallback, &familyArray);
@@ -149,8 +167,12 @@ gfxPangoFontGroup::gfxPangoFontGroup (const nsAString& families,
         familyArray.AppendString(NS_LITERAL_STRING("sans-serif"));
     }
 
-    for (int i = 0; i < familyArray.Count(); i++)
+    for (int i = 0; i < familyArray.Count(); i++) {
-        mFonts.AppendElement(new gfxPangoFont(*familyArray[i], &mStyle));
+        nsRefPtr<gfxPangoFont> font = GetOrMakeFont(*familyArray[i], &mStyle);
+        if (font) {
+            mFonts.AppendElement(font);
+        }
+    }
 }
 
 gfxPangoFontGroup::~gfxPangoFontGroup()
@@ -327,12 +349,12 @@ gfxPangoFont::RealizeFont(PRBool force)
     mPangoFontDesc = pango_font_description_new();
 
     pango_font_description_set_family(mPangoFontDesc, NS_ConvertUTF16toUTF8(mName).get());
-    gfxFloat size = mAdjustedSize ? mAdjustedSize : mStyle->size;
+    gfxFloat size = mAdjustedSize ? mAdjustedSize : GetStyle()->size;
     MOZ_pango_font_description_set_absolute_size(mPangoFontDesc, size * PANGO_SCALE);
-    pango_font_description_set_style(mPangoFontDesc, ThebesStyleToPangoStyle(mStyle));
+    pango_font_description_set_style(mPangoFontDesc, ThebesStyleToPangoStyle(GetStyle()));
-    pango_font_description_set_weight(mPangoFontDesc, ThebesStyleToPangoWeight(mStyle));
+    pango_font_description_set_weight(mPangoFontDesc, ThebesStyleToPangoWeight(GetStyle()));
 
-    //printf ("%s, %f, %d, %d\n", NS_ConvertUTF16toUTF8(mName).get(), mStyle->size, ThebesStyleToPangoStyle(mStyle), ThebesStyleToPangoWeight(mStyle));
+    //printf ("%s, %f, %d, %d\n", NS_ConvertUTF16toUTF8(mName).get(), GetStyle()->size, ThebesStyleToPangoStyle(GetStyle()), ThebesStyleToPangoWeight(GetStyle()));
 #ifndef THEBES_USE_PANGO_CAIRO
     mPangoCtx = pango_xft_get_context(GDK_DISPLAY(), 0);
     gdk_pango_context_set_colormap(mPangoCtx, gdk_rgb_get_cmap());
@@ -340,8 +362,8 @@ gfxPangoFont::RealizeFont(PRBool force)
     mPangoCtx = pango_cairo_font_map_create_context(PANGO_CAIRO_FONT_MAP(pango_cairo_font_map_get_default()));
 #endif
 
-    if (!mStyle->langGroup.IsEmpty())
+    if (!GetStyle()->langGroup.IsEmpty())
-        pango_context_set_language(mPangoCtx, GetPangoLanguage(mStyle->langGroup));
+        pango_context_set_language(mPangoCtx, GetPangoLanguage(GetStyle()->langGroup));
 
     pango_context_set_font_description(mPangoCtx, mPangoFontDesc);
 
@@ -350,15 +372,15 @@ gfxPangoFont::RealizeFont(PRBool force)
     if (mAdjustedSize != 0)
         return;
 
-    mAdjustedSize = mStyle->size;
+    mAdjustedSize = GetStyle()->size;
-    if (mStyle->sizeAdjust == 0)
+    if (GetStyle()->sizeAdjust == 0)
         return;
 
     gfxSize isz, lsz;
     GetSize("x", 1, isz, lsz);
-    gfxFloat aspect = isz.height / mStyle->size;
+    gfxFloat aspect = isz.height / GetStyle()->size;
     mAdjustedSize =
-        PR_MAX(NS_round(mStyle->size*(mStyle->sizeAdjust/aspect)), 1.0);
+        PR_MAX(NS_round(GetStyle()->size*(GetStyle()->sizeAdjust/aspect)), 1.0);
     RealizeFont(PR_TRUE);
 }
 
@@ -523,7 +545,7 @@ gfxPangoFont::GetMetrics()
     PangoFontMetrics *pfm = pango_font_get_metrics (font, NULL);
 
     // ??
-    mMetrics.emHeight = mAdjustedSize ? mAdjustedSize : mStyle->size;
+    mMetrics.emHeight = mAdjustedSize ? mAdjustedSize : GetStyle()->size;
 
     mMetrics.maxAscent = pango_font_metrics_get_ascent(pfm) / FLOAT_PANGO_SCALE;
     mMetrics.maxDescent = pango_font_metrics_get_descent(pfm) / FLOAT_PANGO_SCALE;
@@ -582,7 +604,7 @@ gfxPangoFont::GetMetrics()
 void
 gfxPangoFont::GetMozLang(nsACString &aMozLang)
 {
-    aMozLang.Assign(mStyle->langGroup);
+    aMozLang.Assign(GetStyle()->langGroup);
 }
 
 PangoFont *
@@ -1250,12 +1272,10 @@ public:
         if (ExistsFont(fs, aName))
             return PR_TRUE;
 
-        nsRefPtr<gfxPangoFont> font = fs->mGroup->GetCachedFont(aName);
+        nsRefPtr<gfxPangoFont> font = GetOrMakeFont(aName, fs->mGroup->GetStyle());
-        if (!font) {
+        if (font) {
-            font = new gfxPangoFont(aName, fs->mGroup->GetStyle());
+            fs->mFonts.AppendElement(font);
-            fs->mGroup->PutCachedFont(aName, font);
         }
-        fs->mFonts.AppendElement(font);
 
         return PR_TRUE;
     }

gfx/thebes/src/gfxPlatform.cpp

@@ -101,6 +101,13 @@ gfxPlatform::Init()
     }
 #endif
 
+    rv = gfxFontCache::Init();
+    if (NS_FAILED(rv)) {
+        NS_ERROR("Could not initialize gfxFontCache");
+        Shutdown();
+        return rv;
+    }
+
     rv = gfxTextRunCache::Init();
     if (NS_FAILED(rv)) {
         NS_ERROR("Could not initialize gfxTextRunCache");
@@ -117,6 +124,7 @@ gfxPlatform::Shutdown()
     // These may be called before the corresponding subsystems have actually
     // started up. That's OK, they can handle it.
     gfxTextRunCache::Shutdown();
+    gfxFontCache::Shutdown();
 #if defined(XP_MACOSX)
     gfxQuartzFontCache::Shutdown();
 #endif

gfx/thebes/src/gfxQuartzFontCache.mm

@@ -761,6 +761,7 @@ gfxQuartzFontCache::GetPostscriptNameForFontID(ATSUFontID fid)
     nsRefPtr<FontEntry> fe;
 
     if (!mFontIDTable.Get(PRUint32(fid), &fe)) {
+        NS_WARNING("Invalid font");
         return NS_LITERAL_STRING("INVALID_FONT");
     }
 

gfx/thebes/src/gfxWindowsFonts.cpp

@@ -203,7 +203,7 @@ gfxWindowsFont::MakeHFONT()
     }
 
     PRInt8 baseWeight, weightDistance;
-    mStyle->ComputeWeightAndOffset(&baseWeight, &weightDistance);
+    GetStyle()->ComputeWeightAndOffset(&baseWeight, &weightDistance);
 
     HDC dc = nsnull;
 
@@ -223,7 +223,7 @@ gfxWindowsFont::MakeHFONT()
                 if (!dc)
                     dc = GetDC((HWND)nsnull);
 
-                FillLogFont(mStyle->size, tryWeight);
+                FillLogFont(GetStyle()->size, tryWeight);
                 mFont = CreateFontIndirectW(&mLogFont);
                 HGDIOBJ oldFont = SelectObject(dc, mFont);
                 TEXTMETRIC metrics;
@@ -259,8 +259,8 @@ gfxWindowsFont::MakeHFONT()
     if (chosenWeight == 0)
         chosenWeight = baseWeight * 100;
 
-    mAdjustedSize = mStyle->size;
+    mAdjustedSize = GetStyle()->size;
-    if (mStyle->sizeAdjust > 0) {
+    if (GetStyle()->sizeAdjust > 0) {
         if (!mFont) {
             FillLogFont(mAdjustedSize, chosenWeight);
             mFont = CreateFontIndirectW(&mLogFont);
@@ -270,7 +270,7 @@ gfxWindowsFont::MakeHFONT()
         ComputeMetrics();
         gfxFloat aspect = mMetrics->xHeight / mMetrics->emHeight;
         mAdjustedSize =
-            PR_MAX(ROUND(mStyle->size * (mStyle->sizeAdjust / aspect)), 1.0f);
+            PR_MAX(ROUND(GetStyle()->size * (GetStyle()->sizeAdjust / aspect)), 1.0f);
 
         if (mMetrics != oldMetrics) {
             delete mMetrics;
@@ -417,8 +417,8 @@ gfxWindowsFont::FillLogFont(gfxFloat aSize, PRInt16 aWeight)
     mLogFont.lfWidth          = 0; 
     mLogFont.lfEscapement     = 0;
     mLogFont.lfOrientation    = 0;
-    mLogFont.lfUnderline      = (mStyle->decorations & FONT_DECORATION_UNDERLINE) ? TRUE : FALSE;
+    mLogFont.lfUnderline      = (GetStyle()->decorations & FONT_DECORATION_UNDERLINE) ? TRUE : FALSE;
-    mLogFont.lfStrikeOut      = (mStyle->decorations & FONT_DECORATION_STRIKEOUT) ? TRUE : FALSE;
+    mLogFont.lfStrikeOut      = (GetStyle()->decorations & FONT_DECORATION_STRIKEOUT) ? TRUE : FALSE;
     mLogFont.lfCharSet        = DEFAULT_CHARSET;
 #ifndef WINCE
     mLogFont.lfOutPrecision   = OUT_TT_PRECIS;
@@ -428,7 +428,7 @@ gfxWindowsFont::FillLogFont(gfxFloat aSize, PRInt16 aWeight)
     mLogFont.lfClipPrecision  = CLIP_TURNOFF_FONTASSOCIATION;
     mLogFont.lfQuality        = DEFAULT_QUALITY;
     mLogFont.lfPitchAndFamily = DEFAULT_PITCH | FF_DONTCARE;
-    mLogFont.lfItalic         = (mStyle->style & (FONT_STYLE_ITALIC | FONT_STYLE_OBLIQUE)) ? TRUE : FALSE;
+    mLogFont.lfItalic         = (GetStyle()->style & (FONT_STYLE_ITALIC | FONT_STYLE_OBLIQUE)) ? TRUE : FALSE;
     mLogFont.lfWeight         = aWeight;
 
     int len = PR_MIN(mName.Length(), LF_FACESIZE - 1);
@@ -489,6 +489,26 @@ gfxWindowsFont::SetupCairoFont(cairo_t *aCR)
  *
  **********************************************************************/
 
+/**
+ * Look up the font in the gfxFont cache. If we don't find it, create one.
+ * In either case, add a ref, append it to the aFonts array, and return it ---
+ * except for OOM in which case we do nothing and return null.
+ */
+static already_AddRefed<gfxWindowsFont>
+GetOrMakeFont(const nsAString& aName, const gfxFontStyle *aStyle)
+{
+    nsRefPtr<gfxFont> font = gfxFontCache::GetCache()->Lookup(aName, aStyle);
+    if (!font) {
+        font = new gfxWindowsFont(aName, aStyle);
+        if (!font)
+            return nsnull;
+        gfxFontCache::GetCache()->AddNew(font);
+    }
+    gfxFont *f = nsnull;
+    font.swap(f);
+    return static_cast<gfxWindowsFont *>(f);
+}
+
 PRBool
 gfxWindowsFontGroup::MakeFont(const nsAString& aName,
                               const nsACString& aGenericName,
@@ -500,8 +520,10 @@ gfxWindowsFontGroup::MakeFont(const nsAString& aName,
         if (fg->HasFontNamed(aName))
             return PR_TRUE;
 
-        gfxWindowsFont *font = new gfxWindowsFont(aName, fg->GetStyle());
+        nsRefPtr<gfxWindowsFont> font = GetOrMakeFont(aName, fg->GetStyle());
-        fg->AppendFont(font);
+        if (font) {
+            fg->AppendFont(font);
+        }
 
         if (!aGenericName.IsEmpty() && fg->GetGenericFamily().IsEmpty())
             fg->mGenericFamily = aGenericName;
@@ -514,8 +536,6 @@ gfxWindowsFontGroup::MakeFont(const nsAString& aName,
 gfxWindowsFontGroup::gfxWindowsFontGroup(const nsAString& aFamilies, const gfxFontStyle *aStyle)
     : gfxFontGroup(aFamilies, aStyle)
 {
-    mFontCache.Init(25);
-
     ForEachFont(MakeFont, this);
 
     if (mGenericFamily.IsEmpty())
@@ -930,14 +950,11 @@ public:
             if (aName.Equals(item->mFonts[i]->GetName()))
                 return PR_TRUE;
 
-
+        nsRefPtr<gfxWindowsFont> font =
-        nsRefPtr<gfxWindowsFont> font = item->mGroup->GetCachedFont(aName);
+            GetOrMakeFont(aName, item->mGroup->GetStyle());
-        if (!font) {
+        if (font) {
-            font = new gfxWindowsFont(aName, item->mGroup->GetStyle());
+            item->mFonts.AppendElement(font);
-            item->mGroup->PutCachedFont(aName, font);
         }
-        item->mFonts.AppendElement(font);
-
         return PR_TRUE;
     }