386802 - Mutation events always fired during setting of innerHTML

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

The fix for bug 288392 made mutation events always fire if the document is "in the middle of dispatching a mutation event".  The same patch added mozAutoSubtreeModified objects around the code in nsNode3Tearoff::SetTextContent, nsGenericElement::Normalize, and nsGenericHTMLElement::SetInnerHTML.  This means that every single DOM operation performed by those functions fires a mutation event.

So for example, if I set the innerHTML on a <div> to some string a bunch of times in a row (as in bug 386769), we will fire a DOMNodeRemoved when the old textnode is removed and fire a DOMNodeInserted when the new one is inserted.  This slows down SetInnerHTML by 20% or so on that test.

I really don't think we want this perf hit in 1.9.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

argh.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch

This is a bit ugly, but should work well enough.
With the patch extra mutation events aren't dispatched just to get
right batching for DOMSubtreeModified. Instead of that, targets for the
possible mutation events are collected.
Speed ups the testcase significantly, though didn't profile.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 270880 [details] [diff] [review]
proposed patch

Makes sense.

We should add a test for this somewhere... perhaps Tdhtml or something?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Checked in.

Comment 5

[Mike Schroepfer]

Can confirm a significant improvement in that test (482 down to 251).   Nicely done!