Bug 195987

Summary: [CSS OM] StyledElementInlineStylePropertyMap creates a Ref cycle with its owner element
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: CSSAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, dino, graouts, koivisto, simon.fraser, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fixes the bug
none
Updated the change log simon.fraser: review+

Description Ryosuke Niwa 2019-03-19 21:44:41 PDT
There is a very obvious leak here.
StyledElementInlineStylePropertyMap ref's its element and Element Ref's the map in ElementRareData.

<rdar://problem/47254121>
Comment 1 Ryosuke Niwa 2019-03-19 22:14:43 PDT
Created attachment 365308 [details]
Fixes the bug
Comment 2 Ryosuke Niwa 2019-03-19 22:18:32 PDT
Created attachment 365309 [details]
Updated the change log
Comment 3 Simon Fraser (smfr) 2019-03-20 13:01:45 PDT
Comment on attachment 365309 [details]
Updated the change log

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

> Source/WebCore/dom/Element.cpp:209
> +        if (auto* map = elementRareData()->attributeStyleMap())
> +            map->clearElement();

Maybe ElementRareData should have a clearElement or clearNdoe function on it
Comment 4 Ryosuke Niwa 2019-03-20 13:14:00 PDT
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 365309 [details]
> Updated the change log
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365309&action=review
> 
> > Source/WebCore/dom/Element.cpp:209
> > +        if (auto* map = elementRareData()->attributeStyleMap())
> > +            map->clearElement();
> 
> Maybe ElementRareData should have a clearElement or clearNdoe function on it

That approach makes sense as well. Although I'm hoping that I can just add the support for WeakPtr<Element> and remove this code.
Comment 5 Ryosuke Niwa 2019-03-20 14:23:05 PDT
Committed r243239: <https://trac.webkit.org/changeset/243239>