Implementation of ReadableStream pipeTo and pipeThrough

Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

Also remove the failing expectations for many external/wpt/streams tests
that use piping.

serviceworker still have failing expectations due to using HTTP. When
https://github.com/w3c/web-platform-tests/commit/f36afea64a7478360f7c30b976c87acc2564e7ea
is rolled from upstream the failures will go away.

The upstream version of the streams/piping/multiple-propagation.js test
'Piping from an errored readable stream to a closed writable stream'
doesn't pass in Chromium . This is a known incompatibility with the
reference implementation which will be resolved by
https://github.com/whatwg/streams/pull/634. This CL adds a modified
version of the test in http/tests/streams/piping which verifies the
behaviour of this implementation. I have filed http://crbug.com/684543
to track it.

BUG=668951
Review-Url: https://codereview.chromium.org/2561443004
Cr-Commit-Position: refs/heads/master@{#446358}
Committed: https://chromium.googlesource.com/chromium/src/+/9ba4a643d92a0ec3890aa1c9b7f2be349ad2c928

[Adam Rice, 2017-01-12 12:32:04]

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

[commit-bot: I haz the power, 2017-01-12 12:32:14]

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-01-12 12:41:18]

Description was changed from

==========
WIP: Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

COMMIT=false
BUG=668951
==========

to

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To implement this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

[Adam Rice, 2017-01-12 12:43:02]

Description was changed from

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To implement this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

to

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

[commit-bot: I haz the power, 2017-01-12 14:32:54]

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

[commit-bot: I haz the power, 2017-01-12 14:32:55]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Adam Rice, 2017-01-12 14:58:09]

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

[commit-bot: I haz the power, 2017-01-12 14:58:27]

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

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

[commit-bot: I haz the power, 2017-01-12 17:09:16]

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

[commit-bot: I haz the power, 2017-01-12 17:09:16]

Dry run: This issue passed the CQ dry run.

[Adam Rice, 2017-01-13 12:24:20]

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

[commit-bot: I haz the power, 2017-01-13 12:24:32]

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-01-13 14:32:28]

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

[commit-bot: I haz the power, 2017-01-13 14:32:29]

Dry run: This issue passed the CQ dry run.

[Adam Rice, 2017-01-18 07:03:15]

Description was changed from

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

to

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

[domenic, 2017-01-18 23:51:17]

The undefined error case probably needs to be fixed.

