Bug 146455 – [Streams API] Remove ReadableStreamController.enqueue() custom binding

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

[youenn fablet]

We should remove as much ReadableStream custom binding as possible.

[youenn fablet]

Created attachment 255824 [details]
Patch

[youenn fablet]

Comment on attachment 255824 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255824&action=review

> Source/WebCore/bindings/js/ReadableJSStream.cpp:392
> +        throwVMError(&state, error());

I thought initially about doing "ec = RangeError" here.
But the same exception must be thrown and stored in the stream.
I still see some value in introducing "RangeError" for other binding code.

> Source/WebCore/bindings/js/ReadableJSStream.h:66
>      void error(JSC::ExecState&, ExceptionCode&);

I wonder whether we should rename controller methods (enqueue, error and close), in particular error() which may be misleading with m_error getter.

[youenn fablet]

Created attachment 256008 [details]
Using Default in IDL

[Darin Adler]

Comment on attachment 256008 [details]
Using Default in IDL

View in context: https://bugs.webkit.org/attachment.cgi?id=256008&action=review

> Source/WebCore/bindings/js/ReadableJSStream.cpp:344
> +        throwVMError(&state, error());

This is messy. Having the same function sometimes set exceptions directly on ExecState and other times indicate the exception with an ExceptionCode is a bad idea.

> Source/WebCore/dom/ExceptionCode.h:68
> +        RangeError = 106,

If you add this new type, I think you need to add handling for it in a few places.

[youenn fablet]

Comment on attachment 256008 [details]
Using Default in IDL

View in context: https://bugs.webkit.org/attachment.cgi?id=256008&action=review

>> Source/WebCore/bindings/js/ReadableJSStream.cpp:344
>> +        throwVMError(&state, error());
> 
> This is messy. Having the same function sometimes set exceptions directly on ExecState and other times indicate the exception with an ExceptionCode is a bad idea.

OK

>> Source/WebCore/dom/ExceptionCode.h:68
>> +        RangeError = 106,
> 
> If you add this new type, I think you need to add handling for it in a few places.

I plan to reuse it in ReadableJSStream::create().
I will also add it in some binding places where the range message is not very meaningful.

[youenn fablet]

Created attachment 256021 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 256021 [details]
Patch for landing

Clearing flags on attachment: 256021

Committed r186231: <http://trac.webkit.org/changeset/186231>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.