Update writable streams for the latest spec changes

Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

- https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0fd12c
- https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee558892797c33
- https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9e256
- https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d5867
- https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5441c
- https://github.com/whatwg/streams/commit/43c4b00d440b3c391a48fb761c3967c2e77d6cd1
- https://github.com/whatwg/streams/commit/22b3f03bf803868bf65d9b57f862cc7e3cc80188

BUG=671958
Review-Url: https://codereview.chromium.org/2596883002
Cr-Commit-Position: refs/heads/master@{#441974}
Committed: https://chromium.googlesource.com/chromium/src/+/2d69d55423fa357963f4e5bbaf8b5ab44075a340

[domenic, 2016-12-21 16:55:20]

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

[domenic, 2016-12-21 16:55:20]

https://github.com/whatwg/streams/pull/634 and
https://github.com/whatwg/streams/issues/633 may cause some cleanups but I
thought it was worthwhile getting up to speed with the latest spec first

[domenic, 2016-12-21 18:54:09]

Description was changed from

==========
Update writable streams for the latest spec changes

This incorporates the spec changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...

and their associated test collateral.

BUG=671958
==========

to

==========
Update writable streams for the latest spec changes

This incorporates the spec changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...

and their associated test collateral.

BUG=671958
==========

[domenic, 2016-12-21 18:54:25]

Description was changed from

==========
Update writable streams for the latest spec changes

This incorporates the spec changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...

and their associated test collateral.

BUG=671958
==========

to

==========
Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...

BUG=671958
==========

[Adam Rice, 2017-01-05 11:48:24]

lgtm with nits, thanks.

I assume you just copied the tests so I haven't reviewed them in detail.

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

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:165: class
WritableStreamInternalError extends Error {
I actually had a reason for not inheriting from Error, in that I didn't want any
code thinking it knew how to handle this exception.

Having said that, all the times I've seen an assertion the exception got caught
somewhere and so wasn't useful anyway. I didn't try to add a mechanism to pass
on the assertions like the reference implementation has because it's not worth
the complexity for a temporary debugging feature.

So if this is useful to you, then keep it.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:247:
TEMP_ASSERT(controller !== undefined);
Please add ", 'controller is not undefined.'"

I really do find these messages extremely useful when asserts happen.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:256:
WritableStreamDefaultControllerAbort(controller, reason);
Needs a return statement.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:287:
(getDefaultControllerWritingFlag(controller) === false &&
I would prefer just `!getDefaultControllerWritingFlag(controller)` as there's no
need to be explicit about the type here.

Also " === true" below.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:313: TEMP_ASSERT(state
=== ERRORED);
Message please.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:735:
v8.rejectPromise(stream[_pendingAbortRequest]);
I think we need to supply r as the second argument to rejectPromise here.

[Adam Rice, 2017-01-05 11:52:39]

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

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:61: const Function_call
= v8.uncurryThis(global.Function.prototype.call);
I think this can be removed now.

[domenic, 2017-01-05 21:43:46]

Description was changed from

==========
Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...

BUG=671958
==========

to

==========
Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...
-
https://github.com/whatwg/streams/commit/43c4b00d440b3c391a48fb761c3967c2e77d...
-
https://github.com/whatwg/streams/commit/22b3f03bf803868bf65d9b57f862cc7e3cc8...

BUG=671958
==========

[domenic, 2017-01-05 21:51:00]

The CQ bit was checked by domenic@chromium.org

[domenic, 2017-01-05 21:51:01]

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

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:165: class
WritableStreamInternalError extends Error {
On 2017/01/05 at 11:48:24, Adam Rice wrote:
> I actually had a reason for not inheriting from Error, in that I didn't want
any code thinking it knew how to handle this exception.
> 
> Having said that, all the times I've seen an assertion the exception got
caught somewhere and so wasn't useful anyway. I didn't try to add a mechanism to
pass on the assertions like the reference implementation has because it's not
worth the complexity for a temporary debugging feature.
> 
> So if this is useful to you, then keep it.

Yeah, in debugging in content_shell this was quite useful in the end.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:256:
WritableStreamDefaultControllerAbort(controller, reason);
On 2017/01/05 at 11:48:24, Adam Rice wrote:
> Needs a return statement.

The fact that no tests caught this is worrying. Let me add one.

https://codereview.chromium.org/2596883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:735:
v8.rejectPromise(stream[_pendingAbortRequest]);
On 2017/01/05 at 11:48:24, Adam Rice wrote:
> I think we need to supply r as the second argument to rejectPromise here.

More test coverage gaps!

[domenic, 2017-01-05 21:51:01]

The patchset sent to the CQ was uploaded after l-g-t-m from ricea@chromium.org
Link to the patchset: https://codereview.chromium.org/2596883002/#ps40001
(title: "Fix nits with tests")

[commit-bot: I haz the power, 2017-01-05 21:51:25]

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-01-05 21:59:12]

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

[commit-bot: I haz the power, 2017-01-05 21:59:13]

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...)

[domenic, 2017-01-05 22:14:55]

On 2017/01/05 at 21:59:13, commit-bot wrote:
> 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...)

I guess ricea@ is not an owner. tyoshino@, mind taking a look?

[tyoshino (SeeGerritForStatus), 2017-01-06 05:47:20]

WritableStream.js lgtm
rslgtm on the other files

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

https://codereview.chromium.org/2596883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:286: if (controller ===
undefined ||
wh

https://codereview.chromium.org/2596883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:335:
getDefaultControllerInCloseFlag(stream[_writableStreamController]) ===
use 4 space indent after line-wrapping right after parenthesis for consistency
with the other places

https://codereview.chromium.org/2596883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:705: 'close',
[controller]);
let's adjust indentation using this opportunity (or in a separate patch)

