[work in progress] Implement MutationListener/MutationTarget, a replacement for DOM Mutation Events
Created attachment 103650 [details] Patch
As noted in title, this is is a work in progress, and is neither itself a complete implementation nor small enough to be reviewed reasonably. Instead, it's meant to illustrate a WebKit implementation close to Jonas Sicking's proposal for a replacement to MutationEvents in http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/0369.html.
Also, pulled from the ChangeLog: Partially implements an experimental, vendor-prefixed API with the hope of eventually replacing MutationEvents. The API is based in large part on a proposal by Jonas Sicking in a message to public-webapps: http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/0369.html That thread is very long, but has helpfully been summarized by Rafael Weinstein in a recent message: http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/0779.html Note that this patch is in no way meant to be a complete implementation. The biggest missing feature is the actual dispatching of MutationRecordLists to MutationListeners: the included layout test instead calls a "spinMutationQueue" function to trigger notifications. This is not actually meant to be exposed to JS, though: my hope is to instead call spinMutationQueue from C++, either from somewhere in WebCore or (more likely) in the embedder. I have a separate change which implements the latter in Chromium. The other main thing missing from this patch is that MutationRecords are not dispatched in all the proper places, e.g., the handling of innerHTML is not as we hope it to be. This is simply a matter of wanting to put this patch up to get feedback: it shouldn't be much work to add MutationRecord dispatching in all the proper places.
I suggest sending a note about this work to webkit-dev. The idea of moving away from DOM mutation events is of course a generally accepted one, and there are many people who may want to participate in one way or another.
Created attachment 104260 [details] Patch
Comment on attachment 104260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104260&action=review This looks really cool. Why do apparently new files have licenses so divergent in terms of copyright holder, date and terms? > LayoutTests/fast/dom/mutation-listener.html:91 > + </script> Is it worth adding negative tests, eg when you’re listening to attribute mutation child mutation doesn’t fire the listener, etc.? And tests that merging work? etc. > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:483 > + push(@customInternalFields, "mutationListenerCacheIndex"); Why not just use the event listener cache? > Source/WebCore/dom/MutationRecord.cpp:41 > +// TODO(adamk): Figure out whether we want to share a single empty node list, or Sharing a single empty node list is a bad idea: Presumably want it to appear as a unique object to JavaScript (ie expandos stuck on in one place not appearing in another) and deciding when to create a wrapper could be tedious.
(In reply to comment #6) > (From update of attachment 104260 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104260&action=review > > This looks really cool. > > Why do apparently new files have licenses so divergent in terms of copyright holder, date and terms? Most should be new and marked Google 2011; the main case where this diverges is under bindings/js, where the classes are modified copies of JSEventListener. Not sure what the usual WebKit style is for this. > > LayoutTests/fast/dom/mutation-listener.html:91 > > + </script> > > Is it worth adding negative tests, eg when you’re listening to attribute mutation child mutation doesn’t fire the listener, etc.? And tests that merging work? etc. More tests are definitely in order, this file for now is a basic proof that _something_ is happening. As noted above, the implementation isn't complete, e.g., setting innerHTML doesn't do what it's supposed to (batching all the removals and insertions together into a single MutationRecord). > > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:483 > > + push(@customInternalFields, "mutationListenerCacheIndex"); > > Why not just use the event listener cache? Well, the biggest reason is that it doesn't exist on Node, since there's now an alternative way of handling refcounting for event listeners on nodes. In general I'm fairly unhappy with how much EventListener logic had to be duplicated in the bindings, so suggestions on how that could be reduced are welcome (I plan to look into some template-based approach when I get a chance). > > Source/WebCore/dom/MutationRecord.cpp:41 > > +// TODO(adamk): Figure out whether we want to share a single empty node list, or > > Sharing a single empty node list is a bad idea: Presumably want it to appear as a unique object to JavaScript (ie expandos stuck on in one place not appearing in another) and deciding when to create a wrapper could be tedious.
Created attachment 104930 [details] Replace MutationRecordList with an array of MutationRecords
Created attachment 105032 [details] Remove MutationTypes, use bitmasks everywhere
Created attachment 105241 [details] Make Document, DocumentFragment, and Element implement MutationTarget instead of Node
Created attachment 105544 [details] MutationRecord changes: addedCount, childlistIndex, and inDocument
Created attachment 105558 [details] MutationRecord: removed prevValue/newValue
Created attachment 105559 [details] MutationRecord: memory optimization
Created attachment 105685 [details] Merged to r94094
Created attachment 105727 [details] Coalesce childlist notifications for innerHTML etc
Created attachment 105845 [details] Created MutationScope.{h,cpp}, some API updates
Created attachment 106198 [details] Added tests and fixes for replaceChild, insertBefore, innerHTML
Created attachment 106218 [details] Test reorganization