1459698 - Investigate if we could / should unlink the full stylesheet lists from {nsDocument,ShadowRoot} unlinking.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

The current setup is that we only traverse the stylesheets, the stylesheets clear their JS wrapper and other bits, and we rely on no other cycles being present, instead of clearing the mStyleSheets / mServoStyles references.

It's not totally clear to me why this is the case, and apparently Olli wasn't very excited about this either.

It looks that it's been this way since bug 333078... So presumably there's no strong reason for this?

Comment 1

[Boris Zbarsky [:bzbarsky]]

There's all sorts of stuff that nsDocument traverses but doesn't unlink.  See the comment at https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/dom/base/nsDocument.cpp#2061-2063 that used to be at the top of the unlink impl and now is buried somewhere in the middle...

Basically, I think Graydon was worried about the nsDocument destructor assuming that members are around in some indirect way.

That said, _if_ stylesheets have an unlink that breaks all outgoing references from them then this should not be a problem.  Of course they might not do that.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Of course Graydon's comment is really old, predating strong parentNode edges and what not, and we in general have better understanding how CC works in Gecko.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yeah, this is not a problem for now, let's punt on it unless it becomes one.

Comment 4

[Erik Nordin [:nordzilla]]

Attachment: Bug 1459698 - Unlink DocumentOrShadowRoot::mStyleSheets r=emilio

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a5c41fc35b2
Unlink DocumentOrShadowRoot::mStyleSheets r=emilio

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2a5c41fc35b2