Bug 82832 – CSSStyleRules should own their CSSStyleDeclarations

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

[Antti Koivisto]

Move the rule properties CSSOM wrapper ownership from the StylePropertySet to the rule itself. This is preparation for bug 82728 (Split remaining CSSRules into internal and CSSOM types) easier to do. This temporarily grows the size of CSSStyleRule by a pointer (82728 will give the memory back and more).

[Antti Koivisto]

Created attachment 134961 [details]
patch

[Andreas Kling]

Comment on attachment 134961 [details]
patch

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

r=me with some sherlocking.

> Source/WebCore/css/PropertySetCSSStyleDeclaration.h:93
> +    ~StyleRuleCSSStyleDeclaration();

Should be marked virtual.

> Source/WebCore/css/PropertySetCSSStyleDeclaration.h:97
> +    virtual CSSRule* parentRule() const { return m_parentRule;  }
>      virtual void setNeedsStyleRecalc();
>      virtual CSSStyleSheet* contextStyleSheet() const;

We're missing some OVERRIDE decorations here.

[Antti Koivisto]

http://trac.webkit.org/changeset/112798

[Eric Seidel (no email)]

Comment on attachment 134961 [details]
patch

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

> Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:309
> +void StyleRuleCSSStyleDeclaration::ref()
> +{ 
> +    ++m_refCount;
> +}

Isn't this just RefCounted?

[Andreas Kling]

Comment on attachment 134961 [details]
patch

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

>> Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:309
>> +}
> 
> Isn't this just RefCounted?

ref() and deref() are virtual here, for this subclass it's a simple RefCounted workalike, for e.g PropertySetCSSStyleDeclaration it forwards the refs/derefs to the underlying StylePropertySet.