[Streams API] Add write method to writable stream
Created attachment 264129 [details] Patch Writable stream functionality should be complete now
Comment on attachment 264129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264129&action=review Some nits > Source/WebCore/Modules/streams/WritableStream.js:117 > + if (this.@state === "closed" || this.@state === "closing") Can we use integer comparison like for RS and Promise? > Source/WebCore/Modules/streams/WritableStream.js:124 > + // assert(this.@state === "writable" || this.@state === "waiting" || this.@state === undefined); Why undefined? > Source/WebCore/Modules/streams/WritableStream.js:129 > + chunkSize = this.@strategy.size.@apply(undefined, [chunk]); Use @call instead? This may also apply to other WritableStream*.js places > Source/WebCore/Modules/streams/WritableStream.js:131 > + @errorWritableStream.@apply(this, [e]); Ditto > Source/WebCore/Modules/streams/WritableStream.js:139 > + @enqueueValueWithSize(this.@queue, writeRecord, chunkSize); Maybe inline writeRecord definition. > Source/WebCore/Modules/streams/WritableStream.js:141 > + @errorWritableStream.@apply(this, [e]); @apply -> @call
(In reply to comment #2) > > Source/WebCore/Modules/streams/WritableStream.js:117 > > + if (this.@state === "closed" || this.@state === "closing") > > Can we use integer comparison like for RS and Promise? This is interesting. The difference between RS and WS is that with WS the state is returned by the state attribute as a string. That's why I didn't use the integer comparison. If I did I'd still have to covert it when returning it in the attribute. I'll land this as it is now as it is and do this in a follow up patch if we think it is worth. TBH, I don't know which option can be better, probably the integer comparison. > > Source/WebCore/Modules/streams/WritableStream.js:124 > > + // assert(this.@state === "writable" || this.@state === "waiting" || this.@state === undefined); > > Why undefined? This is even more interesting. It seems that I didn't see until now the initialization of this@[[state]] = "writing" in the spec at the initialization and hence I didn't include it. This caused that in some we were hitting an undefined state. I'll fix this in a follow up, either with an entire rework of the state to make it integer or on its own. > > Source/WebCore/Modules/streams/WritableStream.js:129 > > + chunkSize = this.@strategy.size.@apply(undefined, [chunk]); > > Use @call instead? > This may also apply to other WritableStream*.js places > > > Source/WebCore/Modules/streams/WritableStream.js:131 > > + @errorWritableStream.@apply(this, [e]); > > Ditto > > > Source/WebCore/Modules/streams/WritableStream.js:141 > > + @errorWritableStream.@apply(this, [e]); > > @apply -> @call On it. > > Source/WebCore/Modules/streams/WritableStream.js:139 > > + @enqueueValueWithSize(this.@queue, writeRecord, chunkSize); > > Maybe inline writeRecord definition. I wrote it like this to be more literal to the spec, but I guess we can get it changed.
Created attachment 264209 [details] Patch for landing
Comment on attachment 264209 [details] Patch for landing Clearing flags on attachment: 264209 Committed r191669: <http://trac.webkit.org/changeset/191669>
All reviewed patches have been landed. Closing bug.