Add initial support for IDL union conversion
Created attachment 287913 [details] Patch
Let's see if this compiles anywhere else.
Attachment 287913 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Meta.h:43: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:52: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:53: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:53: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:61: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:89: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WTF/wtf/Meta.h:95: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:103: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:122: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:125: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:130: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:138: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:141: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:147: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:153: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:159: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:164: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:167: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:175: try_ is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Meta.h:179: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:182: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:190: try_ is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Meta.h:194: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:194: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:197: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:197: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:205: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:210: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:215: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:224: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:230: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:235: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:240: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:246: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:255: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:260: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:265: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:271: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:277: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:283: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:288: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:294: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:300: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:306: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:319: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:322: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:329: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:338: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:344: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:349: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:361: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:364: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:370: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:379: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:386: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:404: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:412: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:417: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:423: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:424: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:427: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:447: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:450: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:455: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:461: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:468: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:472: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:473: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:475: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:476: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:478: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:479: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:480: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:492: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:495: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:500: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:506: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Meta.h:507: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:516: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:519: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:529: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:531: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/Meta.h:535: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:535: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WTF/wtf/Meta.h:540: Do not use unnamed namespaces in header files. See http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces for more information. [build/namespaces] [4] ERROR: Source/WebCore/bindings/js/JSDOMConvert.h:327: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/bindings/js/JSDOMConvert.h:364: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/bindings/js/JSDOMConvert.h:364: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WTF/wtf/StdLibExtras.h:351: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:354: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:364: Missing space inside { }. [whitespace/braces] [5] Total errors found: 91 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 287913 [details] Patch Attachment 287913 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2007335 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 287916 [details] Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 287913 [details] Patch Attachment 287913 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2007336 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 287917 [details] Archive of layout-test-results from ews106 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 287913 [details] Patch As I feared, this won't work out of the box on Windows. I'm going to try porting to https://github.com/edouarda/brigand, which purports to have MSVC support.
Comment on attachment 287913 [details] Patch Attachment 287913 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2007344 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 287919 [details] Archive of layout-test-results from ews124 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Comment on attachment 287913 [details] Patch Attachment 287913 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2008689 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 287923 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 288077 [details] Patch
Let's try with the Brigand library instead, which claims MSVC support!
Comment on attachment 288077 [details] Patch Attachment 288077 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2021142 Number of test failures exceeded the failure limit.
Created attachment 288084 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288077 [details] Patch Attachment 288077 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2021157 Number of test failures exceeded the failure limit.
Created attachment 288085 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 288077 [details] Patch Attachment 288077 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2021158 Number of test failures exceeded the failure limit.
Created attachment 288086 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288077 [details] Patch Attachment 288077 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2021186 New failing tests: imported/w3c/web-platform-tests/dom/nodes/append-on-Document.html fast/dom/ChildNode-after.html imported/w3c/web-platform-tests/dom/nodes/ParentNode-append.html imported/w3c/web-platform-tests/dom/nodes/ChildNode-replaceWith.html imported/w3c/web-platform-tests/dom/nodes/ChildNode-before.html imported/w3c/web-platform-tests/dom/nodes/ParentNode-prepend.html userscripts/mixed-case-stylesheet.html js/dom/native-bindings-descriptors.html fast/dom/ParentNode-append.html fast/dom/Window/forbid-showModalDialog.html imported/w3c/web-platform-tests/dom/nodes/prepend-on-Document.html imported/w3c/web-platform-tests/dom/nodes/ChildNode-after.html fast/dom/ChildNode-replaceWith.html fast/dom/ParentNode-prepend.html fast/dom/ChildNode-before.html
Created attachment 288090 [details] Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Created attachment 288165 [details] Patch
Comment on attachment 288165 [details] Patch Attachment 288165 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2027823 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 288177 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288165 [details] Patch Attachment 288165 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2027830 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 288178 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 288165 [details] Patch Attachment 288165 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2027862 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 288179 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288165 [details] Patch Attachment 288165 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2027859 New failing tests: js/dom/native-bindings-descriptors.html
Created attachment 288181 [details] Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Created attachment 288188 [details] Patch
Comment on attachment 288188 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288188&action=review > Source/WebCore/bindings/js/JSDOMWrapper.h:35 > -static const uint8_t JSNodeType = JSC::LastJSCObjectType + 1; > +static const uint8_t JSDOMWrapperType = JSC::LastJSCObjectType + 1; > +static const uint8_t JSNodeType = JSC::LastJSCObjectType + 2; > static const uint8_t JSDocumentWrapperType = JSC::LastJSCObjectType + 2; > static const uint8_t JSElementType = JSC::LastJSCObjectType + 3; Oy! This is wrong. JSNodeType and JSDocumentWrapperType can't both be 2!
Created attachment 288206 [details] Patch
Comment on attachment 288206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288206&action=review I do not mind this approach although I personally find the resulting code a bit hard to read. I wonder why we cannot write more of this in Perl. > Source/WebCore/bindings/generic/IDLTypes.h:40 > +// IDLNull is a special type that serves as a base class for currently unsupported types. Do you mean IDLUnsupportedType ? > Source/WebCore/bindings/generic/IDLTypes.h:46 > +struct IDLAny : IDLUnsupportedType { }; In practice, I believe we use JSC::JSValue for IDL any type, no? > Source/WebCore/bindings/generic/IDLTypes.h:76 > +template<typename T> struct IDLInterfaceType : IDLType<Ref<T>> { I am not 100% clear why Interfaces use Ref<T> here and not RefPtr<T> or T*. > Source/WebCore/bindings/generic/IDLTypes.h:80 > +template<typename T> struct IDLDictionary : IDLType<Optional<T>> { Why is dictionary using Optional? > Source/WebCore/bindings/generic/IDLTypes.h:84 > +template<typename T> struct IDLEnumeration : IDLType<Optional<T>> { Why is enumeration using Optional? > Source/WebCore/bindings/generic/IDLTypes.h:92 > +template<typename T> struct IDLNullable : IDLType<Optional<typename T::ImplementationType>> { It seems odd to use Optional<> for nullable. If T is a wrapper type for e.g., we normally pass a raw pointer. For string types, we could pass a String (with null String() if null). > Source/WebCore/bindings/js/JSDOMConvert.h:136 > +template<> struct Converter<IDLDOMString> : DefaultConverter<String> { It is sad that we duplicate converters (i.e. Converter<IDLDOMString> & Converter<String>). > Source/WebCore/bindings/js/JSDOMConvert.h:345 > +struct Converter<IDLUnion<T...>> : DefaultConverter<typename IDLUnion<T...>::ImplementationType> It seems the algorithm implemented here in C++ is very similar to the one I implemented in Perl for the overload resolution algorithm. What are the benefits of writing this in C++ rather than in Perl? I may be missing something but it looks to me that writing this in C++ leads to more complicated code.
(In reply to comment #35) > Comment on attachment 288206 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288206&action=review > > I do not mind this approach although I personally find the resulting code a > bit hard to read. I wonder why we cannot write more of this in Perl. > > > Source/WebCore/bindings/generic/IDLTypes.h:40 > > +// IDLNull is a special type that serves as a base class for currently unsupported types. > > Do you mean IDLUnsupportedType ? Yes. > > > Source/WebCore/bindings/generic/IDLTypes.h:46 > > +struct IDLAny : IDLUnsupportedType { }; > > In practice, I believe we use JSC::JSValue for IDL any type, no? Yes. > > > Source/WebCore/bindings/generic/IDLTypes.h:76 > > +template<typename T> struct IDLInterfaceType : IDLType<Ref<T>> { > > I am not 100% clear why Interfaces use Ref<T> here and not RefPtr<T> or T*. For my initial purposes, (Node or DOMString) Ref<T> worked, but I could see it not being ideal in some places. I do think it should be the cononical non-null representation of an interface type. > > > Source/WebCore/bindings/generic/IDLTypes.h:80 > > +template<typename T> struct IDLDictionary : IDLType<Optional<T>> { > > Why is dictionary using Optional? Good point. > > > Source/WebCore/bindings/generic/IDLTypes.h:84 > > +template<typename T> struct IDLEnumeration : IDLType<Optional<T>> { > > Why is enumeration using Optional? Good point. > > > Source/WebCore/bindings/generic/IDLTypes.h:92 > > +template<typename T> struct IDLNullable : IDLType<Optional<typename T::ImplementationType>> { > > It seems odd to use Optional<> for nullable. If T is a wrapper type for > e.g., we normally pass a raw pointer. For string types, we could pass a > String (with null String() if null). We could certainly rework this as template<typename T> struct IDLNullable : IDLType<typename T::OptionalType> { }; and have each IDLType declare it's optional representation. I might like that. There was something nice about having it all be uniform, but, probably not practical. > > Source/WebCore/bindings/js/JSDOMConvert.h:136 > > +template<> struct Converter<IDLDOMString> : DefaultConverter<String> { > > It is sad that we duplicate converters (i.e. Converter<IDLDOMString> & > Converter<String>). The idea was to eventually only have Converter<IDLDOMString>, and remove Converter<String>. This would make it clear, when doing conversions, which type of IDL conversion you wanted (DOMString, USVString, ByteString) in the type. > > > Source/WebCore/bindings/js/JSDOMConvert.h:345 > > +struct Converter<IDLUnion<T...>> : DefaultConverter<typename IDLUnion<T...>::ImplementationType> > > It seems the algorithm implemented here in C++ is very similar to the one I > implemented in Perl for the overload resolution algorithm. > > What are the benefits of writing this in C++ rather than in Perl? I may be > missing something but it looks to me that writing this in C++ leads to more > complicated code. I think the biggest benefit is having all the conversion code in one place. We could have written lots of the converters in perl, but have opted for C++ to keep them together. Plus, it was really fun.
Created attachment 290385 [details] Patch
Comment on attachment 290385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290385&action=review r=me if the bots are happy with a couple of comments. > Source/WebCore/bindings/js/JSNodeOrString.cpp:44 > + using InterfaceTypeList = typename ConverterType::InterfaceTypeList; All this block seems unused? > Source/WebCore/bindings/js/JSNodeOrString.cpp:49 > + using StringTypeList = typename ConverterType::StringTypeList; All this block seems unused? > Source/WebCore/bindings/js/JSNodeOrString.cpp:59 > + Vector<typename NodeOrStringType::ImplementationType> result; This gets resolved as a Vector<std::experimental::variant<Ref<Node>, String>>, right?
Committed r206691: <http://trac.webkit.org/changeset/206691>