[tyoshino (SeeGerritForStatus), 2017-01-06 05:47:49]

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

https://codereview.chromium.org/2596883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/WritableStream.js:286: if (controller ===
undefined ||
On 2017/01/06 05:47:19, tyoshino wrote:
> wh

Sorry, it's a mistake. please ignore.

[domenic, 2017-01-06 15:42:18]

The CQ bit was checked by domenic@chromium.org

[domenic, 2017-01-06 15:42:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org, ricea@chromium.org
Link to the patchset: https://codereview.chromium.org/2596883002/#ps60001
(title: "Adjust indents")

[commit-bot: I haz the power, 2017-01-06 15:42:32]

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-01-06 17:59:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483717338059570,
"parent_rev": "f61d29214d9db5de82c5b6f452cb09a142d50634", "commit_rev":
"2d69d55423fa357963f4e5bbaf8b5ab44075a340"}

[commit-bot: I haz the power, 2017-01-06 17:59:48]

Description was changed from

==========
Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...
-
https://github.com/whatwg/streams/commit/43c4b00d440b3c391a48fb761c3967c2e77d...
-
https://github.com/whatwg/streams/commit/22b3f03bf803868bf65d9b57f862cc7e3cc8...

BUG=671958
==========

to

==========
Update writable streams for the latest spec changes

This incorporates the spec and test changes at:

-
https://github.com/whatwg/streams/commit/0745219ca7b7f02ef104145b78ebebd99c0f...
-
https://github.com/whatwg/streams/commit/872188a984722861020ae773a5ee55889279...
-
https://github.com/whatwg/streams/commit/591a6eda9ea0b84881a370519d6aca53fab9...
-
https://github.com/whatwg/streams/commit/551489c68d1f9c582a3a7334fbb88a85500d...
-
https://github.com/whatwg/streams/commit/68aa48c5e463bdc15c43944cd561ce6fcec5...
-
https://github.com/whatwg/streams/commit/43c4b00d440b3c391a48fb761c3967c2e77d...
-
https://github.com/whatwg/streams/commit/22b3f03bf803868bf65d9b57f862cc7e3cc8...

BUG=671958

Review-Url: https://codereview.chromium.org/2596883002
Cr-Commit-Position: refs/heads/master@{#441974}
Committed:
https://chromium.googlesource.com/chromium/src/+/2d69d55423fa357963f4e5bbaf8b...
==========

[commit-bot: I haz the power, 2017-01-06 17:59:49]

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