Fix TypefaceCache races.

http://codereview.appspot.com/5656066/


git-svn-id: http://skia.googlecode.com/svn/trunk@3205 2bbb7eff-a529-9590-31e7-b0007b416f81

src/core/SkTypefaceCache.cpp

@@ -94,12 +94,15 @@ SkTypeface* SkTypefaceCache::FindByID(SkFontID fontID) {
     return Get().findByID(fontID);
 }
 
-SkTypeface* SkTypefaceCache::FindByProc(FindProc proc, void* ctx) {
+SkTypeface* SkTypefaceCache::FindByProcAndRef(FindProc proc, void* ctx) {
     SkAutoMutexAcquire ama(gMutex);
-    return Get().findByProc(proc, ctx);
+    SkTypeface* typeface = Get().findByProc(proc, ctx);
+    SkSafeRef(typeface);
+    return typeface;
 }
 
 void SkTypefaceCache::PurgeAll() {
+    SkAutoMutexAcquire ama(gMutex);
     Get().purgeAll();
 }
 

src/core/SkTypefaceCache.h

@@ -23,6 +23,11 @@
 
 class SkTypefaceCache {
 public:
+    /**
+     * Callback for FindByProc. Returns true if the given typeface is a match
+     * for the given context. The passed typeface is owned by the cache and is
+     * not additionally ref()ed.
+     */
     typedef bool (*FindProc)(SkTypeface*, SkTypeface::Style, void* context);
 
     /**
@@ -33,29 +38,32 @@ public:
 
     /**
      *  Add a typeface to the cache. This ref()s the typeface, so that the
-     *  cache is also an owner. Later, if we need to purge the cache, it will
-     *  unref() typefaces whose refcnt is 1 (meaning only the cache is an owner).
+     *  cache is also an owner. Later, if we need to purge the cache, typefaces
+     *  whose refcnt is 1 (meaning only the cache is an owner) will be
+     *  unref()ed.
      */
     static void Add(SkTypeface*, SkTypeface::Style requested);
 
     /**
      *  Search the cache for a typeface with the specified fontID (uniqueID).
      *  If one is found, return it (its reference count is unmodified). If none
-     *  is found, return NULL.
+     *  is found, return NULL. The reference count is unmodified as it is
+     *  assumed that the stack will contain a ref to the typeface.
      */
     static SkTypeface* FindByID(SkFontID fontID);
 
     /**
      *  Iterate through the cache, calling proc(typeface, ctx) with each
-     *  typeface. If proc returns true, then we return that typeface (its
-     *  reference count is unmodified). If it never returns true, we return NULL.
+     *  typeface. If proc returns true, then we return that typeface (this
+     *  ref()s the typeface). If it never returns true, we return NULL.
      */
-    static SkTypeface* FindByProc(FindProc proc, void* ctx);
+    static SkTypeface* FindByProcAndRef(FindProc proc, void* ctx);
 
     /**
-     *  This will unref all of the typefaces in the cache. Normally this is
-     *  handled automatically as needed. This function is exposed for clients
-     *  that explicitly want to purge the entire cache (e.g. to look for leaks).
+     *  This will unref all of the typefaces in the cache for which the cache
+     *  is the only owner. Normally this is handled automatically as needed.
+     *  This function is exposed for clients that explicitly want to purge the
+     *  cache (e.g. to look for leaks).
      */
     static void PurgeAll();
 

src/ports/SkFontHost_mac_coretext.cpp

@@ -542,11 +542,9 @@ SkTypeface* SkFontHost::CreateTypeface(const SkTypeface* familyFace,
     }
 
     NameStyleRec rec = { familyName, style };
-    SkTypeface* face = SkTypefaceCache::FindByProc(FindByNameStyle, &rec);
+    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(FindByNameStyle, &rec);
 
-    if (face) {
-        face->ref();
-    } else {
+    if (NULL == face) {
         face = NewFromName(familyName, style);
         if (face) {
             SkTypefaceCache::Add(face, style);

src/ports/SkFontHost_win.cpp

@@ -175,10 +175,8 @@ static bool FindByLogFont(SkTypeface* face, SkTypeface::Style requestedStyle, vo
 SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF) {
     LOGFONT lf = origLF;
     make_canonical(&lf);
-    SkTypeface* face = SkTypefaceCache::FindByProc(FindByLogFont, &lf);
-    if (face) {
-        face->ref();
-    } else {
+    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(FindByLogFont, &lf);
+    if (NULL == face) {
         face = LogFontTypeface::Create(lf);
         SkTypefaceCache::Add(face, get_style(lf));
     }
@@ -205,7 +203,9 @@ SkFontID SkFontHost::NextLogicalFont(SkFontID currFontID, SkFontID origFontID) {
 
 static void ensure_typeface_accessible(SkFontID fontID) {
     LogFontTypeface* face = (LogFontTypeface*)SkTypefaceCache::FindByID(fontID);
-    SkFontHost::EnsureTypefaceAccessible(*face);
+    if (face) {
+        SkFontHost::EnsureTypefaceAccessible(*face);
+    }
 }
 
 static void GetLogFontByID(SkFontID fontID, LOGFONT* lf) {