Bug 108229 – CSS matched properties cache exploding when mapped SVG attributes are updated frequently

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

[Florin Malita]

Created attachment 185308 [details]
Leaks memory, eventually crashes Chromium.

https://code.google.com/p/chromium/issues/detail?id=172221

Several issues at play here:

1) the matched properties cache is never swept:

StyleResolver::addToMatchedPropertiesCache():

    if (++m_matchedPropertiesCacheAdditionsSinceLastSweep >= matchedDeclarationCacheAdditionsBetweenSweeps) {
        static const unsigned matchedDeclarationCacheSweepTimeInSeconds = 60;
        m_matchedPropertiesCacheSweepTimer.startOneShot(matchedDeclarationCacheSweepTimeInSeconds);
    }

Since we keep updating the props, the timer keeps getting reset and never fires. I'll post a patch to leave the timer alone if already armed.

StyleResolver::matchAllRules(
2) matched SVG properties are cacheable

I'm not familiar with this area, but it seems strange that SVG attributes result in cacheable matched properties, while the equivalent CSS test (divs, background-color updates) yields non-cacheable matched properties.

For inline style we explicitly make a caching decision:
StyleResolver::matchAllRules(
        // Inline style is immutable as long as there is no CSSOM wrapper.
        // FIXME: Media control shadow trees seem to have problems with caching.
        bool isInlineStyleCacheable = !m_styledElement->inlineStyle()->isMutable() && !m_styledElement->isInShadowTree();

while for presentational attributes we never re-evaluate cacheability based on isMutable.

I don't understand enough of what's going on here to call it a bug, but it does seem a bit odd to have this asymmetry - someone should confirm that it's intended.


3) SVG matched properties do not get a deterministic matched properties hash key

This is because StyledElement::makePresentationAttributeCacheKey() doesn't support SVG attributes:

void StyledElement::makePresentationAttributeCacheKey(PresentationAttributeCacheKey& result) const
{    
    // FIXME: Enable for SVG.
    if (namespaceURI() != xhtmlNamespaceURI)
        return;

=> SVG StylePropertySets are effectively non-cacheable (in the presentation attribute cache)
=> rebuildPresentationAttributeStyle() always allocates a new StylePropertySet
=> the hash computed in StyleResolver::applyMatchedProperties() is always different (is based on MatchedProperties contents which bundles pointers to StylePropertySets)
=> SVG matched properties are always added to the matched properties cache, even if equivalent entries already exist

This probably needs fixing: add SVG support to StyledElement::makePresentationAttributeCacheKey() to enable proper SVG data hashing in both the presentation attribute cache and the matched properties cache.

[Florin Malita]

Andreas, Antti: I'm not familiar with styling and trying to understand what the correct high-level behavior should be here.

For a CSS test where I'm modifying 'background-color' on a <div> dynamically, the resulting MatchResult is non-cacheable because of the following snippet in StyleResolver::matchAllRules():

    // Now check our inline style attribute.
    if (m_matchAuthorAndUserStyles && m_styledElement && m_styledElement->inlineStyle()) {
        // Inline style is immutable as long as there is no CSSOM wrapper.
        // FIXME: Media control shadow trees seem to have problems with caching.
        bool isInlineStyleCacheable = !m_styledElement->inlineStyle()->isMutable() && !m_styledElement->isInShadowTree();
        // FIXME: Constify.
        addElementStyleProperties(result, m_styledElement->inlineStyle(), isInlineStyleCacheable);
    }


Is the intent to prevent caching of script-modifiable matched properties? That would make sense, since with a 1 min purge timer on the matched properties cache one could use up lots of memory otherwise.

But for SVG attributes there is no such mechanism - the resulting matched properties are cached unconditionally on the presentational attributes path:

    // Now check author rules, beginning first with presentational attributes mapped from HTML.
    if (m_styledElement) {
        addElementStyleProperties(result, m_styledElement->presentationAttributeStyle());


I guess the question is: should SVG attributes always yield non-cacheable matched properties?

[Brent Fulgham]

WebKit seems to use the least amount of memory or other resources (compared to Blink and Gecko).