Changeset 200237 in webkit


Ignore:
Timestamp:
Apr 29, 2016, 2:57:49 AM (9 years ago)
Author:
Carlos Garcia Campos
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.
Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r200236 r200237  
     12016-04-29  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [FreeType] ASSERTION FAILED: !lookupForWriting(Extractor::extract(entry)).second in FontCache::getVerticalData()
     4        https://bugs.webkit.org/show_bug.cgi?id=157132
     5
     6        Reviewed by Darin Adler.
     7
     8        I've noticed that some tests fail randomly in the GTK+ debug bot due to an assertion in HashMap when getting
     9        vertical data from the FontCache. I don't know exactly what's wrong, but looks like a problem with the
     10        FontVerticalDataCache hash traits implementation. Looking at the code, I've realized that we could simplify
     11        everything by reusing the FontDataCache hash and traits, since we are actually using the
     12        FontPlatformData::hash() in the end in both cases. Also, I don't see why we need to get the vertical data from
     13        the FontPlatformData while it's actually cached by the font cache. We could just use the FontCache directly
     14        passing only the FontPlatformData. These changes seem to fix the crashes and make the code a lot simpler.
     15
     16        * platform/graphics/Font.cpp:
     17        (WebCore::Font::Font): Use FontCache::verticalData().
     18        * platform/graphics/FontCache.cpp:
     19        (WebCore::fontVerticalDataCache):
     20        (WebCore::FontCache::verticalData):
     21        (WebCore::FontCache::purgeInactiveFontData): Also remove the cached vertical data when removing a font.
     22        (WebCore::FontCache::invalidate): Clear also the vertical data.
     23        * platform/graphics/FontCache.h:
     24        * platform/graphics/FontPlatformData.h:
     25        * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
     26        (WebCore::FontPlatformData::openTypeTable): Deleted.
     27        * platform/graphics/opentype/OpenTypeVerticalData.h: Remove the m_inFontCache member that is now unused.
     28
    1292016-04-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
    230

  • trunk/Source/WebCore/platform/graphics/Font.cpp

    r200150 r200237  
    7373#if ENABLE(OPENTYPE_VERTICAL)
    7474    if (platformData.orientation() == Vertical && !isTextOrientationFallback) {
    75         m_verticalData = platformData.verticalData();
     75        m_verticalData = FontCache::singleton().verticalData(platformData);
    7676        m_hasVerticalGlyphs = m_verticalData.get() && m_verticalData->hasVerticalMetrics();
    7777    }

  • trunk/Source/WebCore/platform/graphics/FontCache.cpp

    r199890 r200237  
    268268}
    269269
    270 #if ENABLE(OPENTYPE_VERTICAL)
    271 struct FontVerticalDataCacheKeyHash {
    272     static unsigned hash(const FontCache::FontFileKey& fontFileKey)
    273     {
    274         return PtrHash<const FontCache::FontFileKey*>::hash(&fontFileKey);
    275     }
    276 
    277     static bool equal(const FontCache::FontFileKey& a, const FontCache::FontFileKey& b)
    278     {
    279         return a == b;
    280     }
    281 
    282     static const bool safeToCompareToEmptyOrDeleted = true;
    283 };
    284 
    285 struct FontVerticalDataCacheKeyTraits : WTF::GenericHashTraits<FontCache::FontFileKey> {
    286     static const bool emptyValueIsZero = true;
    287     static const bool needsDestruction = true;
    288     static const FontCache::FontFileKey& emptyValue()
    289     {
    290         static NeverDestroyed<FontCache::FontFileKey> key = nullAtom;
    291         return key;
    292     }
    293     static void constructDeletedValue(FontCache::FontFileKey& slot)
    294     {
    295         new (NotNull, &slot) FontCache::FontFileKey(HashTableDeletedValue);
    296     }
    297     static bool isDeletedValue(const FontCache::FontFileKey& value)
    298     {
    299         return value.isHashTableDeletedValue();
    300     }
    301 };
    302 
    303 typedef HashMap<FontCache::FontFileKey, RefPtr<OpenTypeVerticalData>, FontVerticalDataCacheKeyHash, FontVerticalDataCacheKeyTraits> FontVerticalDataCache;
    304 
    305 FontVerticalDataCache& fontVerticalDataCacheInstance()
    306 {
    307     static NeverDestroyed<FontVerticalDataCache> fontVerticalDataCache;
    308     return fontVerticalDataCache;
    309 }
    310 
    311 PassRefPtr<OpenTypeVerticalData> FontCache::getVerticalData(const FontFileKey& key, const FontPlatformData& platformData)
    312 {
    313     FontVerticalDataCache& fontVerticalDataCache = fontVerticalDataCacheInstance();
    314     FontVerticalDataCache::iterator result = fontVerticalDataCache.find(key);
    315     if (result != fontVerticalDataCache.end())
    316         return result.get()->value;
    317 
    318     RefPtr<OpenTypeVerticalData> verticalData = OpenTypeVerticalData::create(platformData);
    319     if (!verticalData->isOpenType())
    320         verticalData = nullptr;
    321     fontVerticalDataCache.set(key, verticalData);
    322     return verticalData;
    323 }
    324 #endif
    325 
    326270struct FontDataCacheKeyHash {
    327271    static unsigned hash(const FontPlatformData& platformData)
    … …  
    363307}
    364308
     309#if ENABLE(OPENTYPE_VERTICAL)
     310typedef HashMap<FontPlatformData, RefPtr<OpenTypeVerticalData>, FontDataCacheKeyHash, FontDataCacheKeyTraits> FontVerticalDataCache;
     311
     312FontVerticalDataCache& fontVerticalDataCache()
     313{
     314    static NeverDestroyed<FontVerticalDataCache> fontVerticalDataCache;
     315    return fontVerticalDataCache;
     316}
     317
     318RefPtr<OpenTypeVerticalData> FontCache::verticalData(const FontPlatformData& platformData)
     319{
     320    auto addResult = fontVerticalDataCache().add(platformData, nullptr);
     321    if (addResult.isNewEntry) {
     322        RefPtr<OpenTypeVerticalData> data = OpenTypeVerticalData::create(platformData);
     323        addResult.iterator->value = data->isOpenType() ? WTFMove(data) : nullptr;
     324    }
     325    return addResult.iterator->value;
     326}
     327#endif
    365328
    366329#if PLATFORM(IOS)
    … …  
    441404            bool success = cachedFonts().remove(font->platformData());
    442405            ASSERT_UNUSED(success, success);
     406#if ENABLE(OPENTYPE_VERTICAL)
     407            fontVerticalDataCache().remove(font->platformData());
     408#endif
    443409        }
    444410    };
    … …  
    453419        fontPlatformDataCache().remove(key);
    454420
    455 #if ENABLE(OPENTYPE_VERTICAL)
    456     FontVerticalDataCache& fontVerticalDataCache = fontVerticalDataCacheInstance();
    457     if (!fontVerticalDataCache.isEmpty()) {
    458         // Mark & sweep unused verticalData
    459         for (auto& verticalData : fontVerticalDataCache.values()) {
    460             if (verticalData)
    461                 verticalData->m_inFontCache = false;
    462         }
    463         for (auto& font : cachedFonts().values()) {
    464             auto* verticalData = const_cast<OpenTypeVerticalData*>(font->verticalData());
    465             if (verticalData)
    466                 verticalData->m_inFontCache = true;
    467         }
    468         Vector<FontFileKey> keysToRemove;
    469         keysToRemove.reserveInitialCapacity(fontVerticalDataCache.size());
    470         for (auto& it : fontVerticalDataCache) {
    471             if (!it.value || !it.value->m_inFontCache)
    472                 keysToRemove.append(it.key);
    473         }
    474         for (auto& key : keysToRemove)
    475             fontVerticalDataCache.remove(key);
    476     }
    477 #endif
    478 
    479421    platformPurgeInactiveFontData();
    480422}
    … …  
    532474
    533475    fontPlatformDataCache().clear();
     476#if ENABLE(OPENTYPE_VERTICAL)
     477    fontVerticalDataCache().clear();
     478#endif
    534479    invalidateFontCascadeCache();
    535480

  • trunk/Source/WebCore/platform/graphics/FontCache.h

    r196969 r200237  
    210210
    211211#if ENABLE(OPENTYPE_VERTICAL)
    212     typedef AtomicString FontFileKey;
    213     PassRefPtr<OpenTypeVerticalData> getVerticalData(const FontFileKey&, const FontPlatformData&);
     212    RefPtr<OpenTypeVerticalData> verticalData(const FontPlatformData&);
    214213#endif
    215214

  • trunk/Source/WebCore/platform/graphics/FontPlatformData.h

    r200129 r200237  
    163163    HarfBuzzFace* harfBuzzFace() const;
    164164    bool hasCompatibleCharmap() const;
    165     PassRefPtr<OpenTypeVerticalData> verticalData() const;
    166165    FcFontSet* fallbacks() const;
    167166#endif

  • trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp

    r200129 r200237  
    357357}
    358358
    359 PassRefPtr<OpenTypeVerticalData> FontPlatformData::verticalData() const
    360 {
    361     ASSERT(hash());
    362     return FontCache::singleton().getVerticalData(String::number(hash()), *this);
    363 }
    364 
    365359RefPtr<SharedBuffer> FontPlatformData::openTypeTable(uint32_t table) const
    366360{

  • trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h

    r189465 r200237  
    6666    int16_t m_defaultVertOriginY;
    6767    HashMap<Glyph, int16_t> m_vertOriginY;
    68 
    69     friend class FontCache;
    70     bool m_inFontCache; // for mark & sweep in FontCache::purgeInactiveFontData()
    7168};
    7269
Note: See TracChangeset for help on using the changeset viewer.