Extract hb_face_t object cache mechanism from Layout.cpp.

This CL does following things:
- Extract hb_face_t object cache mechanism from Layout.cpp to be able to
  use it from other cpp file, especially from FontFamily.cpp.
  To address Bug 11256006 and Bug 17759267, need to touch
  hb_face_t from FontFamily.
- Make hb_face_t cache mechanism thread-safe.
- Add unit tests for HbFaceCache test cases.

Bug: 11256006
Bug: 17759267
Change-Id: Ic183634ef34326793bd9a32167236611d0af34d6

engine/src/flutter/libs/minikin/Android.mk

@@ -22,6 +22,7 @@ minikin_src_files := \
     FontCollection.cpp \
     FontFamily.cpp \
     GraphemeBreak.cpp \
+    HbFaceCache.cpp \
     Hyphenator.cpp \
     Layout.cpp \
     LayoutUtils.cpp \

engine/src/flutter/libs/minikin/HbFaceCache.cpp

@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "Minikin"
+
+#include "HbFaceCache.h"
+
+#include <hb.h>
+#include <utils/LruCache.h>
+
+#include <minikin/MinikinFont.h>
+#include "MinikinInternal.h"
+
+namespace android {
+
+static hb_blob_t* referenceTable(hb_face_t* face, hb_tag_t tag, void* userData) {
+    MinikinFont* font = reinterpret_cast<MinikinFont*>(userData);
+    size_t length = 0;
+    bool ok = font->GetTable(tag, NULL, &length);
+    if (!ok) {
+        return 0;
+    }
+    char* buffer = reinterpret_cast<char*>(malloc(length));
+    if (!buffer) {
+        return 0;
+    }
+    ok = font->GetTable(tag, reinterpret_cast<uint8_t*>(buffer), &length);
+#ifdef VERBOSE_DEBUG
+    ALOGD("referenceTable %c%c%c%c length=%d %d",
+        (tag >>24)&0xff, (tag>>16)&0xff, (tag>>8)&0xff, tag&0xff, length, ok);
+#endif
+    if (!ok) {
+        free(buffer);
+        return 0;
+    }
+    return hb_blob_create(const_cast<char*>(buffer), length,
+            HB_MEMORY_MODE_WRITABLE, buffer, free);
+}
+
+static unsigned int disabledDecomposeCompatibility(
+        hb_unicode_funcs_t*, hb_codepoint_t, hb_codepoint_t*, void*) {
+    return 0;
+}
+
+class HbFaceCache : private OnEntryRemoved<int32_t, hb_face_t*> {
+public:
+    HbFaceCache() : mCache(kMaxEntries) {
+        mCache.setOnEntryRemovedListener(this);
+    }
+
+    // callback for OnEntryRemoved
+    void operator()(int32_t& key, hb_face_t*& value) {
+        hb_face_destroy(value);
+    }
+
+    hb_face_t* get(int32_t fontId) {
+        return mCache.get(fontId);
+    }
+
+    void put(int32_t fontId, hb_face_t* face) {
+        mCache.put(fontId, face);
+    }
+
+    void clear() {
+        mCache.clear();
+    }
+
+private:
+    static const size_t kMaxEntries = 100;
+
+    LruCache<int32_t, hb_face_t*> mCache;
+};
+
+HbFaceCache* getFaceCacheLocked() {
+    assertMinikinLocked();
+    static HbFaceCache* cache = nullptr;
+    if (cache == nullptr) {
+        cache = new HbFaceCache();
+    }
+    return cache;
+}
+
+void purgeHbFaceCacheLocked() {
+    assertMinikinLocked();
+    getFaceCacheLocked()->clear();
+}
+
+hb_face_t* getHbFaceLocked(MinikinFont* minikinFont) {
+    assertMinikinLocked();
+    if (minikinFont == nullptr) {
+        return nullptr;
+    }
+
+    HbFaceCache* faceCache = getFaceCacheLocked();
+    const int32_t fontId = minikinFont->GetUniqueId();
+    hb_face_t* face = faceCache->get(fontId);
+    if (face != nullptr) {
+        return face;
+    }
+
+    face = hb_face_create_for_tables(referenceTable, minikinFont, nullptr);
+    faceCache->put(fontId, face);
+    return face;
+}
+
+}  // namespace android

engine/src/flutter/libs/minikin/HbFaceCache.h

@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef MINIKIN_HBFACE_CACHE_H
+#define MINIKIN_HBFACE_CACHE_H
+
+struct hb_face_t;
+
+namespace android {
+class MinikinFont;
+
+void purgeHbFaceCacheLocked();
+hb_face_t* getHbFaceLocked(MinikinFont* minikinFont);
+
+}  // namespace android
+#endif  // MINIKIN_HBFACE_CACHE_H

engine/src/flutter/libs/minikin/Layout.cpp

@@ -35,6 +35,7 @@
 #include <hb-ot.h>
 
 #include "LayoutUtils.h"
