Ignore:
Timestamp:
Oct 2, 2012, 6:03:18 AM (13 years ago)
Author:
[email protected]
Message:

Refactor WebCore::FontData handling to clarify pointer ownership
​https://bugs.webkit.org/show_bug.cgi?id=95866

Reviewed by Eric Seidel.

Re-commit for a rolled-out original, now with Chromium Windows build fix.

This patch makes all FontData and derived classes ref-counted in all
code paths that lead to caching or other retention of a
pointer. The goal is to avert crashes and memory leaks, and to bring
the code more in line with current WebKit practices.

Specifically, this patch allows us to use ref pointers for all the
FontData stored in FontFallbackList objects. The FontFallbackList can
then own custom font data and manage its lifetime (forthcoming patch).
Currently Document owns custom font data and does an end run around
FontFallbackList in deleting glyph pages and custom font data, leaving
FontFallbackList with invalid pointers.

All FontData derived classes have been switched to use static create
methods with private constructors.

All caches that hold FontData now use RefPtrs.

All methods that construct new font data now return PassRefPtr, with the
exception of code only used to generate temporary data for text run layout.

All methods that handle FontData in a call stack that passes through
FontFallbackList::fontDataAt return PassRefPtr.

Performance tested with both WebKit Perf-o-matic, which showed
performance changes in the noise, and Chrome's page cycling tests with
the acid3 benchmark set, which showed no performance difference at all.

No new tests as this is refactoring code only and has no impact on functionality.

  • css/CSSFontFace.cpp:

(WebCore::CSSFontFace::getFontData):

  • css/CSSFontFace.h:

(CSSFontFace):

  • css/CSSFontFaceSource.cpp:

(WebCore::CSSFontFaceSource::getFontData):

  • css/CSSFontFaceSource.h:

(CSSFontFaceSource):

  • css/CSSFontSelector.cpp:

(WebCore::fontDataForGenericFamily):
(WebCore::CSSFontSelector::getFontData):

  • css/CSSFontSelector.h:
  • css/CSSSegmentedFontFace.cpp:

(WebCore::appendFontDataWithInvalidUnicodeRangeIfLoading):
(WebCore::CSSSegmentedFontFace::getFontData):

  • css/CSSSegmentedFontFace.h:

(CSSSegmentedFontFace):

  • dom/Document.cpp:

(WebCore::Document::registerCustomFont):

  • dom/Document.h:

(Document):

  • platform/graphics/Font.h:

(WebCore):

  • platform/graphics/FontCache.cpp:

(WebCore):
(WebCore::FontCache::getCachedFontData):
(WebCore::FontCache::getNonRetainedLastResortFallbackFont):
(WebCore::FontCache::releaseFontData):
(WebCore::FontCache::purgeInactiveFontData):
(WebCore::FontCache::getFontData):

  • platform/graphics/FontCache.h:

(FontCache):

  • platform/graphics/FontData.h:
  • platform/graphics/FontFallbackList.cpp:

(WebCore::FontFallbackList::releaseFontData):
(WebCore::FontFallbackList::fontDataAt):
(WebCore::FontFallbackList::setPlatformFont):

  • platform/graphics/FontFallbackList.h:

(FontFallbackList):

  • platform/graphics/FontFastPath.cpp:

(WebCore::Font::glyphDataAndPageForCharacter):

  • platform/graphics/FontSelector.h:

(FontSelector):

  • platform/graphics/GlyphPageTreeNode.cpp:

(WebCore::GlyphPageTreeNode::initializePage):

  • platform/graphics/SegmentedFontData.cpp:

(WebCore::SegmentedFontData::fontDataForCharacter):

  • platform/graphics/SegmentedFontData.h:

(WebCore::FontDataRange::FontDataRange):
(WebCore::FontDataRange::fontData):
(FontDataRange):
(WebCore::SegmentedFontData::create):
(SegmentedFontData):
(WebCore::SegmentedFontData::SegmentedFontData):

  • platform/graphics/SimpleFontData.cpp:

(WebCore::SimpleFontData::verticalRightOrientationFontData):
(WebCore::SimpleFontData::uprightOrientationFontData):
(WebCore::SimpleFontData::brokenIdeographFontData):

  • platform/graphics/SimpleFontData.h:

(WebCore::SimpleFontData::create):
(SimpleFontData):
(WebCore::SimpleFontData::variantFontData):
(DerivedFontData):

  • platform/graphics/chromium/FontCacheAndroid.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/chromium/FontCacheChromiumWin.cpp:

