- Revision
- 200237
- Author
- [email protected]
- Date
- 2016-04-29 02:57:49 -0700 (Fri, 29 Apr 2016)
Log Message
[FreeType] ASSERTION FAILED: !lookupForWriting(Extractor::extract(entry)).second in FontCache::getVerticalData()
https://bugs.webkit.org/show_bug.cgi?id=157132
Reviewed by Darin Adler.
I've noticed that some tests fail randomly in the GTK+ debug bot due to an assertion in HashMap when getting
vertical data from the FontCache. I don't know exactly what's wrong, but looks like a problem with the
FontVerticalDataCache hash traits implementation. Looking at the code, I've realized that we could simplify
everything by reusing the FontDataCache hash and traits, since we are actually using the
FontPlatformData::hash() in the end in both cases. Also, I don't see why we need to get the vertical data from
the FontPlatformData while it's actually cached by the font cache. We could just use the FontCache directly
passing only the FontPlatformData. These changes seem to fix the crashes and make the code a lot simpler.
* platform/graphics/Font.cpp:
(WebCore::Font::Font): Use FontCache::verticalData().
* platform/graphics/FontCache.cpp:
(WebCore::fontVerticalDataCache):
(WebCore::FontCache::verticalData):
(WebCore::FontCache::purgeInactiveFontData): Also remove the cached vertical data when removing a font.
(WebCore::FontCache::invalidate): Clear also the vertical data.
* platform/graphics/FontCache.h:
* platform/graphics/FontPlatformData.h:
* platform/graphics/freetype/FontPlatformDataFreeType.cpp:
(WebCore::FontPlatformData::openTypeTable): Deleted.
* platform/graphics/opentype/OpenTypeVerticalData.h: Remove the m_inFontCache member that is now unused.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (200236 => 200237)
--- trunk/Source/WebCore/ChangeLog 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/ChangeLog 2016-04-29 09:57:49 UTC (rev 200237)
@@ -1,3 +1,31 @@
+2016-04-29 Carlos Garcia Campos <[email protected]>
+
+ [FreeType] ASSERTION FAILED: !lookupForWriting(Extractor::extract(entry)).second in FontCache::getVerticalData()
+ https://bugs.webkit.org/show_bug.cgi?id=157132
+
+ Reviewed by Darin Adler.
+
+ I've noticed that some tests fail randomly in the GTK+ debug bot due to an assertion in HashMap when getting
+ vertical data from the FontCache. I don't know exactly what's wrong, but looks like a problem with the
+ FontVerticalDataCache hash traits implementation. Looking at the code, I've realized that we could simplify
+ everything by reusing the FontDataCache hash and traits, since we are actually using the
+ FontPlatformData::hash() in the end in both cases. Also, I don't see why we need to get the vertical data from
+ the FontPlatformData while it's actually cached by the font cache. We could just use the FontCache directly
+ passing only the FontPlatformData. These changes seem to fix the crashes and make the code a lot simpler.
+
+ * platform/graphics/Font.cpp:
+ (WebCore::Font::Font): Use FontCache::verticalData().
+ * platform/graphics/FontCache.cpp:
+ (WebCore::fontVerticalDataCache):
+ (WebCore::FontCache::verticalData):
+ (WebCore::FontCache::purgeInactiveFontData): Also remove the cached vertical data when removing a font.
+ (WebCore::FontCache::invalidate): Clear also the vertical data.
+ * platform/graphics/FontCache.h:
+ * platform/graphics/FontPlatformData.h:
+ * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
+ (WebCore::FontPlatformData::openTypeTable): Deleted.
+ * platform/graphics/opentype/OpenTypeVerticalData.h: Remove the m_inFontCache member that is now unused.
+
2016-04-29 Youenn Fablet <[email protected]>
Remove UsePointersEvenForNonNullableObjectArguments keyword
Modified: trunk/Source/WebCore/platform/graphics/Font.cpp (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/Font.cpp 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/Font.cpp 2016-04-29 09:57:49 UTC (rev 200237)
@@ -72,7 +72,7 @@
platformCharWidthInit();
#if ENABLE(OPENTYPE_VERTICAL)
if (platformData.orientation() == Vertical && !isTextOrientationFallback) {
- m_verticalData = platformData.verticalData();
+ m_verticalData = FontCache::singleton().verticalData(platformData);
m_hasVerticalGlyphs = m_verticalData.get() && m_verticalData->hasVerticalMetrics();
}
#endif
Modified: trunk/Source/WebCore/platform/graphics/FontCache.cpp (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/FontCache.cpp 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/FontCache.cpp 2016-04-29 09:57:49 UTC (rev 200237)
@@ -267,62 +267,6 @@
return it->value.get();
}
-#if ENABLE(OPENTYPE_VERTICAL)
-struct FontVerticalDataCacheKeyHash {
- static unsigned hash(const FontCache::FontFileKey& fontFileKey)
- {
- return PtrHash<const FontCache::FontFileKey*>::hash(&fontFileKey);
- }
-
- static bool equal(const FontCache::FontFileKey& a, const FontCache::FontFileKey& b)
- {
- return a == b;
- }
-
- static const bool safeToCompareToEmptyOrDeleted = true;
-};
-
-struct FontVerticalDataCacheKeyTraits : WTF::GenericHashTraits<FontCache::FontFileKey> {
- static const bool emptyValueIsZero = true;
- static const bool needsDestruction = true;
- static const FontCache::FontFileKey& emptyValue()
- {
- static NeverDestroyed<FontCache::FontFileKey> key = nullAtom;
- return key;
- }
- static void constructDeletedValue(FontCache::FontFileKey& slot)
- {
- new (NotNull, &slot) FontCache::FontFileKey(HashTableDeletedValue);
- }
- static bool isDeletedValue(const FontCache::FontFileKey& value)
- {
- return value.isHashTableDeletedValue();
- }
-};
-
-typedef HashMap<FontCache::FontFileKey, RefPtr<OpenTypeVerticalData>, FontVerticalDataCacheKeyHash, FontVerticalDataCacheKeyTraits> FontVerticalDataCache;
-
-FontVerticalDataCache& fontVerticalDataCacheInstance()
-{
- static NeverDestroyed<FontVerticalDataCache> fontVerticalDataCache;
- return fontVerticalDataCache;
-}
-
-PassRefPtr<OpenTypeVerticalData> FontCache::getVerticalData(const FontFileKey& key, const FontPlatformData& platformData)
-{
- FontVerticalDataCache& fontVerticalDataCache = fontVerticalDataCacheInstance();
- FontVerticalDataCache::iterator result = fontVerticalDataCache.find(key);
- if (result != fontVerticalDataCache.end())
- return result.get()->value;
-
- RefPtr<OpenTypeVerticalData> verticalData = OpenTypeVerticalData::create(platformData);
- if (!verticalData->isOpenType())
- verticalData = nullptr;
- fontVerticalDataCache.set(key, verticalData);
- return verticalData;
-}
-#endif
-
struct FontDataCacheKeyHash {
static unsigned hash(const FontPlatformData& platformData)
{
@@ -362,7 +306,26 @@
return cache;
}
+#if ENABLE(OPENTYPE_VERTICAL)
+typedef HashMap<FontPlatformData, RefPtr<OpenTypeVerticalData>, FontDataCacheKeyHash, FontDataCacheKeyTraits> FontVerticalDataCache;
+FontVerticalDataCache& fontVerticalDataCache()
+{
+ static NeverDestroyed<FontVerticalDataCache> fontVerticalDataCache;
+ return fontVerticalDataCache;
+}
+
+RefPtr<OpenTypeVerticalData> FontCache::verticalData(const FontPlatformData& platformData)
+{
+ auto addResult = fontVerticalDataCache().add(platformData, nullptr);
+ if (addResult.isNewEntry) {
+ RefPtr<OpenTypeVerticalData> data = ""
+ addResult.iterator->value = data->isOpenType() ? WTFMove(data) : nullptr;
+ }
+ return addResult.iterator->value;
+}
+#endif
+
#if PLATFORM(IOS)
const unsigned cMaxInactiveFontData = 120;
const unsigned cTargetInactiveFontData = 100;
@@ -440,6 +403,9 @@
for (auto& font : fontsToDelete) {
bool success = cachedFonts().remove(font->platformData());
ASSERT_UNUSED(success, success);
+#if ENABLE(OPENTYPE_VERTICAL)
+ fontVerticalDataCache().remove(font->platformData());
+#endif
}
};
@@ -452,30 +418,6 @@
for (auto& key : keysToRemove)
fontPlatformDataCache().remove(key);
-#if ENABLE(OPENTYPE_VERTICAL)
- FontVerticalDataCache& fontVerticalDataCache = fontVerticalDataCacheInstance();
- if (!fontVerticalDataCache.isEmpty()) {
- // Mark & sweep unused verticalData
- for (auto& verticalData : fontVerticalDataCache.values()) {
- if (verticalData)
- verticalData->m_inFontCache = false;
- }
- for (auto& font : cachedFonts().values()) {
- auto* verticalData = const_cast<OpenTypeVerticalData*>(font->verticalData());
- if (verticalData)
- verticalData->m_inFontCache = true;
- }
- Vector<FontFileKey> keysToRemove;
- keysToRemove.reserveInitialCapacity(fontVerticalDataCache.size());
- for (auto& it : fontVerticalDataCache) {
- if (!it.value || !it.value->m_inFontCache)
- keysToRemove.append(it.key);
- }
- for (auto& key : keysToRemove)
- fontVerticalDataCache.remove(key);
- }
-#endif
-
platformPurgeInactiveFontData();
}
@@ -531,6 +473,9 @@
}
fontPlatformDataCache().clear();
+#if ENABLE(OPENTYPE_VERTICAL)
+ fontVerticalDataCache().clear();
+#endif
invalidateFontCascadeCache();
gGeneration++;
Modified: trunk/Source/WebCore/platform/graphics/FontCache.h (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/FontCache.h 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/FontCache.h 2016-04-29 09:57:49 UTC (rev 200237)
@@ -209,8 +209,7 @@
#endif
#if ENABLE(OPENTYPE_VERTICAL)
- typedef AtomicString FontFileKey;
- PassRefPtr<OpenTypeVerticalData> getVerticalData(const FontFileKey&, const FontPlatformData&);
+ RefPtr<OpenTypeVerticalData> verticalData(const FontPlatformData&);
#endif
private:
Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2016-04-29 09:57:49 UTC (rev 200237)
@@ -162,7 +162,6 @@
#if USE(FREETYPE)
HarfBuzzFace* harfBuzzFace() const;
bool hasCompatibleCharmap() const;
- PassRefPtr<OpenTypeVerticalData> verticalData() const;
FcFontSet* fallbacks() const;
#endif
Modified: trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp 2016-04-29 09:57:49 UTC (rev 200237)
@@ -356,12 +356,6 @@
&& FT_Select_Charmap(freeTypeFace, ft_encoding_apple_roman));
}
-PassRefPtr<OpenTypeVerticalData> FontPlatformData::verticalData() const
-{
- ASSERT(hash());
- return FontCache::singleton().getVerticalData(String::number(hash()), *this);
-}
-
RefPtr<SharedBuffer> FontPlatformData::openTypeTable(uint32_t table) const
{
CairoFtFaceLocker cairoFtFaceLocker(m_scaledFont.get());
Modified: trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h (200236 => 200237)
--- trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h 2016-04-29 08:29:13 UTC (rev 200236)
+++ trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h 2016-04-29 09:57:49 UTC (rev 200237)
@@ -65,9 +65,6 @@
Vector<int16_t> m_topSideBearings;
int16_t m_defaultVertOriginY;
HashMap<Glyph, int16_t> m_vertOriginY;
-
- friend class FontCache;
- bool m_inFontCache; // for mark & sweep in FontCache::purgeInactiveFontData()
};
} // namespace WebCore
