Bug 156955

return-type identifier(type identifier, optional type identifier = value);

Created attachment 277177 [details] Patch

Created attachment 277178 [details] Patch

Created attachment 277179 [details] Patch

Working with EWS to get all ports building.

Comment on attachment 277179 [details] Patch Attachment 277179 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1210734 New failing tests: imported/w3c/web-platform-tests/html/dom/interfaces.html

Created attachment 277183 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4

Created attachment 277184 [details] Patch

Comment on attachment 277184 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277184&action=review Nice work. > Source/WebCore/ChangeLog:30 > + These do not cause any Web-Exposed behavior change. However, when getting > + rid of [Default] started passing more parameters by derefence instead of > + pointer (as is expected for parameters not marked as nullable). As a result, > + I had to mark a few parameters as nullable in the IDL to maintain the > + previous behavior. In some cases, the implementation was already throwing > + when passing null. In such cases, I have not marked the type as nullable > + so that the generated bindings now throw the exception instead of our > + implementation code. In some cases, the exception being thrown is now > + different. This is why some of the layout tests had to be rebaselined. I would have expected this patch to include removal of the code in ShouldPassWrapperByReference in CodeGenerator.pm that looks at the Default extended attribute. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3582 > + $defaultValue = "nullptr" if $defaultValue eq "null"; > + $defaultValue = "PNaN" if $defaultValue eq "NaN"; I suggest putting these after the string related optimizations to avoid the peculiar use of nullptr in the string code. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3583 > + $defaultValue = GetNativeTypeFromSignature($parameter) . "()" if $defaultValue eq "[]"; Could we just use "{}" instead of doing this? > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3592 > + $defaultValue = $useAtomicString ? "AtomicString($defaultValue, AtomicString::ConstructFromLiteral)" : "ASCIILiteral($defaultValue)"; Should talk to Ben about these to be sure they are what we want. For example, if there is something performance critical going on it would be good to avoid allocating and then deleting every time. > Source/WebCore/dom/NodeFilter.idl:50 > #if defined(LANGUAGE_OBJECTIVE_C) && LANGUAGE_OBJECTIVE_C > - short acceptNode([Default=Undefined] optional Node n); > + short acceptNode(optional Node n); > #else > - [Custom] unsigned short acceptNode([Default=Undefined] optional Node n); > + [Custom] unsigned short acceptNode(optional Node n); > #endif We have to be able to do better than the name "n".

Comment on attachment 277184 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277184&action=review >> Source/WebCore/ChangeLog:30 >> + different. This is why some of the layout tests had to be rebaselined. > > I would have expected this patch to include removal of the code in ShouldPassWrapperByReference in CodeGenerator.pm that looks at the Default extended attribute. Will do. >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3582 >> + $defaultValue = "PNaN" if $defaultValue eq "NaN"; > > I suggest putting these after the string related optimizations to avoid the peculiar use of nullptr in the string code. Ok >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3583 >> + $defaultValue = GetNativeTypeFromSignature($parameter) . "()" if $defaultValue eq "[]"; > > Could we just use "{}" instead of doing this? That was my first attempt but it did not build. It complained about not being allowed to use an initializer list on the right side of a '?'. >> Source/WebCore/dom/NodeFilter.idl:50 >> #endif > > We have to be able to do better than the name "n". Ok.

Created attachment 277188 [details] Patch

Committed r199969: <http://trac.webkit.org/changeset/199969>