Bug 94016 - Allow MutationEvents to be enabled/disabled per context
Summary: Allow MutationEvents to be enabled/disabled per context
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on: 94524
Blocks:
  Show dependency tree / graph
 
Reported: 2012-08-14 12:55 PDT by Adam Klein
Modified: 2012-08-20 18:15 PDT (History)
10 users (show)

See Also:

Attachments
Patch (9.47 KB, patch)
2012-08-14 13:03 PDT, Adam Klein 		no flags Details | Formatted Diff | Diff
Patch (8.58 KB, patch)
2012-08-14 16:01 PDT, Adam Klein 		no flags Details | Formatted Diff | Diff
Patch (8.96 KB, patch)
2012-08-17 16:31 PDT, Adam Klein 		no flags Details | Formatted Diff | Diff
Patch (9.30 KB, patch)
2012-08-20 15:37 PDT, Adam Klein 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2012-08-14 12:55:39 PDT 
Make MutationEvents a RuntimeEnabled feature
Comment 1 Elliott Sprehn 2012-08-14 13:00:58 PDT 
I think this might need to be a per document setting.
Comment 2 Adam Klein 2012-08-14 13:03:29 PDT 
Created attachment 158397 [details]
Patch
Comment 3 WebKit Review Bot 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.
Comment 4 Adam Klein 2012-08-14 13:05:53 PDT 
See http://crbug.com/142648 for the Chromium side of this patch.
Comment 5 Adam Barth 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
Comment 6 Adam Klein 2012-08-14 16:01:22 PDT 
Created attachment 158429 [details]
Patch
Comment 7 Ojan Vafai 2012-08-17 16:27:11 PDT 
Comment on attachment 158429 [details]
Patch

You missed some events. At the very least the DOMNodeRemoved* ones.
Comment 8 Adam Klein 2012-08-17 16:31:13 PDT 
Created attachment 159226 [details]
Patch
Comment 9 Adam Klein 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.
Comment 10 Ojan Vafai 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.
Comment 11 Ojan Vafai 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.
Comment 12 Adam Klein 2012-08-20 14:29:18 PDT 
Waiting for some nearby cleanup to carry out Ojan's suggestion.
Comment 13 Adam Klein 2012-08-20 15:37:06 PDT 
Created attachment 159541 [details]
Patch
Comment 14 Adam Barth 2012-08-20 17:26:05 PDT 
Comment on attachment 159541 [details]
Patch

API change LGTM
Comment 15 WebKit Review Bot 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>
Comment 16 WebKit Review Bot 2012-08-20 18:15:11 PDT 
All reviewed patches have been landed.  Closing bug.