Bug 146458 – [Streams API] Remove ReadableStream and Reader cancel() custom binding

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

We should remove as much custom binding as possible

[youenn fablet]

Created attachment 255911 [details]
Patch

[youenn fablet]

Created attachment 256007 [details]
Using Default in IDL

[youenn fablet]

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.

[Darin Adler]

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.

[youenn fablet]

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

[youenn fablet]

Created attachment 256082 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 256082 [details]
Patch for landing

Clearing flags on attachment: 256082

Committed r186257: <http://trac.webkit.org/changeset/186257>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.