[Streams API] Turn WS states into integers and fix state initialization
Created attachment 264311 [details] Patch I wasn't sure if I should rework the rs states or keep them separated. I finally decided to go got reworking them though I don't have a string opinion about it. Turned ws states into integer values and also fixed the state not being initialized.
Comment on attachment 264311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264311&action=review > Source/WebCore/Modules/streams/ReadableStreamInternals.js:-329 > - // TODO: ASSERT(stream.@state !== @readableStreamErrored); If we should use FIXME, maybe change the ones that got changed accordingly > Source/WebCore/Modules/streams/WritableStream.js:195 > + } Might want to rewrite this with return and assert(false) > Source/WebCore/bindings/js/JSDOMWindowBase.cpp:95 > + GlobalPropertyInfo(static_cast<JSVMClientData*>(vm.clientData)->builtinNames().streamWritablePrivateName(), jsNumber(6), DontDelete | ReadOnly), Somehow we should automate this private enumeration step. That said, this seems good enough
(In reply to comment #2) > > Source/WebCore/Modules/streams/ReadableStreamInternals.js:-329 > > - // TODO: ASSERT(stream.@state !== @readableStreamErrored); > > If we should use FIXME, maybe change the ones that got changed accordingly I'll change the ones that appear in the patch, but I was planning to do a wider style change so I'll fix the others later. > > Source/WebCore/Modules/streams/WritableStream.js:195 > > + } > > Might want to rewrite this with return and assert(false) I thought of that too, but I liked it more this because assert(false) seems dirty. I could do write assert_not_reached. Please, let me know if you prefer to leave it as it is now or an assert_not_reached. > > Source/WebCore/bindings/js/JSDOMWindowBase.cpp:95 > > + GlobalPropertyInfo(static_cast<JSVMClientData*>(vm.clientData)->builtinNames().streamWritablePrivateName(), jsNumber(6), DontDelete | ReadOnly), > > Somehow we should automate this private enumeration step. > That said, this seems good enough I thought of that too when I was touching this.
Created attachment 264319 [details] Patch for landing
(In reply to comment #3) > > If we should use FIXME, maybe change the ones that got changed accordingly > > I'll change the ones that appear in the patch, but I was planning to do a > wider style change so I'll fix the others later. Changed the ones affected and things around for coherence. > > > Source/WebCore/Modules/streams/WritableStream.js:195 > > > + } > > > > Might want to rewrite this with return and assert(false) > > I thought of that too, but I liked it more this because assert(false) seems > dirty. I could do write assert_not_reached. Please, let me know if you > prefer to leave it as it is now or an assert_not_reached. Changed to return with assert_not_reached.
Comment on attachment 264319 [details] Patch for landing Clearing flags on attachment: 264319 Committed r191730: <http://trac.webkit.org/changeset/191730>
All reviewed patches have been landed. Closing bug.