Bug 148415

Summary: NodeFilter should be a callback interface
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, cgarcia, commit-queue, darin, ggaren, mark.lam, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://dom.spec.whatwg.org/#interface-nodefilter
See Also: https://bugs.webkit.org/show_bug.cgi?id=161030
Bug Depends on: 148418, 148434, 148449, 148531, 148591    
Bug Blocks:    
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2015-08-24 20:40:38 PDT 
NodeFilter should be a callback interface. Currently, it is a regular interface:
https://dom.spec.whatwg.org/#interface-nodefilter

One major impact if that TreeWalker.filter / NodeIterator is a JSNodeFilter wrapper instead of the JS function / object that was passed by the JavaScript.

Our bindings generator is unfortunately not currently able to handle such complex callback interface at the moment.

W3C Test Suites:
- http://w3c-test.org/dom/traversal/NodeIterator.html
- http://w3c-test.org/dom/traversal/TreeWalker.html
Comment 1 Chris Dumez 2015-08-30 18:07:13 PDT 
Created attachment 260261 [details]
WIP Patch
Comment 2 WebKit Commit Bot 2015-08-30 18:09:47 PDT 
Attachment 260261 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Chris Dumez 2015-08-30 18:16:43 PDT 
Created attachment 260262 [details]
WIP Patch
Comment 4 WebKit Commit Bot 2015-08-30 18:18:47 PDT 
Attachment 260262 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Chris Dumez 2015-08-30 18:33:29 PDT 
Created attachment 260263 [details]
WIP Patch
Comment 6 WebKit Commit Bot 2015-08-30 18:36:02 PDT 
Attachment 260263 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Chris Dumez 2015-08-30 19:07:37 PDT 
Created attachment 260264 [details]
WIP Patch
Comment 8 WebKit Commit Bot 2015-08-30 19:09:49 PDT 
Attachment 260264 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Chris Dumez 2015-08-30 19:26:24 PDT 
Carlos, I did as much as I could to fix the GTK build. However, I now get the following error:
../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp: In member function 'bool WebKitDOMNodeTest::testDOMCache(WebKitWebPage*)':
../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp:225:251: error: 'webkit_dom_document_create_node_iterator' was not declared in this scope
         GRefPtr<WebKitDOMNodeIterator> iter = adoptGRef(webkit_dom_document_create_node_iterator(document, WEBKIT_DOM_NODE(div), WEBKIT_DOM_NODE_FILTER_SHOW_ALL, nullptr, FALSE, nullptr));

I am not quite sure how to fix it.
Comment 10 Chris Dumez 2015-08-30 19:43:11 PDT 
Created attachment 260267 [details]
WIP Patch

