Bug 221175 – StaticRange should keep its start and end containers alive

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ryosuke Niwa]

StaticRange doesn't keep its start and end containers alive right now.
As a result, some DOM nodes can get garbage collected prematurely.

[Ryosuke Niwa]

Created attachment 418807 [details]
Fixes the bug

[Wenson Hsieh]

Comment on attachment 418807 [details]
Fixes the bug

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

> Source/WebCore/ChangeLog:9
> +        To do that, we sff them as opaque roots during the marking phase in visitAdditionalChildren.

Nit - sff => add

[Ryosuke Niwa]

(In reply to Wenson Hsieh from comment #2)
> Comment on attachment 418807 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418807&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        To do that, we sff them as opaque roots during the marking phase in visitAdditionalChildren.
> 
> Nit - sff => add

Fixed.

Thanks for the review!

[Ryosuke Niwa]

Committed r272114: <https://trac.webkit.org/changeset/272114>

[Radar WebKit Bug Importer]

<rdar://problem/73788028>

[Darin Adler]

Should we add a test to make sure that we can’t create a reference cycle that causes leaks by having a static range as the value of a property on one of its endpoint nodes?

[Ryosuke Niwa]

(In reply to Darin Adler from comment #6)
> Should we add a test to make sure that we can’t create a reference cycle
> that causes leaks by having a static range as the value of a property on one
> of its endpoint nodes?

Maybe? I guess we can artificially introduce a leaking bug & verify the test works?

[Darin Adler]

We have tests for this, like LayoutTests/fast/dom/reference-cycle-leaks.html, but I don’t think we have one that covers StaticRange.

[Ryosuke Niwa]

(In reply to Darin Adler from comment #8)
> We have tests for this, like
> LayoutTests/fast/dom/reference-cycle-leaks.html, but I don’t think we have
> one that covers StaticRange.

Alright, filed https://bugs.webkit.org/show_bug.cgi?id=222786 to add those tests.