Changeset 197434 in webkit


Ignore:
Timestamp:
Mar 1, 2016 6:34:50 PM (8 years ago)
Author:
mmaxfield@apple.com
Message:

Extend CSSFontSelector's lifetime to be longer than the Document's lifetime
https://bugs.webkit.org/show_bug.cgi?id=154101

Reviewed by Darin Adler.

Rather than destroying the Document's CSSFontSelector, instead, the object should
live for the lifetime of the document, and it should instead be asked to clear its
contents.

This is important for the CSS Font Loading API, where the identity of objects the
CSSFontSelector references needs to persist throughout the lifetime of the
Document. This patch represents the first step to implementing this correctly.
The second step is for the CSSFontSelector to perform a diff instead of a
wholesale clear of its contents. Once this is done, font loading objects can
survive through a call to Document::clearStyleResolver().

This patch gives the CSSFontSelector two states: building underway and building not
underway. The state is building underway in between calls to clearStyleResolver()
and when the style resolver gets built back up. Otherwise, the state is building
not underway. Because of this new design, creation of all FontFace objects can be
postponed until a state transition from building underway to building not underway.
A subsequent patch will perform the diff at this point. An ASSERT() makes sure that
we never service a font lookup request while Building.

No new tests because there is no behavior change.

  • css/CSSFontFaceSet.cpp:

(WebCore::CSSFontFaceSet::clear):

  • css/CSSFontSelector.cpp:

(WebCore::CSSFontSelector::buildStarted):
(WebCore::CSSFontSelector::buildCompleted):
(WebCore::CSSFontSelector::addFontFaceRule):
(WebCore::CSSFontSelector::fontRangesForFamily):
(WebCore::CSSFontSelector::CSSFontSelector): Deleted.
(WebCore::CSSFontSelector::clearDocument): Deleted.

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

(WebCore::StyleResolver::appendAuthorStyleSheets):

  • dom/Document.cpp:

(WebCore::Document::Document):
(WebCore::Document::~Document):
(WebCore::Document::clearStyleResolver):
(WebCore::Document::fontSelector): Deleted.

  • dom/Document.h:

(WebCore::Document::fontSelector):

