Bug 1809115 Comment 13 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)
> (In reply to Jens Stutte [:jstutte] from comment #11)
> > - `mozilla::dom::Document::ClearStaleServoData` - sounds a bit like just memory clearing? Can we defer that, too?
> 
> The servo data clearing may be removable nowadays, it was added in bug 1414999 but we no longer arena-allocate style structs on the pres shell so possibly fine. We might need to relax [this assert](https://searchfox.org/mozilla-central/rev/8011b6325f7ce05d228a3cdefd45d74fb98ee7b4/dom/base/Element.cpp#2073).

OK, I assume "removable" means deferable, not omitted entirely.

 > > - `nsFrameList::DestroyFrames` - unclear to me if that touches also things outside the frame tree? Probably yes?
> 
> Not sure what your question is exactly? We rely on frames to go away before unbind tho.

The question is if whatever the frames do on destruction has consequences only inside the same tree of things that get destroyed or if other things that will survive this `DestroyFrames` will get updated in between. So if "frames to go away before unbind" could just be "swap in an empty set and keep the old pointers alive elsewhere until deferred destruction".

> > - `nsIContent::UnbindFromTree` - running after `DestroyFrames` suggests it does not need the frames? Would it be enough to just swap in an empty frame list?
> 
> We need to destroy the frames before unbinding because we need the space of the frame pointer to store the subtree root pointer for unbound nodes.

I read this as "`DestroyFrames` updates structures that then are needed to correctly do `nsIContent::UnbindFromTree`, so we cannot defer the whole thing but would need to seperate the "update other things" part of `DestroyFrames` from the "destroy myself" part, which sounds like too much work to be just done.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)
> (In reply to Jens Stutte [:jstutte] from comment #11)
> > - `mozilla::dom::Document::ClearStaleServoData` - sounds a bit like just memory clearing? Can we defer that, too?
> 
> The servo data clearing may be removable nowadays, it was added in bug 1414999 but we no longer arena-allocate style structs on the pres shell so possibly fine. We might need to relax [this assert](https://searchfox.org/mozilla-central/rev/8011b6325f7ce05d228a3cdefd45d74fb98ee7b4/dom/base/Element.cpp#2073).

OK, I assume "removable" means deferable, not omitted entirely.

 > > - `nsFrameList::DestroyFrames` - unclear to me if that touches also things outside the frame tree? Probably yes?
> 
> Not sure what your question is exactly? We rely on frames to go away before unbind tho.

The question is if whatever the frames do on destruction has consequences only inside the same tree of things that get destroyed or if other things that will survive this `DestroyFrames` will get updated in between. So if "frames to go away before unbind" could just be "swap in an empty set and keep the old pointers alive elsewhere until deferred destruction".

> > - `nsIContent::UnbindFromTree` - running after `DestroyFrames` suggests it does not need the frames? Would it be enough to just swap in an empty frame list?
> 
> We need to destroy the frames before unbinding because we need the space of the frame pointer to store the subtree root pointer for unbound nodes.

I read this as "`DestroyFrames` updates structures that then are needed to correctly do `nsIContent::UnbindFromTree`", so we cannot defer the whole thing but would need to seperate the "update other things" part of `DestroyFrames` from the "destroy myself" part, which sounds like too much work to be just done.

Back to Bug 1809115 Comment 13