Bug 145593 – Make CSSStyleDeclaration to reference countable

• Component: CSS
• Status: RESOLVED INVALID

[Hyungwook Lee]

CSSStyleDeclaration class and its child classes have handled reference count manually. I think it would be good if we delegate the reference count to RefCount framework.

[Gyuyoung Kim]

Created attachment 254148 [details]
Patch

[Darin Adler]

Comment on attachment 254148 [details]
Patch

Andreas or Antti did this intentionally, perhaps to save memory. One of them needs to review this.

[Antti Koivisto]

Comment on attachment 254148 [details]
Patch

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

> Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:-129
> -void PropertySetCSSStyleDeclaration::ref()
> -{ 
> -    m_propertySet->ref();
> -}
> -
> -void PropertySetCSSStyleDeclaration::deref()
> -{
> -    m_propertySet->deref(); 
> -}

These types have custom refcounting so we can do this here. Basically the refcount of PropertySetCSSStyleDeclaration is delegated to the underlying property set. This scheme is used so CSSStyleDeclaration wrappers for property sets keep their script-observable identity.

> Source/WebCore/css/PropertySetCSSStyleDeclaration.h:76
> -    MutableStyleProperties* m_propertySet;
> +    RefPtr<MutableStyleProperties> m_propertySet;

This creates an ownership cycle. MutableStyleProperties has

std::unique_ptr<PropertySetCSSStyleDeclaration> m_cssomWrapper;

This will prevent it from dying and we'll leak MutableStyleProperties/PropertySetCSSStyleDeclaration pairs.

[Gyuyoung Kim]

(In reply to comment #3)
> Comment on attachment 254148 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254148&action=review
> 
> > Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:-129
> > -void PropertySetCSSStyleDeclaration::ref()
> > -{ 
> > -    m_propertySet->ref();
> > -}
> > -
> > -void PropertySetCSSStyleDeclaration::deref()
> > -{
> > -    m_propertySet->deref(); 
> > -}
> 
> These types have custom refcounting so we can do this here. Basically the
> refcount of PropertySetCSSStyleDeclaration is delegated to the underlying
> property set. This scheme is used so CSSStyleDeclaration wrappers for
> property sets keep their script-observable identity.
> 
> > Source/WebCore/css/PropertySetCSSStyleDeclaration.h:76
> > -    MutableStyleProperties* m_propertySet;
> > +    RefPtr<MutableStyleProperties> m_propertySet;
> 
> This creates an ownership cycle. MutableStyleProperties has
> 
> std::unique_ptr<PropertySetCSSStyleDeclaration> m_cssomWrapper;
> 
> This will prevent it from dying and we'll leak
> MutableStyleProperties/PropertySetCSSStyleDeclaration pairs.

Thank you for your review !