Bug 94016

Summary: Allow MutationEvents to be enabled/disabled per context
Product: WebKit Reporter: Adam Klein <adamk>
Component: New BugsAssignee: Adam Klein <adamk>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, esprehn, fishd, jamesr, mihaip, ojan, rafaelw, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 94524    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Adam Klein
Reported 2012-08-14 12:55:39 PDT
Make MutationEvents a RuntimeEnabled feature
Attachments
Patch (9.47 KB, patch)
2012-08-14 13:03 PDT, Adam Klein 		no flags
DetailsFormatted DiffDiff
Patch (8.58 KB, patch)
2012-08-14 16:01 PDT, Adam Klein 		no flags
DetailsFormatted DiffDiff
Patch (8.96 KB, patch)
2012-08-17 16:31 PDT, Adam Klein 		no flags
DetailsFormatted DiffDiff
Patch (9.30 KB, patch)
2012-08-20 15:37 PDT, Adam Klein 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Elliott Sprehn
Comment 1 2012-08-14 13:00:58 PDT
I think this might need to be a per document setting.
Adam Klein
Comment 2 2012-08-14 13:03:29 PDT
Created attachment 158397 [details] Patch
WebKit Review Bot
Comment 3 2012-08-14 13:05:09 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Adam Klein
Comment 4 2012-08-14 13:05:53 PDT
See http://crbug.com/142648 for the Chromium side of this patch.
Adam Barth
Comment 5 2012-08-14 13:38:21 PDT
Comment on attachment 158397 [details] Patch I haven't verified that you've hooked all the right code points, but the rest of the patch LGTM
Adam Klein
Comment 6 2012-08-14 16:01:22 PDT
Created attachment 158429 [details] Patch
Ojan Vafai
Comment 7 2012-08-17 16:27:11 PDT
Comment on attachment 158429 [details] Patch You missed some events. At the very least the DOMNodeRemoved* ones.
Adam Klein
Comment 8 2012-08-17 16:31:13 PDT
Created attachment 159226 [details] Patch
Adam Klein
Comment 9 2012-08-17 16:32:02 PDT
(In reply to comment #7) > (From update of attachment 158429 [details]) > You missed some events. At the very least the DOMNodeRemoved* ones. Thanks, fixed. Dmitri, is there any way to test ContextFeatures from DRT? That would have easily caught this.
Ojan Vafai
Comment 10 2012-08-17 16:35:28 PDT
Comment on attachment 159226 [details] Patch How about hooking Document's hasListenerType method instead? It'd be much less code and easier to be sure you catch everything.
Ojan Vafai
Comment 11 2012-08-17 16:36:53 PDT
Or, even better, hook addListenerType to not add if the type is a mutation event type. addListenerType is called a lot less often and in much less hot codepaths is why this might be better than hooking hasListenerType.
Adam Klein
Comment 12 2012-08-20 14:29:18 PDT
Waiting for some nearby cleanup to carry out Ojan's suggestion.
Adam Klein
Comment 13 2012-08-20 15:37:06 PDT
Created attachment 159541 [details] Patch
Adam Barth
Comment 14 2012-08-20 17:26:05 PDT
Comment on attachment 159541 [details] Patch API change LGTM
WebKit Review Bot
Comment 15 2012-08-20 18:15:06 PDT
Comment on attachment 159541 [details] Patch Clearing flags on attachment: 159541 Committed r126113: <http://trac.webkit.org/changeset/126113>
WebKit Review Bot
Comment 16 2012-08-20 18:15:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.