Bug 148415

Summary:

NodeFilter should be a callback interface

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

DOM

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, cgarcia, commit-queue, darin, ggaren, mark.lam, rniwa, sam, webkit-bug-importer

Priority:

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

Chris Dumez

Reported 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

Attachments
WIP Patch (508.20 KB, patch) 2015-08-30 18:07 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (512.83 KB, patch) 2015-08-30 18:16 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (517.50 KB, patch) 2015-08-30 18:33 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (518.55 KB, patch) 2015-08-30 19:07 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (519.40 KB, patch) 2015-08-30 19:43 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (532.18 KB, patch) 2015-08-30 20:28 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (531.67 KB, patch) 2015-08-31 14:38 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (531.59 KB, patch) 2015-08-31 15:06 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (540.36 KB, patch) 2015-08-31 21:21 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (541.65 KB, patch) 2015-09-01 13:43 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2015-08-30 18:07:13 PDT

Created attachment 260261 [details] WIP Patch

WebKit Commit Bot

Comment 2 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.

Chris Dumez

Comment 3 2015-08-30 18:16:43 PDT

Created attachment 260262 [details] WIP Patch

WebKit Commit Bot

Comment 4 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.

Chris Dumez

Comment 5 2015-08-30 18:33:29 PDT

Created attachment 260263 [details] WIP Patch

WebKit Commit Bot

Comment 6 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.

Chris Dumez

Comment 7 2015-08-30 19:07:37 PDT

Created attachment 260264 [details] WIP Patch

WebKit Commit Bot

Comment 8 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.

Chris Dumez

Comment 9 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.

Chris Dumez

Comment 10 2015-08-30 19:43:11 PDT

Created attachment 260267 [details] WIP Patch Another attempt to fix the GTK build.

WebKit Commit Bot

Comment 11 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.

Chris Dumez

Comment 12 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.

Chris Dumez

Comment 13 2015-08-30 20:28:40 PDT

Created attachment 260270 [details] Patch

WebKit Commit Bot

Comment 14 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.

Chris Dumez

Comment 15 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.

Carlos Garcia Campos

Comment 16 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.

Chris Dumez

Comment 17 2015-08-31 14:38:41 PDT

Created attachment 260319 [details] Patch

Chris Dumez

Comment 18 2015-08-31 15:06:07 PDT

Created attachment 260321 [details] Patch

WebKit Commit Bot

Comment 19 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.

Geoffrey Garen

Comment 20 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

Chris Dumez

Comment 21 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.

Chris Dumez

Comment 22 2015-08-31 21:21:29 PDT

Created attachment 260348 [details] Patch

WebKit Commit Bot

Comment 23 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.

Chris Dumez

Comment 24 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).

Geoffrey Garen

Comment 25 2015-09-01 13:20:31 PDT

Comment on attachment 260348 [details] Patch r=me Is the Windows EWs failure a problem?

Chris Dumez

Comment 26 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.

Chris Dumez

Comment 27 2015-09-01 13:43:30 PDT

Created attachment 260379 [details] Patch

WebKit Commit Bot

Comment 28 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.

WebKit Commit Bot

Comment 29 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>

WebKit Commit Bot

Comment 30 2015-09-01 14:50:13 PDT

All reviewed patches have been landed. Closing bug.

Chris Dumez

Comment 31 2015-09-02 13:18:58 PDT

rdar://problem/22544293

Note You need to log in before you can comment on or make changes to this bug.