+#include "HbFaceCache.h"
 #include "MinikinInternal.h"
 #include <minikin/MinikinFontFreeType.h>
 #include <minikin/Layout.h>
@@ -188,22 +189,6 @@ private:
     static const size_t kMaxEntries = 5000;
 };
 
-class HbFaceCache : private OnEntryRemoved<int32_t, hb_face_t*> {
-public:
-    HbFaceCache() : mCache(kMaxEntries) {
-        mCache.setOnEntryRemovedListener(this);
-    }
-
-    // callback for OnEntryRemoved
-    void operator()(int32_t& key, hb_face_t*& value) {
-        hb_face_destroy(value);
-    }
-
-    LruCache<int32_t, hb_face_t*> mCache;
-private:
-    static const size_t kMaxEntries = 100;
-};
-
 static unsigned int disabledDecomposeCompatibility(hb_unicode_funcs_t*, hb_codepoint_t,
                                                    hb_codepoint_t*, void*) {
     return 0;
@@ -223,7 +208,6 @@ public:
     hb_buffer_t* hbBuffer;
     hb_unicode_funcs_t* unicodeFunctions;
     LayoutCache layoutCache;
-    HbFaceCache hbFaceCache;
 };
 
 ANDROID_SINGLETON_STATIC_INSTANCE(LayoutEngine);
@@ -291,30 +275,6 @@ void Layout::setFontCollection(const FontCollection* collection) {
     mCollection = collection;
 }
 