I am also impressed you got away without a "wait for current write" line like I
had to add in the reference implementation. In general it's great to get these
two independent implementations that both work.

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/ReadableStream.js (right):

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:234: function
initialStateOk() {
It might make sense to pull some of these out to top-level in the file (passing
them some arguments) so that they are not re-created every time someone calls
pipeTo. But it's a judgement call as to which is more appropriate, and can
certainly be left for a follow-up.

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:241: if
(binding.IsWritableStreamErrored(dest)) {
Maybe since these are not abstract operation names they should go with the more
conventional camel case? isWritableStreamErrored?

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:279: function
readResolved({value, done}) {
Nit: "fulfilled" is more correct than "resolved" here.
https://github.com/domenic/promises-unwrapping/blob/master/docs/states-and-fa...

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:365: if (error !==
undefined) {
This seems like it will not work correctly when the error is actually undefined,
explicitly. We should have tests for that case; I guess we do not. In the
reference implementation I had to solve this by passing along an additional
boolean flag.

[Adam Rice, 2017-01-19 14:02:47]

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/streams/ReadableStream.js (right):

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:234: function
initialStateOk() {
On 2017/01/18 23:51:17, domenic wrote:
> It might make sense to pull some of these out to top-level in the file
(passing
> them some arguments) so that they are not re-created every time someone calls
> pipeTo. But it's a judgement call as to which is more appropriate, and can
> certainly be left for a follow-up.

This function calls state-changing functions, which means it would have to be
passed an object encapsulating the current state, which ends up having to bind a
lot of parameters.

I started out implementing pipeTo as a class, but I found I had to bind |this|
to the methods and it was very ugly. So I took the painful step of rewriting it
with nested functions. Having done that I don't really want to go back to
binding stuff again.

This whole thing will change drastically when we start optimising pipeTo anyway,
so I'm not worried.

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:241: if
(binding.IsWritableStreamErrored(dest)) {
On 2017/01/18 23:51:17, domenic wrote:
> Maybe since these are not abstract operation names they should go with the
more
> conventional camel case? isWritableStreamErrored?

It's not great for the getters, because I don't want to write WritableStream
with a lowercase W. I added a "get" prefix instead.

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:279: function
readResolved({value, done}) {
On 2017/01/18 23:51:17, domenic wrote:
> Nit: "fulfilled" is more correct than "resolved" here.
>
https://github.com/domenic/promises-unwrapping/blob/master/docs/states-and-fa...

I hadn't seen that document before. It's cleared up some stuff that was fuzzy in
my mind. Thanks!

https://codereview.chromium.org/2561443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/streams/ReadableStream.js:365: if (error !==
undefined) {
On 2017/01/18 23:51:17, domenic wrote:
> This seems like it will not work correctly when the error is actually
undefined,
> explicitly. We should have tests for that case; I guess we do not. In the
> reference implementation I had to solve this by passing along an additional
> boolean flag.

Yep. I went with passing booleans around as well. I have written tests and will
submit them to the web-platform-tests repository.

[domenic, 2017-01-19 18:37:37]

lgtm

[Adam Rice, 2017-01-20 04:19:38]

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

[tyoshino (SeeGerritForStatus), 2017-01-23 10:42:58]

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/streams/ReadableStream.js (right):

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:234: function
initialStateOk() {
This function name sounds like checking states and returning go/no-go without
side-effect, but this is actually handling some initial state values to perform
state transition.

Maybe you factored out the following logic and named this function like this for
describing the meaning of the return value while avoiding redundant code. I
understand that motivation but to follow our usual convention, how about name
this like checkInitialState() and add a small comment about the return value?

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:244: }
are these error check blocks for optimization?

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:247: readableClosed();
how about calling factoring out the latter half of readableClosed() and call it
here?

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:249: }
Is this an optimization or to have pipeTo finish successfully even if the dest
is errored/closed/closing if preventAbort and preventClose are specified?

if the latter point is concerned, this should go before the block for checking
dest's error (L241).

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:285: readableClosed();
how about calling factoring out the latter half of readableClosed() and call it
here?

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:309: const storedError
= binding.getWritableStreamStoredError(dest);
can't we use the rejection value? or intentionally using this for better
correspondence with the spec text?

same about readableError()

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:344: errorGiven =
false) {
default argument is used for better correspondence with the spec text where
"optionally ..." idiom is used?

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:353: }
ah, we need to update the reference implementation by removing the logic to wait
for ongoing write().

[Adam Rice, 2017-01-23 13:24:31]

The presubmit suddenly decided it wanted me to format my Javascript. Sorry for
the diff churn.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/streams/ReadableStream.js (right):

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:234: function
initialStateOk() {
On 2017/01/23 10:42:57, tyoshino wrote:
> This function name sounds like checking states and returning go/no-go without
> side-effect, but this is actually handling some initial state values to
perform
> state transition.
> 
> Maybe you factored out the following logic and named this function like this
for
> describing the meaning of the return value while avoiding redundant code. I
> understand that motivation but to follow our usual convention, how about name
> this like checkInitialState() and add a small comment about the return value?

Done.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:244: }
On 2017/01/23 10:42:57, tyoshino wrote:
> are these error check blocks for optimization?

They are to perform the checks in the correct order. This is the order that
passes the tests; it's not currently specified by the standard.

The uncommitted pull request https://github.com/whatwg/streams/pull/647 contains
more details.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:247: readableClosed();
On 2017/01/23 10:42:57, tyoshino wrote:
> how about calling factoring out the latter half of readableClosed() and call
it
> here?

The first check in readableClosed() didn't belong in that operation, so I have
moved it to a separate onReaderClosed() function instead.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:249: }
On 2017/01/23 10:42:57, tyoshino wrote:
> Is this an optimization or to have pipeTo finish successfully even if the dest
> is errored/closed/closing if preventAbort and preventClose are specified?
> 
> if the latter point is concerned, this should go before the block for checking
> dest's error (L241).

It's purely to perform the checks in the correct order to pass the tests.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:285: readableClosed();
On 2017/01/23 10:42:57, tyoshino wrote:
> how about calling factoring out the latter half of readableClosed() and call
it
> here?

I removed the first half instead, to make the operations match the ones in the
standard.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:309: const storedError
= binding.getWritableStreamStoredError(dest);
On 2017/01/23 10:42:57, tyoshino wrote:
> can't we use the rejection value? or intentionally using this for better
> correspondence with the spec text?
> 
> same about readableError()

I'm not sure about this, because now we're looking up the [[storedError]] in two
different places instead of inside this function. PTAL.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:344: errorGiven =
false) {
On 2017/01/23 10:42:57, tyoshino wrote:
> default argument is used for better correspondence with the spec text where
> "optionally ..." idiom is used?

Exactly. This worked better before Domenic pointed out that I needed to add the
errorGiven boolean.

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:353: }
On 2017/01/23 10:42:57, tyoshino wrote:
> ah, we need to update the reference implementation by removing the logic to
wait
> for ongoing write().

Yes. I discussed this with Domenic but it looks like I never filed an issue
about it. I just filed https://github.com/whatwg/streams/issues/668 for the
standard text change.

[tyoshino (SeeGerritForStatus), 2017-01-24 09:52:33]

> The presubmit suddenly decided it wanted me to format my Javascript. Sorry for
the diff churn.

No problem!

Skimmed the imported tests and reviewed the rest.

lgtm

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/streams/ReadableStream.js (right):

https://codereview.chromium.org/2561443004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/ReadableStream.js:244: }
On 2017/01/23 13:24:30, Adam Rice wrote:
> On 2017/01/23 10:42:57, tyoshino wrote:
> > are these error check blocks for optimization?
> 
> They are to perform the checks in the correct order. This is the order that
> passes the tests; it's not currently specified by the standard.
> 
> The uncommitted pull request https://github.com/whatwg/streams/pull/647
contains
> more details.

Oh, thanks for the pointer. Sorry for being unaware of that.

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/WritableStream.js:605: 'writer is a
WritableStreamDefaultWriter');
period

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/WritableStream.js:612: 'writer is a
WritableStreamDefaultWriter');
period

[Adam Rice, 2017-01-24 10:12:09]

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/streams/WritableStream.js (right):

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/WritableStream.js:605: 'writer is a
WritableStreamDefaultWriter');
On 2017/01/24 09:52:33, tyoshino wrote:
> period

Done.

https://codereview.chromium.org/2561443004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/streams/WritableStream.js:612: 'writer is a
WritableStreamDefaultWriter');
On 2017/01/24 09:52:33, tyoshino wrote:
> period

Done.

[Adam Rice, 2017-01-24 10:15:41]

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

[Adam Rice, 2017-01-24 10:15:43]

+brettw for v8.gni

[Adam Rice, 2017-01-24 10:16:16]

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

[commit-bot: I haz the power, 2017-01-24 10:16:38]

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-01-24 11:57:02]

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

[commit-bot: I haz the power, 2017-01-24 11:57:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Adam Rice, 2017-01-24 14:13:06]

Description was changed from

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

This CL also imports the piping tests from the upstream github repository.
This is temporary until they have been merged to the w3c web platform tests
repository.

BUG=668951
==========

to

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

Also remove the failing expectations for many external/wpt/streams tests
that use piping.

serviceworker still have failing expectations due to using HTTP. When
https://github.com/w3c/web-platform-tests/commit/f36afea64a7478360f7c30b976c8...
is rolled from upstream the failures will go away.

The upstream version of the streams/piping/multiple-propagation.js test
'Piping from an errored readable stream to a closed writable stream'
doesn't pass in Chromium . This is a known incompatibility with the
reference implementation which will be resolved by
https://github.com/whatwg/streams/pull/634. This CL adds a modified
version of the test in http/tests/streams/piping which verifies the
behaviour of this implementation. I have filed http://crbug.com/684543
to track it.

BUG=668951
==========

[Adam Rice, 2017-01-24 14:22:16]

Patchset #10 (id:180001) has been deleted

[Adam Rice, 2017-01-24 14:30:04]

Patchset #10 (id:200001) has been deleted

[Adam Rice, 2017-01-24 14:47:26]

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

[commit-bot: I haz the power, 2017-01-24 14:47:42]

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-01-24 16:29:16]

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

[commit-bot: I haz the power, 2017-01-24 16:29:17]

Dry run: This issue passed the CQ dry run.

[Adam Rice, 2017-01-26 04:16:56]

brettw, PTAL.

[brettw, 2017-01-26 04:31:19]

.gn LGTM

[Adam Rice, 2017-01-26 04:35:23]

The CQ bit was checked by ricea@chromium.org

[Adam Rice, 2017-01-26 04:35:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
domenic@chromium.org, tyoshino@chromium.org
Link to the patchset: https://codereview.chromium.org/2561443004/#ps220001
(title: "Rebase and move includes to .gn")

[commit-bot: I haz the power, 2017-01-26 04:35:58]

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-26 06:42:23]

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

[commit-bot: I haz the power, 2017-01-26 06:42:24]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Adam Rice, 2017-01-26 13:21:16]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2017-01-26 13:21:36]

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-26 14:46:20]

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

