Remove requestedStyle from SkTypefaceCache.

The typeface cache contains typefaces which can be compared against to
determine if an already known typeface will work instead of creating a
new typeface id. This is primarily for sharing scaler contexts. How that
typeface was requested is not relevant to this comparison, so don't cache
it. Request caching must be handled separately.

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1879423002

Review URL: https://codereview.chromium.org/1879423002

8 files changed, 106 insertions, 67 deletions

diff --git a/src/core/SkTypefaceCache.cpp b/src/core/SkTypefaceCache.cpp
index faf39788ed..05a7a8eb5f 100644
--- a/src/core/SkTypefaceCache.cpp
+++ b/src/core/SkTypefaceCache.cpp
@@ -15,46 +15,29 @@
 
 SkTypefaceCache::SkTypefaceCache() {}
 
-SkTypefaceCache::~SkTypefaceCache() {
-    const Rec* curr = fArray.begin();
-    const Rec* stop = fArray.end();
-    while (curr < stop) {
-        curr->fFace->unref();
-        curr += 1;
-    }
-}
-
-void SkTypefaceCache::add(SkTypeface* face, const SkFontStyle& requestedStyle) {
-    if (fArray.count() >= TYPEFACE_CACHE_LIMIT) {
+void SkTypefaceCache::add(SkTypeface* face) {
+    if (fTypefaces.count() >= TYPEFACE_CACHE_LIMIT) {
         this->purge(TYPEFACE_CACHE_LIMIT >> 2);
     }
 
-    Rec* rec = fArray.append();
-    rec->fFace = SkRef(face);
-    rec->fRequestedStyle = requestedStyle;
+    fTypefaces.emplace_back(SkRef(face));
 }
 
 SkTypeface* SkTypefaceCache::findByProcAndRef(FindProc proc, void* ctx) const {
-    const Rec* curr = fArray.begin();
-    const Rec* stop = fArray.end();
-    while (curr < stop) {
-        SkTypeface* currFace = curr->fFace;
-        if (proc(currFace, curr->fRequestedStyle, ctx)) {
-            return SkRef(currFace);
+    for (const sk_sp<SkTypeface>& typeface : fTypefaces) {
+        if (proc(typeface.get(), ctx)) {
+            return SkRef(typeface.get());
         }
-        curr += 1;
     }
     return nullptr;
 }
 
 void SkTypefaceCache::purge(int numToPurge) {
-    int count = fArray.count();
+    int count = fTypefaces.count();
     int i = 0;
     while (i < count) {
-        SkTypeface* face = fArray[i].fFace;
-        if (face->unique()) {
-            face->unref();
-            fArray.remove(i);
+        if (fTypefaces[i]->unique()) {
+            fTypefaces.removeShuffle(i);
             --count;
             if (--numToPurge == 0) {
                 return;
@@ -66,7 +49,7 @@ void SkTypefaceCache::purge(int numToPurge) {
 }
 
 void SkTypefaceCache::purgeAll() {
-    this->purge(fArray.count());
+    this->purge(fTypefaces.count());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -83,15 +66,14 @@ SkFontID SkTypefaceCache::NewFontID() {
 
 SK_DECLARE_STATIC_MUTEX(gMutex);
 
-void SkTypefaceCache::Add(SkTypeface* face, const SkFontStyle& requestedStyle) {
+void SkTypefaceCache::Add(SkTypeface* face) {
     SkAutoMutexAcquire ama(gMutex);
-    Get().add(face, requestedStyle);
+    Get().add(face);
 }
 
 SkTypeface* SkTypefaceCache::FindByProcAndRef(FindProc proc, void* ctx) {
     SkAutoMutexAcquire ama(gMutex);
-    SkTypeface* typeface = Get().findByProcAndRef(proc, ctx);
-    return typeface;
+    return Get().findByProcAndRef(proc, ctx);
 }
 
 void SkTypefaceCache::PurgeAll() {
@@ -102,16 +84,19 @@ void SkTypefaceCache::PurgeAll() {
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_DEBUG
-static bool DumpProc(SkTypeface* face, const SkFontStyle& s, void* ctx) {
-    SkDebugf("SkTypefaceCache: face %p fontID %d weight %d width %d style %d refcnt %d\n",
-             face, face->uniqueID(), s.weight(), s.width(), s.slant(), face->getRefCnt());
+static bool DumpProc(SkTypeface* face, void* ctx) {
+    SkString n;
+    face->getFamilyName(&n);
+    SkFontStyle s = face->fontStyle();
+    SkFontID id = face->uniqueID();
+    SkDebugf("SkTypefaceCache: face %p fontID %d weight %d width %d style %d refcnt %d name %s\n",
+             face, id, s.weight(), s.width(), s.slant(), face->getRefCnt(), n.c_str());
     return false;
 }
 #endif
 
 void SkTypefaceCache::Dump() {
 #ifdef SK_DEBUG
-    SkAutoMutexAcquire ama(gMutex);
     (void)Get().findByProcAndRef(DumpProc, nullptr);
 #endif
 }
diff --git a/src/core/SkTypefaceCache.h b/src/core/SkTypefaceCache.h
index bb2cb85bfc..bf90dae3e2 100644
--- a/src/core/SkTypefaceCache.h
+++ b/src/core/SkTypefaceCache.h
@@ -10,27 +10,20 @@
 #ifndef SkTypefaceCache_DEFINED
 #define SkTypefaceCache_DEFINED
 
+#include "SkRefCnt.h"
 #include "SkTypeface.h"
-#include "SkTDArray.h"
-
-/*  TODO
- *  Provide std way to cache name+requestedStyle aliases to the same typeface.
- *
- *  The current mechanism ends up create a diff typeface for each one, even if
- *  they map to the same internal obj (e.g. CTFontRef on the mac)
- */
+#include "SkTArray.h"
 
 class SkTypefaceCache {
 public:
     SkTypefaceCache();
-    ~SkTypefaceCache();
 
     /**
      * 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. The typeface may be in the disposed state.
      */
-    typedef bool(*FindProc)(SkTypeface*, const SkFontStyle&, void* context);
+    typedef bool(*FindProc)(SkTypeface*, void* context);
 
     /**
      *  Add a typeface to the cache. This ref()s the typeface, so that the
@@ -38,7 +31,7 @@ public:
      *  whose refcnt is 1 (meaning only the cache is an owner) will be
      *  unref()ed.
      */
-    void add(SkTypeface*, const SkFontStyle& requested);
+    void add(SkTypeface*);
 
     /**
      *  Iterate through the cache, calling proc(typeface, ctx) with each
@@ -63,7 +56,7 @@ public:
 
     // These are static wrappers around a global instance of a cache.
 
-    static void Add(SkTypeface*, const SkFontStyle& requested);
+    static void Add(SkTypeface*);
     static SkTypeface* FindByProcAndRef(FindProc proc, void* ctx);
     static void PurgeAll();
 
@@ -77,11 +70,7 @@ private:
 
     void purge(int count);
 
-    struct Rec {
-        SkTypeface* fFace;
-        SkFontStyle fRequestedStyle;
-    };
-    SkTDArray<Rec> fArray;
+    SkTArray<sk_sp<SkTypeface>> fTypefaces;
 };
 
 #endif
diff --git a/src/ports/SkFontHost_fontconfig.cpp b/src/ports/SkFontHost_fontconfig.cpp
index 0ed8d8fa3e..1394efdda3 100644
--- a/src/ports/SkFontHost_fontconfig.cpp
+++ b/src/ports/SkFontHost_fontconfig.cpp
@@ -58,7 +58,7 @@ SkFontConfigInterface* SkFontHost_fontconfig_ref_global() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-static bool find_by_FontIdentity(SkTypeface* cachedTypeface, const SkFontStyle&, void* ctx) {
+static bool find_by_FontIdentity(SkTypeface* cachedTypeface, void* ctx) {
     typedef SkFontConfigInterface::FontIdentity FontIdentity;
     FontConfigTypeface* cachedFCTypeface = static_cast<FontConfigTypeface*>(cachedTypeface);
     FontIdentity* identity = static_cast<FontIdentity*>(ctx);
@@ -196,7 +196,7 @@ SkTypeface* FontConfigTypeface::LegacyCreateTypeface(const char requestedFamilyN
     if (!face) {
         face = FontConfigTypeface::Create(outStyle, identity, outFamilyName);
         // Add this FontIdentity to the FontIdentity cache.
-        SkTypefaceCache::Add(face, outStyle);
+        SkTypefaceCache::Add(face);
     }
     // Add this request to the request cache.
     SkFontHostRequestCache::Add(face, request.release());
diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 049d08b6fc..02c54640bb 100644
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -486,7 +486,7 @@ static SkTypeface* NewFromFontRef(CTFontRef fontRef, CFTypeRef resourceRef, bool
     return new SkTypeface_Mac(fontRef, resourceRef, style, isFixedPitch, isLocalStream);
 }
 
-static bool find_by_CTFontRef(SkTypeface* cached, const SkFontStyle&, void* context) {
+static bool find_by_CTFontRef(SkTypeface* cached, void* context) {
     CTFontRef self = (CTFontRef)context;
     CTFontRef other = ((SkTypeface_Mac*)cached)->fFontRef;
 
@@ -546,7 +546,7 @@ static SkTypeface* NewFromName(const char familyName[], const SkFontStyle& theSt
         return face;
     }
     face = NewFromFontRef(ctFont.release(), nullptr, false);
-    SkTypefaceCache::Add(face, face->fontStyle());
+    SkTypefaceCache::Add(face);
     return face;
 }
 
@@ -583,7 +583,7 @@ SkTypeface* SkCreateTypefaceFromCTFont(CTFontRef fontRef, CFTypeRef resourceRef)
         CFRetain(resourceRef);
     }
     face = NewFromFontRef(fontRef, resourceRef, false);
-    SkTypefaceCache::Add(face, face->fontStyle());
+    SkTypefaceCache::Add(face);
     return face;
 }
 
@@ -2166,7 +2166,7 @@ static SkTypeface* createFromDesc(CTFontDescriptorRef desc) {
     }
 
     face = NewFromFontRef(ctFont.release(), nullptr, false);
-    SkTypefaceCache::Add(face, face->fontStyle());
+    SkTypefaceCache::Add(face);
     return face;
 }
 
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index c9781ba23a..5ded39c1b4 100644
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -308,13 +308,11 @@ static const LOGFONT& get_default_font() {
     return gDefaultFont;
 }
 
-static bool FindByLogFont(SkTypeface* face, const SkFontStyle& requestedStyle, void* ctx) {
+static bool FindByLogFont(SkTypeface* face, void* ctx) {
     LogFontTypeface* lface = static_cast<LogFontTypeface*>(face);
     const LOGFONT* lf = reinterpret_cast<const LOGFONT*>(ctx);
 
-    return lface &&
-           get_style(lface->fLogFont) == requestedStyle &&
-           !memcmp(&lface->fLogFont, lf, sizeof(LOGFONT));
+    return !memcmp(&lface->fLogFont, lf, sizeof(LOGFONT));
 }
 
 /**
@@ -327,7 +325,7 @@ SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF) {
     SkTypeface* face = SkTypefaceCache::FindByProcAndRef(FindByLogFont, &lf);
     if (nullptr == face) {
         face = LogFontTypeface::Create(lf);
-        SkTypefaceCache::Add(face, get_style(lf));
+        SkTypefaceCache::Add(face);
     }
     return face;
 }
diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp
index b672f43177..4b26f41ea5 100644
--- a/src/ports/SkFontMgr_fontconfig.cpp
+++ b/src/ports/SkFontMgr_fontconfig.cpp
@@ -605,7 +605,7 @@ class SkFontMgr_fontconfig : public SkFontMgr {
                                           sizes.begin(), names.count());
     }
 
-    static bool FindByFcPattern(SkTypeface* cached, const SkFontStyle&, void* ctx) {
+    static bool FindByFcPattern(SkTypeface* cached, void* ctx) {
         SkTypeface_fontconfig* cshFace = static_cast<SkTypeface_fontconfig*>(cached);
         FcPattern* ctxPattern = static_cast<FcPattern*>(ctx);
         return FcTrue == FcPatternEqual(cshFace->fPattern, ctxPattern);
@@ -624,7 +624,7 @@ class SkFontMgr_fontconfig : public SkFontMgr {
             FcPatternReference(pattern);
             face = SkTypeface_fontconfig::Create(pattern);
             if (face) {
-                fTFCache.add(face, SkFontStyle());
+                fTFCache.add(face);
             }
         }
         return face;
diff --git a/src/ports/SkFontMgr_win_dw.cpp b/src/ports/SkFontMgr_win_dw.cpp
index a03fcf8d42..e178598a55 100644
--- a/src/ports/SkFontMgr_win_dw.cpp
+++ b/src/ports/SkFontMgr_win_dw.cpp
@@ -353,7 +353,7 @@ struct ProtoDWriteTypeface {
     IDWriteFontFamily* fDWriteFontFamily;
 };
 
-static bool FindByDWriteFont(SkTypeface* cached, const SkFontStyle&, void* ctx) {
+static bool FindByDWriteFont(SkTypeface* cached, void* ctx) {
     DWriteFontTypeface* cshFace = reinterpret_cast<DWriteFontTypeface*>(cached);
     ProtoDWriteTypeface* ctxFace = reinterpret_cast<ProtoDWriteTypeface*>(ctx);
     bool same;
@@ -457,7 +457,7 @@ SkTypeface* SkFontMgr_DirectWrite::createTypefaceFromDWriteFont(
     if (nullptr == face) {
         face = DWriteFontTypeface::Create(fFactory.get(), fontFace, font, fontFamily);
         if (face) {
-            fTFCache.add(face, get_style(font));
+            fTFCache.add(face);
         }
     }
     return face;
diff --git a/tests/TypefaceTest.cpp b/tests/TypefaceTest.cpp
index b3bc157eba..130cabf37a 100644
--- a/tests/TypefaceTest.cpp
+++ b/tests/TypefaceTest.cpp
@@ -5,7 +5,9 @@
  * found in the LICENSE file.
  */
 
+#include "SkRefCnt.h"
 #include "SkTypeface.h"
+#include "SkTypefaceCache.h"
 #include "Test.h"
 
 DEF_TEST(Typeface, reporter) {
@@ -24,3 +26,68 @@ DEF_TEST(Typeface, reporter) {
     REPORTER_ASSERT(reporter, nullptr == t3.get());
 #endif
 }
+
+class SkEmptyTypeface : public SkTypeface {
+public:
+    static sk_sp<SkTypeface> Create(SkFontID id) { return sk_sp<SkTypeface>(new SkEmptyTypeface(id)); }
+protected:
+    SkEmptyTypeface(SkFontID id) : SkTypeface(SkFontStyle(), id, true) { }
+
+    SkStreamAsset* onOpenStream(int* ttcIndex) const override { return nullptr; }
+    SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
+                                           const SkDescriptor*) const override {
+        return nullptr;
+    }
+    void onFilterRec(SkScalerContextRec*) const override { }
+    virtual SkAdvancedTypefaceMetrics* onGetAdvancedTypefaceMetrics(
+                                PerGlyphInfo,
+                                const uint32_t*, uint32_t) const override { return nullptr; }
+    void onGetFontDescriptor(SkFontDescriptor*, bool*) const override { }
+    virtual int onCharsToGlyphs(const void* chars, Encoding encoding,
+                                uint16_t glyphs[], int glyphCount) const override {
+        SK_ABORT("unimplemented");
+        return 0;
+    }
+    int onCountGlyphs() const override { return 0; };
+    int onGetUPEM() const override { return 0; };
+    void onGetFamilyName(SkString* familyName) const override { familyName->reset(); }
+    SkTypeface::LocalizedStrings* onCreateFamilyNameIterator() const override {
+        SK_ABORT("unimplemented");
+        return nullptr;
+    };
+    int onGetTableTags(SkFontTableTag tags[]) const override { return 0; }
+    size_t onGetTableData(SkFontTableTag, size_t, size_t, void*) const override { return 0; }
+};
+
+static bool count_proc(SkTypeface* face, void* ctx) {
+    int* count = static_cast<int*>(ctx);
+    *count = *count + 1;
+    return false;
+}
+static int count(skiatest::Reporter* reporter, const SkTypefaceCache& cache) {
+    int count = 0;
+    SkTypeface* none = cache.findByProcAndRef(count_proc, &count);
+    REPORTER_ASSERT(reporter, none == nullptr);
+    return count;
+}
+
+DEF_TEST(TypefaceCache, reporter) {
+    sk_sp<SkTypeface> t1(SkEmptyTypeface::Create(1));
+    {
+        SkTypefaceCache cache;
+        REPORTER_ASSERT(reporter, count(reporter, cache) == 0);
+        {
+            sk_sp<SkTypeface> t0(SkEmptyTypeface::Create(0));
+            cache.add(t0.get());
+            REPORTER_ASSERT(reporter, count(reporter, cache) == 1);
+            cache.add(t1.get());
+            REPORTER_ASSERT(reporter, count(reporter, cache) == 2);
+            cache.purgeAll();
+            REPORTER_ASSERT(reporter, count(reporter, cache) == 2);
+        }
+        REPORTER_ASSERT(reporter, count(reporter, cache) == 2);
+        cache.purgeAll();
+        REPORTER_ASSERT(reporter, count(reporter, cache) == 1);
+    }
+    REPORTER_ASSERT(reporter, t1->unique());
+}