Bug 195987 - [CSS OM] StyledElementInlineStylePropertyMap creates a Ref cycle with its owner element
Summary: [CSS OM] StyledElementInlineStylePropertyMap creates a Ref cycle with its own...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-19 21:44 PDT by Ryosuke Niwa
Modified: 2019-03-20 14:23 PDT (History)
6 users (show)

See Also:


Attachments
Fixes the bug (6.76 KB, patch)
2019-03-19 22:14 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Updated the change log (7.24 KB, patch)
2019-03-19 22:18 PDT, Ryosuke Niwa
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>