[commit-bot: I haz the power, 2017-01-26 14:46:22]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Adam Rice, 2017-01-26 16:27:05]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2017-01-26 16:27:19]

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-26 17:12:01]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1485448025852180,
"parent_rev": "3c4d50886437cc67b8e71485b478d835bc672c0d", "commit_rev":
"9ba4a643d92a0ec3890aa1c9b7f2be349ad2c928"}

[commit-bot: I haz the power, 2017-01-26 17:12:39]

Description was changed from

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

Also remove the failing expectations for many external/wpt/streams tests
that use piping.

serviceworker still have failing expectations due to using HTTP. When
https://github.com/w3c/web-platform-tests/commit/f36afea64a7478360f7c30b976c8...
is rolled from upstream the failures will go away.

The upstream version of the streams/piping/multiple-propagation.js test
'Piping from an errored readable stream to a closed writable stream'
doesn't pass in Chromium . This is a known incompatibility with the
reference implementation which will be resolved by
https://github.com/whatwg/streams/pull/634. This CL adds a modified
version of the test in http/tests/streams/piping which verifies the
behaviour of this implementation. I have filed http://crbug.com/684543
to track it.

BUG=668951
==========

to

==========
Implementation of ReadableStream pipeTo and pipeThrough

Implement the pipeTo() and pipeThrough() methods on the ReadableStream
class. They are only enabled when V8 experimental extras are, eg. when the
--enable-experimental-web-platform-features flag is used. The methods are not
visible when running without flags.