Another attempt to fix the GTK build.
Comment 11 WebKit Commit Bot 2015-08-30 19:44:50 PDT 
Attachment 260267 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Chris Dumez 2015-08-30 20:05:04 PDT 
(In reply to comment #9)
> Carlos, I did as much as I could to fix the GTK build. However, I now get
> the following error:
> ../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp: In member
> function 'bool WebKitDOMNodeTest::testDOMCache(WebKitWebPage*)':
> ../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp:225:251: error:
> 'webkit_dom_document_create_node_iterator' was not declared in this scope
>          GRefPtr<WebKitDOMNodeIterator> iter =
> adoptGRef(webkit_dom_document_create_node_iterator(document,
> WEBKIT_DOM_NODE(div), WEBKIT_DOM_NODE_FILTER_SHOW_ALL, nullptr, FALSE,
> nullptr));
> 
> I am not quite sure how to fix it.

Actually, it looks like I managed to do it but please double-check the change to CodeGeneratorGObject.pm.
Comment 13 Chris Dumez 2015-08-30 20:28:40 PDT 
Created attachment 260270 [details]
Patch
Comment 14 WebKit Commit Bot 2015-08-30 20:30:07 PDT 
Attachment 260270 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:60:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:61:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:62:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:64:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:65:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:66:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:67:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:68:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:69:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 41 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Chris Dumez 2015-08-30 20:31:43 PDT 
The patch seems big mostly due to tests (w3c / bindings) rebaselining and large changelogs. The actual change is not *that* big.
Comment 16 Carlos Garcia Campos 2015-08-30 23:32:47 PDT 
(In reply to comment #12)
> (In reply to comment #9)
> > Carlos, I did as much as I could to fix the GTK build. However, I now get
> > the following error:
> > ../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp: In member
> > function 'bool WebKitDOMNodeTest::testDOMCache(WebKitWebPage*)':
> > ../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/DOMNodeTest.cpp:225:251: error:
> > 'webkit_dom_document_create_node_iterator' was not declared in this scope
> >          GRefPtr<WebKitDOMNodeIterator> iter =
> > adoptGRef(webkit_dom_document_create_node_iterator(document,
> > WEBKIT_DOM_NODE(div), WEBKIT_DOM_NODE_FILTER_SHOW_ALL, nullptr, FALSE,
> > nullptr));
> > 
> > I am not quite sure how to fix it.
> 
> Actually, it looks like I managed to do it but please double-check the
> change to CodeGeneratorGObject.pm.

Thanks Chris, I really appreciate it. Looks good to me, fortunately NodeIterator and TreeWalker interfaces in GObject DOM bindings are covered by our unit tests, so if it builds I'm pretty sure there are no API breaks and it will work.
Comment 17 Chris Dumez 2015-08-31 14:38:41 PDT 
Created attachment 260319 [details]
Patch
Comment 18 Chris Dumez 2015-08-31 15:06:07 PDT 
Created attachment 260321 [details]
Patch
Comment 19 WebKit Commit Bot 2015-08-31 15:07:35 PDT 
Attachment 260321 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 41 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 Geoffrey Garen 2015-08-31 17:21:06 PDT 
Comment on attachment 260321 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260321&action=review

JSCallbackData is a strong reference to a global object, which will create a retain cycle if the node iterator or tree walker is referenced by the global object.

I think we should fix this by having a kind of JSCallbackData that holds weak references, and is marked by the node iterator or tree walker wrapper.

> Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:2
>  /*
> - * Copyright (C) 2007, 2008, 2009 Apple Inc. All rights reserved.
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Did you remove these old dates on purpose?

> Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:38
> +// callback function calls if they return something else than a

else than => other than
Comment 21 Chris Dumez 2015-08-31 18:41:48 PDT 
Comment on attachment 260321 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260321&action=review

>> Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:2
>> + * Copyright (C) 2015 Apple Inc. All rights reserved.
> 
> Did you remove these old dates on purpose?

Yes, even though the file name is the same, all the previous code is gone.
Comment 22 Chris Dumez 2015-08-31 21:21:29 PDT 
Created attachment 260348 [details]
Patch
Comment 23 WebKit Commit Bot 2015-08-31 21:23:58 PDT 
Attachment 260348 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Chris Dumez 2015-08-31 21:32:35 PDT 
Comment on attachment 260348 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260348&action=review

Geoff, is this what you had in mind? I don't know much about JS lifetime management but I tried to match the previous implementation.

> Source/WebCore/bindings/js/JSCallbackData.cpp:95
> +    return visitor.containsOpaqueRoot(context);

This is our condition to keep it alive. (context will be a NodeFilter* in the case of NodeFilter).
The NodeFilter is added to the SlotVisitor's opaque roots via pre-exisinting JSTreeWalker::visitAdditionalChildren() in JSTreeWalkerCustom.cpp and JSNodeIterator::visitAdditionalChildren() in JSNodeIteratorCustom.cpp.

> Source/WebCore/bindings/js/JSCallbackData.h:46
>  class JSCallbackData {

JSCallbackData now has 2 subclasses:
- JSCallbackDataStrong which has a Strong pointer to the callback Object (used by all previous callbacks).
- JSCallbackDataWeak which has a Weak pointer to the callback Object (used by NodeFilter).

> Source/WebCore/bindings/js/JSCallbackData.h:50
> +protected:

Made the destructor protected as it is not virtual.

> Source/WebCore/bindings/js/JSCallbackData.h:117
> +    template <typename CallbackDataType>

Made this templated so we can call the subclass's destructor and avoid adding a vtable to JSCallbackData.

> Source/WebCore/bindings/js/JSNodeFilterCondition.cpp:-81
> -bool JSNodeFilterCondition::WeakOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, SlotVisitor& visitor)

This was moved to JSCallbackDataWeak.

> Source/WebCore/bindings/js/JSNodeFilterCondition.h:-47
> -        class WeakOwner : public JSC::WeakHandleOwner {

This was moved to JSCallbackDataWeak.

> Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:-33
> -void JSNodeFilter::visitAdditionalChildren(JSC::SlotVisitor& visitor)

JSNodeFilter is not longer a JS Wrapper so this is gone.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:198
> +    return $callbackInterface->extendedAttributes->{"IsWeakCallback"} ? "JSCallbackDataWeak" : "JSCallbackDataStrong";

In the generated bindings, we now use JSCallbackDataWeak / JSCallbackDataStrong as needed, based on the new "IsWeakCallback" IDL extended attribute (Couldn't think of a better name).
Comment 25 Geoffrey Garen 2015-09-01 13:20:31 PDT 
Comment on attachment 260348 [details]
Patch

r=me

Is the Windows EWs failure a problem?
Comment 26 Chris Dumez 2015-09-01 13:38:40 PDT 
(In reply to comment #25)
> Comment on attachment 260348 [details]
> Patch
> 
> r=me
> 
> Is the Windows EWs failure a problem?

Yes, it seems caused by my patch. I will fix it.
Comment 27 Chris Dumez 2015-09-01 13:43:30 PDT 
Created attachment 260379 [details]
Patch
Comment 28 WebKit Commit Bot 2015-09-01 13:45:37 PDT 
Attachment 260379 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/NodeFilter.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/NodeFilter.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 29 WebKit Commit Bot 2015-09-01 14:50:09 PDT 
Comment on attachment 260379 [details]
Patch

Clearing flags on attachment: 260379

Committed r189230: <http://trac.webkit.org/changeset/189230>
Comment 30 WebKit Commit Bot 2015-09-01 14:50:13 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 31 Chris Dumez 2015-09-02 13:18:58 PDT 
rdar://problem/22544293