Update WritableStream to new standard version

Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/830c24e023d24b353afe755aba47e5c67ad5cf5c

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change, ReadableStream has *not* been updated to the latest spec version.

BUG=658144, 626703
Review-Url: https://codereview.chromium.org/2772293002
Cr-Commit-Position: refs/heads/master@{#463954}
Committed: https://chromium.googlesource.com/chromium/src/+/3105d5e2ffe590989bbbda65e5b0e66a4a5049bd

[Adam Rice, 2017-03-27 12:16:14]

The CQ bit was checked by ricea@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 12:16:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Adam Rice, 2017-03-27 12:29:46]

+tyoshino, +domenic FYI

I'm not sending this for review yet. I currently plan to add Domenic's latest
standard changes first. If
https://github.com/w3c/web-platform-tests/commit/1e7798083b15d8df340598009e8f...
doesn't get rolled into Blink soon then I might reconsider and try to land this
CL before that one breaks the tests.

[Adam Rice, 2017-03-27 12:40:09]

The CQ bit was checked by ricea@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 12:40:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 12:43:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 12:43:32]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Adam Rice, 2017-03-31 14:56:47]

Description was changed from

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/253391df9a9588c696352e574494cd3d2646....

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo().

BUG=658144,626703
==========

to

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/990973cdc1a4617222406621a3e449b7f89f...

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change,
ReadableStream has *not* been updated to the latest spec version.

BUG=658144,626703
==========

[Adam Rice, 2017-03-31 14:57:04]

ricea@chromium.org changed reviewers:
+ domenic@chromium.org

[domenic, 2017-04-05 07:02:43]

lgtm with some nits.

You may want to change the commit target to
https://github.com/whatwg/streams/commit/830c24e023d24b353afe755aba47e5c67ad5...

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:25: const
_stateAndFlags = v8.createPrivateSymbol('[[stateAndFlags]]');
Nit: I might give this the name "[[state]] and flags" or something similar that
makes it clear it isn't an internal slot name from the spec.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:208: TEMP_ASSERT(state
=== WRITABLE,
We need to remember to remove the asserts before shipping given the perf issue
you discovered. Or I can try to work on that V8 extras bug to add debug-only
asserts...

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:233:
stream[_stateAndFlags] = (stream[_stateAndFlags] & ~STATE_MASK) | ERRORED;
Did you want to abstract out these things like you did elsewhere?
setState(stream, state) and getState(stream) I guess?

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:744: // when adding a
byte controller.
In particular private symbol lookup doesn't go through the prototype chain. Not
sure what a good solution is. A bad solution is if(IsX(...)) statements.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:799: value =
Function_apply(strategySize, undefined, [chunk]);
Function_call so you don't need to create the temporary array?

[Adam Rice, 2017-04-10 01:03:20]

Description was changed from

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/990973cdc1a4617222406621a3e449b7f89f...

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change,
ReadableStream has *not* been updated to the latest spec version.

BUG=658144,626703
==========

to

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/830c24e023d24b353afe755aba47e5c67ad5...

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change,
ReadableStream has *not* been updated to the latest spec version.

BUG=658144,626703
==========

[Adam Rice, 2017-04-10 01:04:04]

Commit target changed.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:25: const
_stateAndFlags = v8.createPrivateSymbol('[[stateAndFlags]]');
On 2017/04/05 07:02:42, domenic wrote:
> Nit: I might give this the name "[[state]] and flags" or something similar
that
> makes it clear it isn't an internal slot name from the spec.

Great idea, thanks.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:208: TEMP_ASSERT(state
=== WRITABLE,
On 2017/04/05 07:02:42, domenic wrote:
> We need to remember to remove the asserts before shipping given the perf issue
> you discovered. Or I can try to work on that V8 extras bug to add debug-only
> asserts...

I have filed an issue as a reminder:
https://bugs.chromium.org/p/chromium/issues/detail?id=709832

I will do it in a separate CL after I am done making changes.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:233:
stream[_stateAndFlags] = (stream[_stateAndFlags] & ~STATE_MASK) | ERRORED;
On 2017/04/05 07:02:42, domenic wrote:
> Did you want to abstract out these things like you did elsewhere?
> setState(stream, state) and getState(stream) I guess?

When I did this before I felt that it didn't improve readability. Together with
the additional motivation that nothing gets inlined until the optimising
compiler runs, I'd rather leave it as is.

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:744: // when adding a
byte controller.
On 2017/04/05 07:02:42, domenic wrote:
> In particular private symbol lookup doesn't go through the prototype chain.
Not
> sure what a good solution is. A bad solution is if(IsX(...)) statements.

One possibility would be to explicitly chase the prototype chain using
this.__proto__.prototype[AbortSteps]() or similar.

Another would be to place an explicit vtable on the object so calls would looks
like this[vtable][AbortSteps]().

https://codereview.chromium.org/2772293002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:799: value =
Function_apply(strategySize, undefined, [chunk]);
On 2017/04/05 07:02:42, domenic wrote:
> Function_call so you don't need to create the temporary array?

Done.

[Adam Rice, 2017-04-10 01:05:09]

ricea@chromium.org changed reviewers:
+ tyoshino@chromium.org

[Adam Rice, 2017-04-10 01:05:09]

+tyoshino for streams OWNERS.

[Adam Rice, 2017-04-10 04:19:34]

The CQ bit was checked by ricea@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 04:19:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 05:39:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 05:39:09]

