231676 - Investigate implementing mutation events using a document-observer

Status: RESOLVED INACTIVE

(Core :: DOM: Core & HTML, defect, P5)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Right now we've implemented mutation-events by adding code everywhere where
events might need to be fired, such as in SetAttr and InsertChildAt that
constantly check to see if events needs to be fired.

A cleaner and probably faster (at least for pages that do not have
mutation-listeners) way to accomplish the same thing would be to use an
nsIDocumentObserver. When a mutation-eventlistener is registered we set up an
observer that keeps a hash of which nodes have listeners and also keeps track of
which types listeners that we have. The observer is then attached to the
document. So in other words, if we have no listeners we aren't spending a single
cycle on mutation-events.

The observer would then listen to all types of notification and create and send
off events as neccesary. We could still off course have all kinds of shortcuts
for when listeners for a specific type is implemented.

Using this scheme it should also be fairly easy to implement the mutation-events
that we don't implement yet.

There is however a downside. Attribute-modified events and
characterdata-modified contains the old nodevalues which we don't send to
nsIDocumentObservers currently. We could fix this in two ways:
1. Adding an argument to the AttributeChanged/ContentChanged notifications, or
2. Add AttributeWillChange/ContentWillChange notifications and let the
   mutation-observer grab the values at that time if it needs to.

I think the former alternative would be the slower one since we would need to
get the value off all attributes when we know we are going to send off a
notification. Which will be ignored by all observers other then the seldomly
used mutation-observer. Unfortunatly the latter alternative would probably be
uglier since the mutation-observer will have to deal with
*WillChange-notifiactions going off without there being a *Changed notification.

I still tend to lean towards the second alternative though, the ugliness will be
less then what we have now. Unless there are other observers that could use
this. Note that AttributeChanged or ContentChanged notifications aren't fired
during parsing and i so the hit on performance wouldn't be that bad.

Which brings me to my last point. This entire solution makes it impossible to
fire mutation-events reliably during parsing since we don't send out many
notifications then. I do think that this would be ok according to specs though.
We even have a bug 90983 filed on it.

Comments welcome.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note that attr changes can nest (so you can get three AttributeWillChange in a
row followed by three AttributeChanged).  So taking that path may be _very_ ugly.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yes, i am painfully aware of this :(

Also, a small correction, we do in fact call ContentChanged during loading. But
I think only on textruns longer then 4096 bytes. Maybe also on textruns that
happen to get chopped up in network traffic. In any event i *think* this is
uncommon enough that we don't need to worry about adding a ContentWillChange
notification.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

bz points out in bug 232009 that some of the mutationevents are supposed to fire
before the actual change takes place. This applies to DOMNodeRemoved and
DOMNodeRemovedFromDocument. However we fire off nsIDocumentObserver
notifications after removing nodes which is a big problem.

One solution is to add yet another notification, but we're starting to get a
whole lot of notifications just for the mutation-observer it seems. This is
mostly bad for performance-reasons since we'd send off more notifications that
noone is interested in.

We could maybe fix that by letting observers register for different families of
notifications, for example content-changeing notifications, style-changeing
notifications, load-related notifications and so on. We could have a
content-will-change family which basically only the mutation-observer would
register for for now.


Another solution to the problem is that we could change notifications to be
fired off before children are removed from the document. However that feels like
a scary change and we'd have to check with all current observers if they would
work with that.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ack, another problem is the ContentReplaced notification. I can't find anything
that specifies what mutation-events are supposed to be sent off for a
'replaceChild' action. The logical thing would be to regard it as a removal and
addition which means that we should fire events both before (DOMNodeRemoved) and
after (DOMNodeInserted) a replacement happens. But nsIDocumentObserver only has
a single notification which happens after.

This entire idea seems less and less possible :(

Comment 5

[Boris Zbarsky [:bzbarsky]]

The frame constructor really does want to be modified once the DOM tree _has_
changed (removal, addition, whatever).  Changing that would be highly nontrivial.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I suspected that would be the case. That leaves us with adding a slew (3) new
notifications, which might or might not be a big deal. Especially if we would
add 'families' of notifications that observers subscribe to. That would in
crease performance at the cost of memory-usage since we'd have several lists of
observers in each document.

Comment 7

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.