Location:
trunk/Source/WebCore
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r197433 r197434  
     12016-03-01  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        Extend CSSFontSelector's lifetime to be longer than the Document's lifetime
     4        https://bugs.webkit.org/show_bug.cgi?id=154101
     5
     6        Reviewed by Darin Adler.
     7
     8        Rather than destroying the Document's CSSFontSelector, instead, the object should
     9        live for the lifetime of the document, and it should instead be asked to clear its
     10        contents.
     11
     12        This is important for the CSS Font Loading API, where the identity of objects the
     13        CSSFontSelector references needs to persist throughout the lifetime of the
     14        Document. This patch represents the first step to implementing this correctly.
     15        The second step is for the CSSFontSelector to perform a diff instead of a
     16        wholesale clear of its contents. Once this is done, font loading objects can
     17        survive through a call to Document::clearStyleResolver().
     18
     19        This patch gives the CSSFontSelector two states: building underway and building not
     20        underway. The state is building underway in between calls to clearStyleResolver()
     21        and when the style resolver gets built back up. Otherwise, the state is building
     22        not underway. Because of this new design, creation of all FontFace objects can be
     23        postponed until a state transition from building underway to building not underway.
     24        A subsequent patch will perform the diff at this point. An ASSERT() makes sure that
     25        we never service a font lookup request while Building.
     26
     27        No new tests because there is no behavior change.
     28
     29        * css/CSSFontFaceSet.cpp:
     30        (WebCore::CSSFontFaceSet::clear):
     31        * css/CSSFontSelector.cpp:
     32        (WebCore::CSSFontSelector::buildStarted):
     33        (WebCore::CSSFontSelector::buildCompleted):
     34        (WebCore::CSSFontSelector::addFontFaceRule):
     35        (WebCore::CSSFontSelector::fontRangesForFamily):
     36        (WebCore::CSSFontSelector::CSSFontSelector): Deleted.
     37        (WebCore::CSSFontSelector::clearDocument): Deleted.
     38        * css/CSSFontSelector.h:
     39        * css/StyleResolver.cpp:
     40        (WebCore::StyleResolver::appendAuthorStyleSheets):
     41        * dom/Document.cpp:
     42        (WebCore::Document::Document):
     43        (WebCore::Document::~Document):
     44        (WebCore::Document::clearStyleResolver):
     45        (WebCore::Document::fontSelector): Deleted.
     46        * dom/Document.h:
     47        (WebCore::Document::fontSelector):
     48
    1492016-03-01  Alexey Proskuryakov  <ap@apple.com>
    250
  • trunk/Source/WebCore/css/CSSFontFaceSet.cpp

    r196954 r197434  
    243243    m_locallyInstalledFacesLookupTable.clear();
    244244    m_cache.clear();
     245    m_facesPartitionIndex = 0;
     246    m_status = Status::Loaded;
    245247}
    246248
  • trunk/Source/WebCore/css/CSSFontSelector.cpp

    r196954 r197434  
    7171    , m_version(0)
    7272{
    73     // FIXME: An old comment used to say there was no need to hold a reference to m_document
    74     // because "we are guaranteed to be destroyed before the document". But there does not
    75     // seem to be any such guarantee.
    76 
    7773    ASSERT(m_document);
    7874    FontCache::singleton().addClient(*this);
     
    10298}
    10399
     100void CSSFontSelector::buildStarted()
     101{
     102    m_buildIsUnderway = true;
     103    ++m_version;
     104}
     105
     106void CSSFontSelector::buildCompleted()
     107{
     108    if (!m_buildIsUnderway)
     109        return;
     110
     111    m_buildIsUnderway = false;
     112
     113    m_cssFontFaceSet->clear();
     114
     115    for (auto& item : m_stagingArea)
     116        addFontFaceRule(item.styleRuleFontFace, item.isInitiatingElementInUserAgentShadowTree);
     117    m_stagingArea.clear();
     118}
     119
    104120void CSSFontSelector::addFontFaceRule(StyleRuleFontFace& fontFaceRule, bool isInitiatingElementInUserAgentShadowTree)
    105121{
     122    if (m_buildIsUnderway) {
     123        m_stagingArea.append({fontFaceRule, isInitiatingElementInUserAgentShadowTree});
     124        return;
     125    }
     126
    106127    const StyleProperties& style = fontFaceRule.properties();
    107128    RefPtr<CSSValue> fontFamily = style.getPropertyCSSValue(CSSPropertyFontFamily);
     
    235256FontRanges CSSFontSelector::fontRangesForFamily(const FontDescription& fontDescription, const AtomicString& familyName)
    236257{
     258    ASSERT(!m_buildIsUnderway); // If this ASSERT() fires, it usually means you forgot a document.updateStyleIfNeeded() somewhere.
     259
    237260    // FIXME: The spec (and Firefox) says user specified generic families (sans-serif etc.) should be resolved before the @font-face lookup too.
    238261    bool resolveGenericFamilyFirst = familyName == standardFamily;
     
    268291    m_document = nullptr;
    269292
    270     // FIXME: This object should outlive the Document.
    271293    m_cssFontFaceSet->clear();
    272294    m_clients.clear();
  • trunk/Source/WebCore/css/CSSFontSelector.h

    r196954 r197434  
    6666
    6767    void clearDocument();
     68    void buildStarted();
     69    void buildCompleted();
    6870
    6971    void addFontFaceRule(StyleRuleFontFace&, bool isInitiatingElementInUserAgentShadowTree);
     
    9294    void beginLoadTimerFired();
    9395
     96    struct PendingFontFaceRule {
     97        StyleRuleFontFace& styleRuleFontFace;
     98        bool isInitiatingElementInUserAgentShadowTree;
     99    };
     100    Vector<PendingFontFaceRule> m_stagingArea;
     101
    94102    Document* m_document;
    95103    RefPtr<FontFaceSet> m_fontFaceSet;
     
    103111    unsigned m_version;
    104112    bool m_creatingFont { false };
     113    bool m_buildIsUnderway { false };
    105114};
    106115
  • trunk/Source/WebCore/css/CSSStyleSheet.cpp

    r194496 r197434  
    424424
    425425CSSStyleSheet::RuleMutationScope::RuleMutationScope(CSSRule* rule)
    426     : m_styleSheet(rule ? rule->parentStyleSheet() : 0)
     426    : m_styleSheet(rule ? rule->parentStyleSheet() : nullptr)
    427427    , m_mutationType(OtherMutation)
    428428    , m_contentsWereClonedForMutation(ContentsWereNotClonedForMutation)
  • trunk/Source/WebCore/css/SourceSizeList.cpp

    r194496 r197434  
    7171    if (!view)
    7272        return 0;
    73     RenderStyle& style = view->style();
    74     for (auto& sourceSize : CSSParser(CSSStrictMode).parseSizesAttribute(sizesAttribute)) {
    75         if (match(WTFMove(sourceSize.expression), style, frame))
    76             return computeLength(sourceSize.length.get(), style, view);
     73    if (!sizesAttribute.empty()) {
     74        view->document().updateStyleIfNeeded();
     75        RenderStyle& style = view->style();
     76        for (auto& sourceSize : CSSParser(CSSStrictMode).parseSizesAttribute(sizesAttribute)) {
     77            if (match(WTFMove(sourceSize.expression), style, frame))
     78                return computeLength(sourceSize.length.get(), style, view);
     79        }
    7780    }
    7881    return defaultLength(style, view);
  • trunk/Source/WebCore/css/StyleResolver.cpp

    r197300 r197434  
    289289{
    290290    m_ruleSets.appendAuthorStyleSheets(styleSheets, m_medium.get(), m_inspectorCSSOMWrappers, this);
     291
     292    document().fontSelector().buildCompleted();
     293
    291294    if (auto renderView = document().renderView())
    292295        renderView->style().fontCascade().update(&document().fontSelector());
  • trunk/Source/WebCore/dom/Document.cpp

    r197429 r197434  
    540540    , m_templateDocumentHost(nullptr)
    541541#endif
     542    , m_fontSelector(CSSFontSelector::create(*this))
    542543#if ENABLE(WEB_REPLAY)
    543544    , m_inputCursor(EmptyInputCursor::create())
     
    573574    initSecurityContext();
    574575    initDNSPrefetch();
     576
     577    m_fontSelector->registerForInvalidationCallbacks(*this);
    575578
    576579    for (auto& nodeListAndCollectionCount : m_nodeListAndCollectionCounts)
     
    650653
    651654    clearStyleResolver(); // We need to destroy CSSFontSelector before destroying m_cachedResourceLoader.
     655    m_fontSelector->clearDocument();
     656    m_fontSelector->unregisterForInvalidationCallbacks(*this);
    652657
    653658    // It's possible for multiple Documents to end up referencing the same CachedResourceLoader (e.g., SVGImages
     
    21902195}
    21912196
    2192 CSSFontSelector& Document::fontSelector()
    2193 {
    2194     if (!m_fontSelector) {
    2195         m_fontSelector = CSSFontSelector::create(*this);
    2196         m_fontSelector->registerForInvalidationCallbacks(*this);
    2197     }
    2198     return *m_fontSelector;
    2199 }
    2200 
    22012197void Document::clearStyleResolver()
    22022198{
     
    22042200    m_userAgentShadowTreeStyleResolver = nullptr;
    22052201
    2206     // FIXME: It would be better if the FontSelector could survive this operation.
    2207     if (m_fontSelector) {
    2208         m_fontSelector->clearDocument();
    2209         m_fontSelector->unregisterForInvalidationCallbacks(*this);
    2210         m_fontSelector = nullptr;
    2211     }
     2202    m_fontSelector->buildStarted();
    22122203}
    22132204
  • trunk/Source/WebCore/dom/Document.h

    r197429 r197434  
    504504    StyleResolver& userAgentShadowTreeStyleResolver();
    505505
    506     CSSFontSelector& fontSelector();
     506    CSSFontSelector& fontSelector() { return m_fontSelector; }
    507507
    508508    void notifyRemovePendingSheetIfNeeded();
     
    17431743#endif
    17441744
    1745     RefPtr<CSSFontSelector> m_fontSelector;
     1745    Ref<CSSFontSelector> m_fontSelector;
    17461746
    17471747#if ENABLE(WEB_REPLAY)
  • trunk/Source/WebCore/style/StyleTreeResolver.cpp

    r197401 r197434  
    964964
    965965    Element* documentElement = m_document.documentElement();
    966     if (!documentElement)
    967         return;
     966    if (!documentElement) {
     967        m_document.ensureStyleResolver();
     968        return;
     969    }
    968970    if (change != Force && !documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
    969971        return;
Note: See TracChangeset for help on using the changeset viewer.