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.
Created attachment 254148 [details] Patch
Comment on attachment 254148 [details] Patch Andreas or Antti did this intentionally, perhaps to save memory. One of them needs to review this.
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.
(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 !