-hb_blob_t* referenceTable(hb_face_t* face, hb_tag_t tag, void* userData)  {
-    MinikinFont* font = reinterpret_cast<MinikinFont*>(userData);
-    size_t length = 0;
-    bool ok = font->GetTable(tag, NULL, &length);
-    if (!ok) {
-        return 0;
-    }
-    char* buffer = reinterpret_cast<char*>(malloc(length));
-    if (!buffer) {
-        return 0;
-    }
-    ok = font->GetTable(tag, reinterpret_cast<uint8_t*>(buffer), &length);
-#ifdef VERBOSE_DEBUG
-    ALOGD("referenceTable %c%c%c%c length=%d %d",
-        (tag >>24) & 0xff, (tag>>16)&0xff, (tag>>8)&0xff, tag&0xff, length, ok);
-#endif
-    if (!ok) {
-        free(buffer);
-        return 0;
-    }
-    return hb_blob_create(const_cast<char*>(buffer), length,
-        HB_MEMORY_MODE_WRITABLE, buffer, free);
-}
-
 static hb_position_t harfbuzzGetGlyphHorizontalAdvance(hb_font_t* hbFont, void* fontData, hb_codepoint_t glyph, void* userData)
 {
     MinikinPaint* paint = reinterpret_cast<MinikinPaint*>(fontData);
@@ -342,19 +302,8 @@ hb_font_funcs_t* getHbFontFuncs() {
     return hbFontFuncs;
 }
 
-static hb_face_t* getHbFace(MinikinFont* minikinFont) {
-    HbFaceCache& cache = LayoutEngine::getInstance().hbFaceCache;
-    int32_t fontId = minikinFont->GetUniqueId();
-    hb_face_t* face = cache.mCache.get(fontId);
-    if (face == NULL) {
-        face = hb_face_create_for_tables(referenceTable, minikinFont, NULL);
-        cache.mCache.put(fontId, face);
-    }
-    return face;
-}
-
 static hb_font_t* create_hb_font(MinikinFont* minikinFont, MinikinPaint* minikinPaint) {
-    hb_face_t* face = getHbFace(minikinFont);
+    hb_face_t* face = getHbFaceLocked(minikinFont);
     hb_font_t* parent_font = hb_font_create(face);
     hb_ot_font_set_funcs(parent_font);
 
@@ -885,8 +834,7 @@ void Layout::purgeCaches() {
     AutoMutex _l(gMinikinLock);
     LayoutCache& layoutCache = LayoutEngine::getInstance().layoutCache;
     layoutCache.clear();
-    HbFaceCache& hbCache = LayoutEngine::getInstance().hbFaceCache;
-    hbCache.mCache.clear();
+    purgeHbFaceCacheLocked();
 }
 
 }  // namespace android

engine/src/flutter/libs/minikin/MinikinInternal.cpp

@@ -18,8 +18,14 @@
 
 #include "MinikinInternal.h"
 
+#include <cutils/log.h>
+
 namespace android {
 
 Mutex gMinikinLock;
 
+void assertMinikinLocked() {
+    LOG_FATAL_IF(gMinikinLock.tryLock() == 0);
+}
+
 }

engine/src/flutter/libs/minikin/MinikinInternal.h

@@ -29,6 +29,9 @@ namespace android {
 
 extern Mutex gMinikinLock;
 
+// Aborts if gMinikinLock is not acquired. Do nothing on the release build.
+void assertMinikinLocked();
+
 }
 
 #endif  // MINIKIN_INTERNAL_H

engine/src/flutter/tests/Android.mk

@@ -43,12 +43,14 @@ LOCAL_SRC_FILES += \
     FontTestUtils.cpp \
     MinikinFontForTest.cpp \
     GraphemeBreakTests.cpp \
+    HbFaceCacheTest.cpp \
     LayoutUtilsTest.cpp \
     UnicodeUtils.cpp
 
 LOCAL_C_INCLUDES := \
     $(LOCAL_PATH)/../libs/minikin/ \
+    external/harfbuzz_ng/src \
     external/libxml2/include \
-    external/skia/src/core \
+    external/skia/src/core
 
 include $(BUILD_NATIVE_TEST)

engine/src/flutter/tests/HbFaceCacheTest.cpp

@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include "HbFaceCache.h"
+
+#include <cutils/log.h>
+#include <hb.h>
+#include <utils/Mutex.h>
+
+#include "MinikinInternal.h"
+#include <minikin/MinikinFont.h>
+
+namespace android {
+namespace {
+
+// A mock implementation of MinikinFont. The passed integer value will be
+// returned in GetUniqueId().
+class MockMinikinFont : public MinikinFont {
+public:
+    MockMinikinFont(int32_t id) : mId(id) {
+    }
+
+    virtual bool GetGlyph(uint32_t codepoint, uint32_t *glyph) const {
+        LOG_ALWAYS_FATAL("MockMinikinFont::GetGlyph is not implemented.");
+        return false;
+    }
+
+    virtual float GetHorizontalAdvance(
+            uint32_t glyph_id, const MinikinPaint &paint) const {
+        LOG_ALWAYS_FATAL("MockMinikinFont::GetHorizontalAdvance is not implemented.");
+        return 0.0f;
+    }
+
+    virtual void GetBounds(MinikinRect* bounds, uint32_t glyph_id,
+            const MinikinPaint &paint) const {
+        LOG_ALWAYS_FATAL("MockMinikinFont::GetBounds is not implemented.");
+    }
+
+    virtual bool GetTable(uint32_t tag, uint8_t *buf, size_t *size) {
+        LOG_ALWAYS_FATAL("MockMinikinFont::GetTable is not implemented.");
+        return false;
+    }
+
+    virtual int32_t GetUniqueId() const {
+        return mId;
+    }
+
+private:
+    int32_t mId;
+};
+
+class HbFaceCacheTest : public testing::Test {
+public:
+    virtual void TearDown() {
+        AutoMutex _l(gMinikinLock);
+        purgeHbFaceCacheLocked();
+    }
+};
+
+TEST_F(HbFaceCacheTest, getHbFaceLockedTest) {
+    AutoMutex _l(gMinikinLock);
+
+    MockMinikinFont fontA(1);
+    MockMinikinFont fontB(2);
+    MockMinikinFont fontC(2);
+
+    // Never return NULL.
+    EXPECT_TRUE(getHbFaceLocked(&fontA));
+    EXPECT_TRUE(getHbFaceLocked(&fontB));
+    EXPECT_TRUE(getHbFaceLocked(&fontC));
+
+    // Must return same object if same font object is passed.
+    EXPECT_EQ(getHbFaceLocked(&fontA), getHbFaceLocked(&fontA));
+    EXPECT_EQ(getHbFaceLocked(&fontB), getHbFaceLocked(&fontB));
+    EXPECT_EQ(getHbFaceLocked(&fontC), getHbFaceLocked(&fontC));
+
+    // Different object must be returned if the passed minikinFont has different ID.
+    EXPECT_NE(getHbFaceLocked(&fontA), getHbFaceLocked(&fontB));
+    EXPECT_NE(getHbFaceLocked(&fontA), getHbFaceLocked(&fontC));
+
+    // Same object must be returned if the minikinFont has same Id.
+    EXPECT_EQ(getHbFaceLocked(&fontB), getHbFaceLocked(&fontC));
+}
+
+TEST_F(HbFaceCacheTest, purgeCacheTest) {
+    AutoMutex _l(gMinikinLock);
+    MockMinikinFont font(1);
+
+    hb_face_t* face = getHbFaceLocked(&font);
+    EXPECT_TRUE(face);
+
+    // Set user data to identify the face object.
+    hb_user_data_key_t key;
+    void* data = (void*)0xdeadbeef;
+    hb_face_set_user_data(face, &key, data, NULL, false);
+    EXPECT_EQ(data, hb_face_get_user_data(face, &key));
+
+    purgeHbFaceCacheLocked();
+
+    // By checking user data, confirm that the object after purge is different from previously
+    // created one. Do not compare the returned pointer here since memory allocator may assign
+    // same region for new object.
+    face = getHbFaceLocked(&font);
+    EXPECT_EQ(nullptr, hb_face_get_user_data(face, &key));
+}
+
+}  // namespace
+}  // namespace android