Re: [whatwg/dom] MutationObserver invocation order (#713)

You are still wrong about this @domenic. I think it's time you start listening to what I am saying... you realize I am also a browser implementer right? I wouldn't start talking about this if I wasn't entirely sure of what I am saying...

So, yes, the nodes have a strong reference to their MO but this reference list is *not* cleared when a notification is dispatched; quite the opposite this is the list that is used to find out which notifications to dispatch in the first place, it is only ever cleared when an MO is disconnect()ed.

See the spec:
> To queue a mutation record of type for target with name, namespace, oldValue, addedNodes, removedNodes, previousSibling, and nextSibling, run these steps: 
> - Let interestedObservers be an empty map. 
> - Let nodes be the inclusive ancestors of target. 
> - For each node in *nodes*, and then for each registered observer of node’s *registered observer list*: 
>    - [determine if that MO should be added to interestedObservers]
> - [...]
> - Queue a mutation observer compound microtask. 
> 
> https://dom.spec.whatwg.org/#ref-for-registered-observer-list⑦


if this list was cleared, it could not be used to create the notifications in the first place. The list that is cleared after each notification is the "active MOs list" which holds the reference to the MOs until their notifications have been processed.

Now, to address your point that the nodes have a strong ref to the MOs, let's consider this practical case:

- Node `n` is in the document
- A mutation observer `m` is created to listen to node `n` to detect when it's being removed from the document
- Neither the node `n` nor the mutation observer `m` are stored in a javascript variable, this is quite common for this scenario
---> here are all the references at that point in Blink:
-------> `activeMOs` has a strong reference to an empty list: [] // no MO has a pending notification
-------> `n.registeredObservers` has a strong reference to MO `m` // it is used on every dom change to see if a MO must be notified of the change
-------> `m.registrations` has a weak reference to node `n` // it is used to remove `m` from `n.registeredObservers` on `disconnect()`

Okay, not let's say node `n` is removed from the document.

- This changes triggers the notification algorithm on node `n`, and it turns out there is an MO that listens to such change ( `m`)
- A mutation record is appended to `m` and a strong reference to `m` is pushed to `activeMOs`
 - Note that the mutation record has a strong reference to the node `n` that was mutated
- A task is qeued to process notifications
- Then `n` is removed from the document

At this point, `n` is no longer reference by any strong reference, except the one from the mutation record. It is not in any document, and javascript doesn't hold a reference to it. Now let's consider the MO. If it wasn't in the `activeMOs` list, it also wouldn't have any reference to it except the node. Indeed, it is not reference by JS and only the node stores a reference to it. We therefore have a closed cycle, and both the MO and the node can safely be garbage collected. 

This is wrong, because the MO should get to process the notification. For instance, it could process the notification, and reinsert the node in the document, keeping itself alive.

So, this doesn't happen in Blink because there is the `activeMOs` list which holds a strong reference to all MO that are currently pending a notification; this list is what prevents the MO from being garbage-collected, and therefore it also keeps the node alive, since the node must be alive to be exposed in the mutation record.

The model in the spec is incorrect because it doesn't have an `activeMOs` list, and instead has a global list of MOs. This global list cannot function properly, because either it has strong references to every MOs and they can never be garbage collected, or it holds a weak reference, and then they can be garbage collected while they have a pending notifications. See above message.

Please reread this until you get it, seriously.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/713#issuecomment-443976932