Closed Bug 1459698 Opened 3 years ago Closed 6 months ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: emilio, Assigned: nordzilla)

References

Details

Attachments

(1 file)

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?
Flags: needinfo?(emilio)
Priority: -- → P3
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.
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.
Yeah, this is not a problem for now, let's punt on it unless it becomes one.
Flags: needinfo?(emilio)
Component: DOM → DOM: Core & HTML
Assignee: nobody → enordin
Status: NEW → ASSIGNED
Attachment #9140273 - Attachment description: Bug 1459698 - Unlink DocumentOrShadowRoot::mStyleSheets r=emilio,smaug → Bug 1459698 - Unlink DocumentOrShadowRoot::mStyleSheets r=emilio
Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a5c41fc35b2
Unlink DocumentOrShadowRoot::mStyleSheets r=emilio
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.