Align WritableStream structure with ReadableStream structure #462
Conversation
tyoshino
commented
May 31, 2016
tyoshino
commented
- Add WritableStreamDefaultWriter
- Add WritableStreamDefaultController
- Add lock related methods
- Update reader.pipeTo() to use the new WritableStream classes
- Add WritableStreamDefaultWriter - Add WritableStreamDefaultController - Add lock related methods - Update reader.pipeTo() to use the new WritableStream classes
|
This is WIP. I'll create some issues to discuss design decisions. |
|
|
|
|
I also started updating TransformStream. It should reveal issues of WritableStream if any. |
| } | ||
|
|
||
| return AcquireWritableStreamDefaultWriter(this); | ||
| } |
There was a problem hiding this comment.
Although I know there is a thread on abort(), for now (not having looked at that thread in detail yet) I think we should keep abort() as a lock-abort-unlock, similar to how cancel() on readable streams is lock-cancel-unlock.
There was a problem hiding this comment.
Oh, I see, you did that in the follow-up commit; nevermind!
|
Wow, thank you so much for taking this on. This is so great. I wonder what strategy you have in mind for doing this work. For example, IMO it would be valuable to fix writable streams in the ways we know we want to change, and get the spec + tests for them done. All of the stuff in the first two commits seems great to me. Then we could work on transform streams and other less-obvious fixes afterward. What do you think? I am open to doing everything all at once too if you'd prefer that. Another thing we need to do is convert the existing tests to web platform tests :(. Don't necessarily worry about that for now, but maybe if writing new tests try to keep them in WPT format and directories. I'll go off to comment on the other issues now! |
Yeah. There some experimental features are implemented in this PR, but they'll be reverted and this PR would just include only what we agreed e.g. writer introduction. |
|
All tests pass except for skipped ones which are related to the locking semantics. I'll fix them and then tweak it / fix style issues. |
| @@ -578,14 +578,15 @@ class WritableStreamDefaultController { | |||
| const startResult = InvokeOrNoop(underlyingSink, 'start', [this]); | |||
| Promise.resolve(startResult).then( | |||
| () => { | |||
| throw new assert.AssertionError('ss'); | |||
|
Moved to #488 to keep all the logs while making the PR rebased and squashed for easier review. |
