Closed Bug 231676 Opened 21 years ago Closed 4 years ago

Investigate implementing mutation events using a document-observer

Categories

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

defect

Tracking

()

RESOLVED INACTIVE

People

(Reporter: sicking, Unassigned)

References

Details

(Keywords: perf)

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.
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.
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.
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.
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 :(
The frame constructor really does want to be modified once the DOM tree _has_ changed (removal, addition, whatever). Changing that would be highly nontrivial.
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.
Assignee: general → nobody
QA Contact: ian → general
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.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.