Cache hb_font_t and the user data container instead of hb_face_t

third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp

Index: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
index cfec720964a7de76e6b2eb87e0f7fc4505fb3b5d..dd535b57ae234233585dce634780f0fb4b9fff0a 100644
--- a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
@@ -30,16 +30,6 @@
 
 #include "platform/fonts/shaping/HarfBuzzFace.h"
 
-#include "hb-ot.h"
-#include "hb.h"
-#if OS(MACOSX)
-#include "hb-coretext.h"
-#endif
-#include "SkPaint.h"
-#include "SkPath.h"
-#include "SkPoint.h"
-#include "SkRect.h"
-#include "SkTypeface.h"
 #include "platform/fonts/FontCache.h"
 #include "platform/fonts/FontPlatformData.h"
 #include "platform/fonts/SimpleFontData.h"
@@ -48,86 +38,101 @@
 #include "wtf/HashMap.h"
 #include "wtf/MathExtras.h"
 
-namespace blink {
+#include <hb-ot.h>
+#include <hb.h>
+#if OS(MACOSX)
+#include <hb-coretext.h>
+#endif
 
-// Though we have FontCache class, which provides the cache mechanism for
-// WebKit's font objects, we also need additional caching layer for HarfBuzz
-// to reduce the memory consumption because hb_face_t should be associated with
-// underling font data (e.g. CTFontRef, FTFace).
+#include <SkPaint.h>
+#include <SkPath.h>
+#include <SkPoint.h>
+#include <SkRect.h>
+#include <SkTypeface.h>
 
-class FaceCacheEntry : public RefCounted<FaceCacheEntry> {
+
+namespace blink {
+
+// struct to carry user-pointer data for hb_font_t callback functions.
+struct HarfBuzzFontData {
+    USING_FAST_MALLOC(HarfBuzzFontData);
+    WTF_MAKE_NONCOPYABLE(HarfBuzzFontData);
 public:
-    static PassRefPtr<FaceCacheEntry> create(hb_face_t* face)
+
+    HarfBuzzFontData()
+        : m_paint(SkPaint())
+        , m_simpleFontData(nullptr)
+        , m_rangeSet(nullptr)
     {
-        ASSERT(face);
-        return adoptRef(new FaceCacheEntry(face));
     }
-    ~FaceCacheEntry()
+
+    SkPaint m_paint;
+    RefPtr<SimpleFontData> m_simpleFontData;
+    RefPtr<UnicodeRangeSet> m_rangeSet;
+};
+
+// Though we have FontCache class, which provides the cache mechanism for
+// WebKit's font objects, we also need additional caching layer for HarfBuzz to
+// reduce the number of hb_font_t objects created. Without it, we would create
+// an hb_font_t object for every FontPlatformData object. But insted, we only
+// need one for each unique SkTypeface.
+// FIXME: We should fix the FontCache to only keep one FontPlatformData object
+// independet of size, then consider using this here.

[eae, 2016/05/04 12:46:03]

If (when) we fix that, would we even need this cache? Long term it would be nice
to have one true font cache instead of the multiple layers of caches we have
today...

+class HbFontCacheEntry : public RefCounted<HbFontCacheEntry> {
+public:
+    static PassRefPtr<HbFontCacheEntry> create(hb_font_t* hbFont)
     {
-        hb_face_destroy(m_face);
+        ASSERT(hbFont);
+        return adoptRef(new HbFontCacheEntry(hbFont));
     }
 
-    hb_face_t* face() { return m_face; }
+    hb_font_t* hbFont() { return m_hbFont.get(); }
+    HarfBuzzFontData* hbFontData() { return m_hbFontData.get(); }
 
 private:
-    explicit FaceCacheEntry(hb_face_t* face)
-        : m_face(face)
-    { }
+    explicit HbFontCacheEntry(hb_font_t* font)
+        : m_hbFont(adoptPtr(font))
+        , m_hbFontData(adoptPtr(new HarfBuzzFontData()))
+    { };
 
-    hb_face_t* m_face;
+    OwnPtr<hb_font_t> m_hbFont;
+    OwnPtr<HarfBuzzFontData> m_hbFontData;
 };
 
-typedef HashMap<uint64_t, RefPtr<FaceCacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t>> HarfBuzzFaceCache;
+typedef HashMap<uint64_t, RefPtr<HbFontCacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t>> HarfBuzzFontCache;
 
-static HarfBuzzFaceCache* harfBuzzFaceCache()
+static HarfBuzzFontCache* harfBuzzFontCache()
 {
-    DEFINE_STATIC_LOCAL(HarfBuzzFaceCache, s_harfBuzzFaceCache, ());
-    return &s_harfBuzzFaceCache;
+    DEFINE_STATIC_LOCAL(HarfBuzzFontCache, s_harfBuzzFontCache, ());
+    return &s_harfBuzzFontCache;
 }
 
+static PassRefPtr<HbFontCacheEntry> createHbFontCacheEntry(hb_face_t*);
+
 HarfBuzzFace::HarfBuzzFace(FontPlatformData* platformData, uint64_t uniqueID)
     : m_platformData(platformData)
     , m_uniqueID(uniqueID)
 {
-    HarfBuzzFaceCache::AddResult result = harfBuzzFaceCache()->add(m_uniqueID, nullptr);
-    if (result.isNewEntry)
-        result.storedValue->value = FaceCacheEntry::create(createFace());
+    HarfBuzzFontCache::AddResult result = harfBuzzFontCache()->add(m_uniqueID, nullptr);
+    if (result.isNewEntry) {
+        OwnPtr<hb_face_t> face = adoptPtr(createFace());
+        result.storedValue->value = createHbFontCacheEntry(face.get());
+    }
     result.storedValue->value->ref();
-    m_face = result.storedValue->value->face();
-    prepareHarfBuzzFontData();
+    m_unscaledFont = result.storedValue->value->hbFont();
+    m_harfBuzzFontData = result.storedValue->value->hbFontData();
 }
 
 HarfBuzzFace::~HarfBuzzFace()
 {
-    HarfBuzzFaceCache::iterator result = harfBuzzFaceCache()->find(m_uniqueID);
-    ASSERT_WITH_SECURITY_IMPLICATION(result != harfBuzzFaceCache()->end());
+    HarfBuzzFontCache::iterator result = harfBuzzFontCache()->find(m_uniqueID);
+    ASSERT_WITH_SECURITY_IMPLICATION(result != harfBuzzFontCache()->end());
     ASSERT(result.get()->value->refCount() > 1);
     result.get()->value->deref();
     if (result.get()->value->refCount() == 1)
-        harfBuzzFaceCache()->remove(m_uniqueID);
+        harfBuzzFontCache()->remove(m_uniqueID);
 }
 
-// struct to carry user-pointer data for hb_font_t callback functions.
-struct HarfBuzzFontData {
-    USING_FAST_MALLOC(HarfBuzzFontData);
-    WTF_MAKE_NONCOPYABLE(HarfBuzzFontData);
-public:
-
-    HarfBuzzFontData() {}
-    HarfBuzzFontData(WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry, hb_face_t* face, PassRefPtr<UnicodeRangeSet> rangeSet)
-        : m_face(face)
-        , m_hbOpenTypeFont(nullptr)
-        , m_rangeSet(rangeSet)
-    {
-    }
-
-    SkPaint m_paint;
-    RefPtr<SimpleFontData> m_simpleFontData;
-    hb_face_t* m_face;
-    hb_font_t* m_hbOpenTypeFont;
-    RefPtr<UnicodeRangeSet> m_rangeSet;
-};
-
 static hb_position_t SkiaScalarToHarfBuzzPosition(SkScalar value)
 {
     // We treat HarfBuzz hb_position_t as 16.16 fixed-point.
@@ -280,7 +285,6 @@ static hb_blob_t* harfBuzzSkiaGetTable(hb_face_t* face, hb_tag_t tag, void* user
         WTF::Partitions::fastFree(buffer);
         return nullptr;
     }
-
     return hb_blob_create(const_cast<char*>(buffer), tableSize, HB_MEMORY_MODE_WRITABLE, buffer, WTF::Partitions::fastFree);
 }
 #endif
@@ -296,26 +300,27 @@ hb_face_t* HarfBuzzFace::createFace()
     return face;
 }
 
-void HarfBuzzFace::prepareHarfBuzzFontData()
+PassRefPtr<HbFontCacheEntry> createHbFontCacheEntry(hb_face_t* face)
 {
-    m_harfBuzzFontData = adoptPtr(new HarfBuzzFontData());
-    m_harfBuzzFontData->m_simpleFontData = FontCache::fontCache()->fontDataFromFontPlatformData(m_platformData);
-    ASSERT(m_harfBuzzFontData->m_simpleFontData);
-    OwnPtr<hb_font_t> otFont = adoptPtr(hb_font_create(m_face));
+    OwnPtr<hb_font_t> otFont = adoptPtr(hb_font_create(face));
     hb_ot_font_set_funcs(otFont.get());
     // Creating a sub font means that non-available functions
     // are found from the parent.
-    m_unscaledFont = adoptPtr(hb_font_create_sub_font(otFont.get()));
-    hb_font_set_funcs(m_unscaledFont.get(), harfBuzzSkiaGetFontFuncs(), m_harfBuzzFontData.get(), nullptr);
+    hb_font_t* unscaledFont = hb_font_create_sub_font(otFont.get());
+    RefPtr<HbFontCacheEntry> cacheEntry = HbFontCacheEntry::create(unscaledFont);
+    hb_font_set_funcs(unscaledFont, harfBuzzSkiaGetFontFuncs(), cacheEntry->hbFontData(), nullptr);
+    return cacheEntry;
 }
 
-hb_font_t* HarfBuzzFace::getScaledFont(PassRefPtr<UnicodeRangeSet> rangeSet)
+hb_font_t* HarfBuzzFace::getScaledFont(PassRefPtr<UnicodeRangeSet> rangeSet) const
 {
     m_platformData->setupPaint(&m_harfBuzzFontData->m_paint);
     m_harfBuzzFontData->m_rangeSet = rangeSet;
+    m_harfBuzzFontData->m_simpleFontData = FontCache::fontCache()->fontDataFromFontPlatformData(m_platformData);
+    ASSERT(m_harfBuzzFontData->m_simpleFontData);
     int scale = SkiaScalarToHarfBuzzPosition(m_platformData->size());
-    hb_font_set_scale(m_unscaledFont.get(), scale, scale);
-    return m_unscaledFont.get();
+    hb_font_set_scale(m_unscaledFont, scale, scale);
+    return m_unscaledFont;
 }
 
 } // namespace blink