Bug 75948 – Matched declaration cache should support mapped attribute declarations.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

CSSStyleSelector's matched decl cache currently doesn't support mapped attribute declarations due to lifetime differences.

[Andreas Kling]

Created attachment 121817 [details]
Patch

[Antti Koivisto]

Comment on attachment 121817 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121817&action=review

r=me, nice!

> Source/WebCore/css/CSSStyleSelector.cpp:139
> +static unsigned gCacheAdditionsBetweenSweeps = 100;

A more complete name would be better. And drop the g. (matchedDeclarationCacheAdditionsBetweenSweeps)

> Source/WebCore/css/CSSStyleSelector.cpp:335
> +    , m_cacheAdditionsSinceLastSweep(0)

Use more complete name here too. It is not clear which cache is being referred.

> Source/WebCore/css/CSSStyleSelector.cpp:493
> +    // FIXME: This could be improved/replaced by some kind of invalidation mechanism.
> +    // Look for cache entries containing a style declaration with a single ref and remove them.
> +    // This may happen when an element attribute mutation causes it to swap out its Attribute::decl()
> +    // for another CSSMappedAttributeDeclaration, potentially leaving this cache with the last ref.

FIXME could be dropped as it is not clear if that would be any better (the alternative strategy could still be mentioned)

> Source/WebCore/css/CSSStyleSelector.cpp:504
> +    for (; it != end; ++it) {
> +        for (size_t i = 0; i < it->second.matchedStyleDeclarations.size(); ++i) {
> +            if (it->second.matchedStyleDeclarations[i].styleDeclaration->hasOneRef()) {
> +                toRemove.append(it->first);
> +                break;
> +            }
> +        }
> +    }

This might be less confusing if you assigned it->first and it->second to variables.

> Source/WebCore/css/CSSStyleSelector.h:348
> -    struct MatchedStyleDeclaration {
> +    class MatchedStyleDeclaration {

Better keep it struct since it has all public members.

> Source/WebCore/css/CSSStyleSelector.h:349
> +    public:

No need for public with struct.

> Source/WebCore/css/CSSStyleSelector.h:372
> +    unsigned m_cacheAdditionsSinceLastSweep;

Maybe move this field next to the cache field.

[Andreas Kling]

Committed r104550: <http://trac.webkit.org/changeset/104550>