To achieve this, the methods are defined in ReadableStream.js but not
added to the global object. ReadableStreamExperimentalPipeTo.js is
executed at renderer startup time when the experimental flag is set
and simply adds the two methods to the global ReadableStream object.

This initial implementation is intentionally unoptimised. While it does
use internal APIs it won't perform any better than a polyfill using the
public APIs would. Additional optimisation work is expected after
shipping.

Also remove the failing expectations for many external/wpt/streams tests
that use piping.

serviceworker still have failing expectations due to using HTTP. When
https://github.com/w3c/web-platform-tests/commit/f36afea64a7478360f7c30b976c8...
is rolled from upstream the failures will go away.

The upstream version of the streams/piping/multiple-propagation.js test
'Piping from an errored readable stream to a closed writable stream'
doesn't pass in Chromium . This is a known incompatibility with the
reference implementation which will be resolved by
https://github.com/whatwg/streams/pull/634. This CL adds a modified
version of the test in http/tests/streams/piping which verifies the
behaviour of this implementation. I have filed http://crbug.com/684543
to track it.

BUG=668951

Review-Url: https://codereview.chromium.org/2561443004
Cr-Commit-Position: refs/heads/master@{#446358}
Committed:
https://chromium.googlesource.com/chromium/src/+/9ba4a643d92a0ec3890aa1c9b7f2...
==========

[commit-bot: I haz the power, 2017-01-26 17:12:41]

Committed patchset #10 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/9ba4a643d92a0ec3890aa1c9b7f2...