Dry run: This issue passed the CQ dry run.

[tyoshino (SeeGerritForStatus), 2017-04-10 05:58:10]

On 2017/04/10 01:05:09, Adam Rice wrote:
> +tyoshino for streams OWNERS.

Will do.

Adam, please create a CL to add yourself to the OWNERS file :)

[tyoshino (SeeGerritForStatus), 2017-04-12 04:21:13]

lgtm

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:903:
ResetQueue(controller);
In the spec, only [[queue]] is cleared here. Was that intentional or we just
forgot to update the line in the spec?

[Adam Rice, 2017-04-12 06:02:48]

The CQ bit was checked by ricea@chromium.org

[Adam Rice, 2017-04-12 06:02:49]

The patchset sent to the CQ was uploaded after l-g-t-m from domenic@chromium.org
Link to the patchset: https://codereview.chromium.org/2772293002/#ps60001
(title: "Changes from domenic@ review")

[commit-bot: I haz the power, 2017-04-12 06:03:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 08:00:38]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491976968613410,
"parent_rev": "db77d9c68e2df6c5d5c626455a19d9a1aef76598", "commit_rev":
"3105d5e2ffe590989bbbda65e5b0e66a4a5049bd"}

[commit-bot: I haz the power, 2017-04-12 08:01:24]

Description was changed from

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/830c24e023d24b353afe755aba47e5c67ad5...

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change,
ReadableStream has *not* been updated to the latest spec version.

BUG=658144,626703
==========

to

==========
Update WritableStream to new standard version

This CL updates the WritableStream implementation from standard version
347667724b8a4b83fcc379b730603cf4cd0d6e91 to
https://github.com/whatwg/streams/commit/830c24e023d24b353afe755aba47e5c67ad5...

Most significant changes (all come from the standard):
* WritableStream abort logic clean up
* Sink abort() will no longer be called while sink start() is executing
* desiredSize calculation is now clamped to match the standard
* Protection against re-entrant strategy size() functions
* Many bug fixes

Implementation notes:
* WritableStream now has a [[backpressure]] flag.
  This is packed into the same field as the [[state]] field, which is
  now called _stateAndFlags.
* WritableStreamDefaultController now only has one flag, [[started]],
  so _defaultControllerFlags is gone and [[started]] has a field to
  itself.
* It does not appear to be possible to dispatch to a method named by a 
  V8 PrivateSymbol via the prototype chain, so static dispatch is used
  for [[AbortSteps]], [[ErrorSteps]] and [[StartSteps]] instead.
* A new V8 internal method v8.promiseState() is used to check the
  [[PromiseStatus]] field mentioned in the standard.

All streams/writable-streams web-platform-tests now pass; as a result
expectations are removed.

The obsolete copy of the web-platform-tests in
http/tests/streams/writable-streams is also gone.

The changed semantics of abort() necessitated a minor change in the
implementation of ReadableStream.pipeTo(). Apart from this change,
ReadableStream has *not* been updated to the latest spec version.

BUG=658144,626703

Review-Url: https://codereview.chromium.org/2772293002
Cr-Commit-Position: refs/heads/master@{#463954}
Committed:
https://chromium.googlesource.com/chromium/src/+/3105d5e2ffe590989bbbda65e5b0...
==========

[commit-bot: I haz the power, 2017-04-12 08:01:25]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3105d5e2ffe590989bbbda65e5b0...

[Adam Rice, 2017-04-13 05:54:40]

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:903:
ResetQueue(controller);
On 2017/04/12 04:21:13, tyoshino wrote:
> In the spec, only [[queue]] is cleared here. Was that intentional or we just
> forgot to update the line in the spec?

I think I intended to update the spec, but it looks like I didn't.

Since https://github.com/whatwg/streams/pull/721 removes this line anyway, it is
not worth worrying about.

[tyoshino (SeeGerritForStatus), 2017-04-13 07:38:07]

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2772293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:903:
ResetQueue(controller);
On 2017/04/13 05:54:40, Adam Rice wrote:
> On 2017/04/12 04:21:13, tyoshino wrote:
> > In the spec, only [[queue]] is cleared here. Was that intentional or we just
> > forgot to update the line in the spec?
> 
> I think I intended to update the spec, but it looks like I didn't.
> 
> Since https://github.com/whatwg/streams/pull/721 removes this line anyway, it
is
> not worth worrying about.

OK. Thanks