We should remove as much ReadableStream custom binding as possible.
Created attachment 255824 [details] Patch
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.
Created attachment 256008 [details] Using Default in IDL
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.
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.
Created attachment 256021 [details] Patch for landing
Comment on attachment 256021 [details] Patch for landing Clearing flags on attachment: 256021 Committed r186231: <http://trac.webkit.org/changeset/186231>
All reviewed patches have been landed. Closing bug.