We should remove as much custom binding as possible
Created attachment 255911 [details] Patch
Created attachment 256007 [details] Using Default in IDL
Comment on attachment 256007 [details] Using Default in IDL View in context: https://bugs.webkit.org/attachment.cgi?id=256007&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3246 > + push @optionalCallbackArguments, GenerateReturnParameters($function); With the use of Default=Undefined, the binding generator changes are not needed anymore to support cancel custom removal. But they are still useful in theory, hence why I kept it in this patch. I can remove these changes if needed or add a specific binding expectation test for it.
Comment on attachment 256007 [details] Using Default in IDL View in context: https://bugs.webkit.org/attachment.cgi?id=256007&action=review > Source/WebCore/Modules/streams/ReadableStream.idl:35 > + [RaisesException] Promise cancel([Default=Undefined] optional any reason); What default do we get if we don’t specify [Default=Undefined]? JavaScript null? C++ nullptr? >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3246 >> + push @optionalCallbackArguments, GenerateReturnParameters($function); > > With the use of Default=Undefined, the binding generator changes are not needed anymore to support cancel custom removal. > But they are still useful in theory, hence why I kept it in this patch. > I can remove these changes if needed or add a specific binding expectation test for it. Definitely should add a bindings test for this if you are keeping it.
Comment on attachment 256007 [details] Using Default in IDL View in context: https://bugs.webkit.org/attachment.cgi?id=256007&action=review >> Source/WebCore/Modules/streams/ReadableStream.idl:35 >> + [RaisesException] Promise cancel([Default=Undefined] optional any reason); > > What default do we get if we don’t specify [Default=Undefined]? JavaScript null? C++ nullptr? If no default is specified, the DOM class must provide methods with and without the argument, meaning in that case the two following methods: ReadableStream::cancel(JSPromiseDeferred*, ExceptionCode&); ReadableStream::cancel(JSC::JSValue, JSPromiseDeferred*, ExceptionCode&); >>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3246 >>> + push @optionalCallbackArguments, GenerateReturnParameters($function); >> >> With the use of Default=Undefined, the binding generator changes are not needed anymore to support cancel custom removal. >> But they are still useful in theory, hence why I kept it in this patch. >> I can remove these changes if needed or add a specific binding expectation test for it. > > Definitely should add a bindings test for this if you are keeping it. OK
Created attachment 256082 [details] Patch for landing
Comment on attachment 256082 [details] Patch for landing Clearing flags on attachment: 256082 Committed r186257: <http://trac.webkit.org/changeset/186257>
All reviewed patches have been landed. Closing bug.