(WebCore::FontCache::fontDataFromDescriptionAndLogFont):
(GetLastResortFallbackFontProcData):
(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/chromium/SimpleFontDataChromiumWin.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/freetype/FontCacheFreeType.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/freetype/SimpleFontDataFreeType.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/mac/ComplexTextControllerCoreText.mm:

(WebCore::ComplexTextController::collectComplexTextRunsForCharacters):

  • platform/graphics/mac/FontCacheMac.mm:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/mac/FontComplexTextMac.cpp:

(WebCore::Font::fontDataForCombiningCharacterSequence):

  • platform/graphics/mac/SimpleFontDataMac.mm:

(WebCore::SimpleFontData::platformDestroy):
(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/pango/FontCachePango.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/pango/SimpleFontDataPango.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/qt/FontCacheQt.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/qt/SimpleFontDataQt.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/skia/FontCacheSkia.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/skia/SimpleFontDataSkia.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/win/FontCacheWin.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::fontDataFromDescriptionAndLogFont):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/win/SimpleFontDataWin.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/wince/FontCacheWinCE.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/wince/SimpleFontDataWinCE.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

  • platform/graphics/wx/FontCacheWx.cpp:

(WebCore::FontCache::getFontDataForCharacters):
(WebCore::FontCache::getSimilarFontPlatformData):
(WebCore::FontCache::getLastResortFallbackFont):

  • platform/graphics/wx/SimpleFontDataWx.cpp:

(WebCore::SimpleFontData::createScaledFontData):
(WebCore::SimpleFontData::smallCapsFontData):
(WebCore::SimpleFontData::emphasisMarkFontData):

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/css/CSSFontFaceSource.cpp

    r130105 r130160  
    3434#include "FontCache.h"
    3535#include "FontDescription.h"
    36 #include "GlyphPageTreeNode.h"
    3736#include "SimpleFontData.h"
    3837
    … …  
    9695}
    9796
    98 SimpleFontData* CSSFontFaceSource::getFontData(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic, CSSFontSelector* fontSelector)
     97PassRefPtr<SimpleFontData> CSSFontFaceSource::getFontData(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic, CSSFontSelector* fontSelector)
    9998{
    10099    // If the font hasn't loaded or an error occurred, then we've got nothing.
    … …  
    115114                       | (fontDescription.textOrientation() == TextOrientationUpright ? 8 : 0) | (fontDescription.orientation() == Vertical ? 4 : 0) | (syntheticBold ? 2 : 0) | (syntheticItalic ? 1 : 0);
    116115
    117     SimpleFontData*& cachedData = m_fontDataTable.add(hashKey, 0).iterator->second;
    118     if (cachedData)
    119         return cachedData;
    120 
    121     OwnPtr<SimpleFontData> fontData;
     116    RefPtr<SimpleFontData>& fontData = m_fontDataTable.add(hashKey, 0).iterator->second;
     117    if (fontData)
     118        return fontData; // No release, because fontData is a reference to a RefPtr that is held in the m_fontDataTable.
    122119
    123120    // If we are still loading, then we let the system pick a font.
    … …  
    158155                    }
    159156
    160                     fontData = adoptPtr(new SimpleFontData(SVGFontData::create(fontFaceElement), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic));
     157                    fontData = SimpleFontData::create(SVGFontData::create(fontFaceElement), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic);
    161158                }
    162159            } else
    … …  
    167164                    return 0;
    168165
    169                 fontData = adoptPtr(new SimpleFontData(m_font->platformDataFromCustomData(fontDescription.computedPixelSize(), syntheticBold, syntheticItalic, fontDescription.orientation(),
    170                                                                                    fontDescription.textOrientation(), fontDescription.widthVariant(), fontDescription.renderingMode()), true, false));
     166                fontData = SimpleFontData::create(m_font->platformDataFromCustomData(fontDescription.computedPixelSize(), syntheticBold, syntheticItalic,
     167                    fontDescription.orientation(), fontDescription.textOrientation(), fontDescription.widthVariant(), fontDescription.renderingMode()), true, false);
    171168            }
    172169        } else {
    … …  
    174171            // In-Document SVG Fonts
    175172            if (m_svgFontFaceElement)
    176                 fontData = adoptPtr(new SimpleFontData(SVGFontData::create(m_svgFontFaceElement.get()), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic));
     173                fontData = SimpleFontData::create(SVGFontData::create(m_svgFontFaceElement.get()), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic);
    177174#endif
    178175        }
    … …  
    185182        FontCachePurgePreventer fontCachePurgePreventer;
    186183        SimpleFontData* temporaryFont = fontCache()->getNonRetainedLastResortFallbackFont(fontDescription);
    187         fontData = adoptPtr(new SimpleFontData(temporaryFont->platformData(), true, true));
     184        fontData = SimpleFontData::create(temporaryFont->platformData(), true, true);
    188185    }
    189186
    190     if (Document* document = fontSelector->document()) {
    191         cachedData = fontData.get();
    192         document->registerCustomFont(fontData.release());
    193     }
    194 
    195     return cachedData;
     187    if (Document* document = fontSelector->document())
     188        document->registerCustomFont(fontData);
     189    else
     190        fontData.clear();
     191
     192    return fontData; // No release, because fontData is a reference to a RefPtr that is held in the m_fontDataTable.
    196193}
    197194
Note: See TracChangeset for help on using the changeset viewer.