Response construction with a ReadableStream

Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}

Committed: https://crrev.com/ed81e1bf7e1424c3470d164c73339936141f1f06
Cr-Commit-Position: refs/heads/master@{#365760}

[yhirano, 2015-12-09 02:15:44]

Patchset #1 (id:1) has been deleted

[yhirano, 2015-12-09 02:15:54]

Patchset #1 (id:20001) has been deleted

[yhirano, 2015-12-09 03:12:53]

Patchset #1 (id:40001) has been deleted

[yhirano, 2015-12-09 03:19:46]

Description was changed from

==========
[WIP] Response construction with a ReadableStream

BUG=
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The change is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.

But these problems don't bother stable users.

BUG=564479
==========

[yhirano, 2015-12-09 03:19:46]

yhirano@chromium.org changed reviewers:
+ bashi@google.com, tyoshino@chromium.org

[yhirano, 2015-12-09 03:20:02]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The change is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.

But these problems don't bother stable users.

BUG=564479
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The change is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

[yhirano, 2015-12-09 03:20:49]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The change is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

[yhirano, 2015-12-09 03:24:41]

PTAL

tyoshino@: modules/fetch/ and layout tests
bashi@: bindings/

[bashi, 2015-12-09 03:49:21]

bashi@chromium.org changed reviewers:
+ bashi@chromium.org

[bashi, 2015-12-09 03:49:21]

https://codereview.chromium.org/1506023003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:33: *done =
doneValue->ToBoolean()->Value();
ToBoolean() is deprecated. We should use ToBoolean(context) or
BooleanValue(context) like below (depends on the semantics of |done|).

if (!r.IsEmpty()) {
  v8::Local<v8::Boolean> booleanValue;
  if (!doneValue->ToBoolean(scriptState->context()).ToLocal(&booleanValue))
    return v8::MaybeLocal<v8::Value>();
  *done = booleanValue->Value();
}

[yhirano, 2015-12-09 04:03:11]

Patchset #2 (id:80001) has been deleted

[yhirano, 2015-12-09 04:12:49]

https://codereview.chromium.org/1506023003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:33: *done =
doneValue->ToBoolean()->Value();
On 2015/12/09 03:49:21, bashi1 wrote:
> ToBoolean() is deprecated. We should use ToBoolean(context) or
> BooleanValue(context) like below (depends on the semantics of |done|).
> 
> if (!r.IsEmpty()) {
>   v8::Local<v8::Boolean> booleanValue;
>   if (!doneValue->ToBoolean(scriptState->context()).ToLocal(&booleanValue))
>     return v8::MaybeLocal<v8::Value>();
>   *done = booleanValue->Value();
> }

Done.

[bashi, 2015-12-09 04:41:47]

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:24:
V8ThrowException::throwTypeError(scriptState->isolate(), "The iteration item is
not an object.");
nit: Can we do IsObject() check on caller side? Throwing an exception aligns our
guideline[1], but it would be nice not to throw an exception if we don't have
to.

[1]
https://docs.google.com/document/d/1S1QH0bZGry90wfD11ehBw-a9bgeC2jz_FX513SRVf...

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:28:
v8::MaybeLocal<v8::Value> r = object->Get(scriptState->context(),
v8String(scriptState->isolate(), "value"));
nit: could we rename |r| to more descriptive one?

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:34: return
v8::MaybeLocal<v8::Value>();
v8Call(v8::Maybe<Boolean>) is a bit confusing one (sorry, me is to blame). It
returns false when |doneValue| is converted to "false". This function returns an
empty MaybeLocal without throwing an exception if |doneValue| becomes "false"
successfully. Is this a intended behavior?

[yhirano, 2015-12-09 06:13:26]

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:24:
V8ThrowException::throwTypeError(scriptState->isolate(), "The iteration item is
not an object.");
On 2015/12/09 04:41:47, bashi1 wrote:
> nit: Can we do IsObject() check on caller side? Throwing an exception aligns
our
> guideline[1], but it would be nice not to throw an exception if we don't have
> to.
> 
> [1]
>
https://docs.google.com/document/d/1S1QH0bZGry90wfD11ehBw-a9bgeC2jz_FX513SRVf...

Done.

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:28:
v8::MaybeLocal<v8::Value> r = object->Get(scriptState->context(),
v8String(scriptState->isolate(), "value"));
On 2015/12/09 04:41:47, bashi1 wrote:
> nit: could we rename |r| to more descriptive one?

Done.

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:34: return
v8::MaybeLocal<v8::Value>();
On 2015/12/09 04:41:47, bashi1 wrote:
> v8Call(v8::Maybe<Boolean>) is a bit confusing one (sorry, me is to blame). It
> returns false when |doneValue| is converted to "false". This function returns
an
> empty MaybeLocal without throwing an exception if |doneValue| becomes "false"
> successfully. Is this a intended behavior?

The intention here is that |done| should be set only when the member value is
successfully read.
That said, void v8Call(v8::Maybe<T> maybe, T& outVariable) [T = bool] returns
true when |maybe| is not empty, right? Aren't you talking about v8CallBoolean?

[bashi, 2015-12-09 07:06:37]

LGTM

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:34: return
v8::MaybeLocal<v8::Value>();
On 2015/12/09 06:13:26, yhirano wrote:
> On 2015/12/09 04:41:47, bashi1 wrote:
> > v8Call(v8::Maybe<Boolean>) is a bit confusing one (sorry, me is to blame).
It
> > returns false when |doneValue| is converted to "false". This function
returns
> an
> > empty MaybeLocal without throwing an exception if |doneValue| becomes
"false"
> > successfully. Is this a intended behavior?
> 
> The intention here is that |done| should be set only when the member value is
> successfully read.
> That said, void v8Call(v8::Maybe<T> maybe, T& outVariable) [T = bool] returns
> true when |maybe| is not empty, right? Aren't you talking about v8CallBoolean?

You are right. I was talking about v8CallBoolean(). Sorry for the noise.

[tyoshino (SeeGerritForStatus), 2015-12-09 08:54:27]

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:20:
#include "public/platform/WebTraceLocation.h"
add wtf/Assertions.h

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:243:
// JavaScript object. Fix it.
TODO

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:245:
// leackage because we read only Uint8Array chunks from the reader.
leackage -> leakage

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.h
(right):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.h:23:
// TODO(yhirano): CURRENTLY THIS HANDLE SUPPORTS READING FROM THE THREAD ON
only?

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.idl (left):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.idl:9: // FIXME: Add union
type: URLSearchParams
keep this, maybe in the .cpp file

[yhirano, 2015-12-09 10:54:10]

PS4 inserted V8RecursionScope to address assertion failures.

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:20:
#include "public/platform/WebTraceLocation.h"
On 2015/12/09 08:54:26, tyoshino wrote:
> add wtf/Assertions.h

Done.

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:243:
// JavaScript object. Fix it.
On 2015/12/09 08:54:26, tyoshino wrote:
> TODO

Done.

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:245:
// leackage because we read only Uint8Array chunks from the reader.
On 2015/12/09 08:54:26, tyoshino wrote:
> leackage -> leakage

Done.

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.h
(right):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.h:23:
// TODO(yhirano): CURRENTLY THIS HANDLE SUPPORTS READING FROM THE THREAD ON
On 2015/12/09 08:54:26, tyoshino wrote:
> only?

Done.

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.idl (left):

https://codereview.chromium.org/1506023003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.idl:9: // FIXME: Add union
type: URLSearchParams
On 2015/12/09 08:54:26, tyoshino wrote:
> keep this, maybe in the .cpp file

Done.

[domenic, 2015-12-09 13:03:17]

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

[domenic, 2015-12-09 13:03:18]

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js
(right):

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:8:
function read_until_end(reader) {
You could probably reuse the helper from rs-utils.js if you want.

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:43:
assert_true(stream.locked);
Unreachable code

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:84:
return response.text().then(unreached_fulfillment(t), e => {
You can use promise_rejects for these types of tests

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/response-stream-construction.html
(right):

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/response-stream-construction.html:11:
service_worker_test(
Would it be better to copy the three-tests-in-one-.html format that the rest of
the stream tests use? Or are these tests different?

[tyoshino (SeeGerritForStatus), 2015-12-10 10:49:51]

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:248:
TrackExceptionState es;
please write the reason you can ignore exceptions

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.cpp (right):

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.cpp:141: contentType =
"text/plain;charset=UTF-8";
no need to call replaceUnmatchedSurrogates()? I'm comparing this with
toUSVString()

[yhirano, 2015-12-10 10:51:05]

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js
(right):

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:8:
function read_until_end(reader) {
On 2015/12/09 13:03:18, domenic wrote:
> You could probably reuse the helper from rs-utils.js if you want.

Done.

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:43:
assert_true(stream.locked);
On 2015/12/09 13:03:18, domenic wrote:
> Unreachable code

Nice catch! Fixed.

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:84:
return response.text().then(unreached_fulfillment(t), e => {
On 2015/12/09 13:03:18, domenic wrote:
> You can use promise_rejects for these types of tests

Done.

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/response-stream-construction.html
(right):

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/response-stream-construction.html:11:
service_worker_test(
On 2015/12/09 13:03:18, domenic wrote:
> Would it be better to copy the three-tests-in-one-.html format that the rest
of
> the stream tests use? Or are these tests different?

We think separating them is useful to see the failures easily.

[yhirano, 2015-12-10 11:18:10]

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:248:
TrackExceptionState es;
On 2015/12/10 10:49:51, tyoshino wrote:
> please write the reason you can ignore exceptions

Done.

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.cpp (right):

https://codereview.chromium.org/1506023003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.cpp:141: contentType =
"text/plain;charset=UTF-8";
On 2015/12/10 10:49:51, tyoshino wrote:
> no need to call replaceUnmatchedSurrogates()? I'm comparing this with
> toUSVString()

You're right. Let's use it.

[tyoshino (SeeGerritForStatus), 2015-12-10 13:49:37]

lgtm

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction-base-https-other-https.html
(right):

https://codereview.chromium.org/1506023003/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction-base-https-other-https.html:15:
script.src =
'../script-tests/response-stream-construction.js?-base-https-other-https';
is -other-https necessary?

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:5:
importScripts('../resources/fetch-test-helpers.js');
is this used?

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction.html
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction.html:9:
<script src = "../resources/fetch-test-helpers.js"></script>
are these two scripts used?

[haraken, 2015-12-10 14:24:34]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-12-10 14:24:35]

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:21:
v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState* scriptState,
v8::Local<v8::Object> item, bool* done)

I don't see much benefit in returning a MaybeLocal from this method. Just like
v8IteratorResultValue, we can just return a Local handle.

(bashi@ would know the rule better.)

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:24: if
(maybeValue.IsEmpty())

Can you use:

  if (!v8Call(item->Get(...)))
    return v8::MaybeLocal<v8::Value>();

?

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h (right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h:19:
CORE_EXPORT v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState*,
v8::Local<v8::Object> item, bool* done);

v8IterationItemUnpack => v8UnpackIteratorResultValue or v8GetIteratorResultValue
?

[yhirano, 2015-12-11 03:27:18]

haraken@, can you take a look at platform/ changes as well?

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/response-stream-construction.js:5:
importScripts('../resources/fetch-test-helpers.js');
On 2015/12/10 13:49:37, tyoshino wrote:
> is this used?

Yes, in serviceworker test setup steps.

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction.html
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/fetch/window/response-stream-construction.html:9:
<script src = "../resources/fetch-test-helpers.js"></script>
On 2015/12/10 13:49:37, tyoshino wrote:
> are these two scripts used?

This is an auto-generated file and we don't trim unused imports individually.

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:21:
v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState* scriptState,
v8::Local<v8::Object> item, bool* done)
On 2015/12/10 14:24:35, haraken wrote:
> 
> I don't see much benefit in returning a MaybeLocal from this method. Just like
> v8IteratorResultValue, we can just return a Local handle.
> 
> (bashi@ would know the rule better.)

bashi@'s document says we should return a maybe handle. I think
v8IteratorResultValue should, too, but it is out of scope of this change.

https://docs.google.com/document/d/1S1QH0bZGry90wfD11ehBw-a9bgeC2jz_FX513SRVf...

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:24: if
(maybeValue.IsEmpty())
On 2015/12/10 14:24:35, haraken wrote:
> 
> Can you use:
> 
>   if (!v8Call(item->Get(...)))
>     return v8::MaybeLocal<v8::Value>();
> 
> ?

It is technically possible, but we don't have to unwrap the maybe handle since
we are returning a maybe handle. So I don't call v8Call / ToLocal here.

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h (right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h:19:
CORE_EXPORT v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState*,
v8::Local<v8::Object> item, bool* done);
On 2015/12/10 14:24:35, haraken wrote:
> 
> v8IterationItemUnpack => v8UnpackIteratorResultValue or
v8GetIteratorResultValue
> ?

Fixed.

Here "iterator result" means an object having two members "done" and 
"value".

 - v8IteratorResultValue takes a value and returns {done: true, value}.
 - v8IteratorResultDone returns {done: false}.
 - v8UnpackIteratorResult takes an iterator result and returns its done and
value members.

Makes sense?

[yhirano, 2015-12-11 03:42:56]

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h (right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.h:19:
CORE_EXPORT v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState*,
v8::Local<v8::Object> item, bool* done);
On 2015/12/11 03:27:18, yhirano wrote:
> On 2015/12/10 14:24:35, haraken wrote:
> > 
> > v8IterationItemUnpack => v8UnpackIteratorResultValue or
> v8GetIteratorResultValue
> > ?
> 
> Fixed.
> 
> Here "iterator result" means an object having two members "done" and 
> "value".
> 
>  - v8IteratorResultValue takes a value and returns {done: true, value}.
>  - v8IteratorResultDone returns {done: false}.
>  - v8UnpackIteratorResult takes an iterator result and returns its done and
> value members.
> 
> Makes sense?
correction:
 - v8IteratorResult takes a value and returns {done: true, value}.
 - v8IteratorResultDone returns {done: false}.
 - v8IteratorResultValue takes two values for done and value members and returns
{done, value}.
 - v8UnpackIteratorResult takes an iterator result and returns its done and
value members.

[bashi, 2015-12-11 04:03:20]

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp
(right):

https://codereview.chromium.org/1506023003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8IteratorResultValue.cpp:21:
v8::MaybeLocal<v8::Value> v8IterationItemUnpack(ScriptState* scriptState,
v8::Local<v8::Object> item, bool* done)
On 2015/12/11 03:27:18, yhirano wrote:
> On 2015/12/10 14:24:35, haraken wrote:
> > 
> > I don't see much benefit in returning a MaybeLocal from this method. Just
like
> > v8IteratorResultValue, we can just return a Local handle.
> > 
> > (bashi@ would know the rule better.)
> 
> bashi@'s document says we should return a maybe handle. I think
> v8IteratorResultValue should, too, but it is out of scope of this change.
> 
>
https://docs.google.com/document/d/1S1QH0bZGry90wfD11ehBw-a9bgeC2jz_FX513SRVf...

+1 for MaybeLocal because this could throw an exception and Value could be
empty. Returning Local doesn't force empty check.

[haraken, 2015-12-11 04:17:52]

bindings/ and platform/ LGTM

[yhirano, 2015-12-14 01:29:22]

Domenic, do you have any other comments?

[domenic, 2015-12-14 02:18:02]

LGTM with potential nits or things you could work on in a follow-up. Very
excited!

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:50:
if (!item->IsObject() || !v8Call(v8UnpackIteratorResult(v.scriptState(),
item.As<v8::Object>(), &done), value)) {
Could these be asserts instead? They should never fail, right?

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:239:
Context(ScriptState* scriptState, v8::Local<v8::Value> stream)
Context was a confusing name for me because it is easy to confuse with
v8::Context. Maybe name it ReadingState or ReadingContext or something?

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
(right):

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp:31:
const Result kUnexpectedError = WebDataConsumerHandle::UnexpectedError;
Should we try to give more useful errors? E.g. it seems especially important to
tell the user that non-Uint8Arrays are bad if they did that.

[yhirano, 2015-12-14 03:42:54]

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp
(right):

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:50:
if (!item->IsObject() || !v8Call(v8UnpackIteratorResult(v.scriptState(),
item.As<v8::Object>(), &done), value)) {
On 2015/12/14 02:18:02, domenic wrote:
> Could these be asserts instead? They should never fail, right?

Done.

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp:239:
Context(ScriptState* scriptState, v8::Local<v8::Value> stream)
On 2015/12/14 02:18:02, domenic wrote:
> Context was a confusing name for me because it is easy to confuse with
> v8::Context. Maybe name it ReadingState or ReadingContext or something?

Done.

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
(right):

https://codereview.chromium.org/1506023003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp:31:
const Result kUnexpectedError = WebDataConsumerHandle::UnexpectedError;
On 2015/12/14 02:18:02, domenic wrote:
> Should we try to give more useful errors? E.g. it seems especially important
to
> tell the user that non-Uint8Arrays are bad if they did that.

I agree that we need more meaningful error definitions, but we currently don't
know what errors is needed, because we are not 100% sure how
WebDataConsumerHandle will be used in the future. In addition to that, we may
use a js implementation for consume + package data from streams. I will revisit
this problem later.

[yhirano, 2015-12-14 03:43:04]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-12-14 03:43:04]

The patchset sent to the CQ was uploaded after l-g-t-m from bashi@chromium.org,
tyoshino@chromium.org, haraken@chromium.org, domenic@chromium.org
Link to the patchset: https://codereview.chromium.org/1506023003/#ps320001
(title: " ")

[commit-bot: I haz the power, 2015-12-14 03:43:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1506023003/320001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1506023003/320001

[commit-bot: I haz the power, 2015-12-14 05:43:06]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

[commit-bot: I haz the power, 2015-12-14 05:43:07]

Committed patchset #13 (id:320001)

[commit-bot: I haz the power, 2015-12-14 05:44:07]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

[commit-bot: I haz the power, 2015-12-14 05:44:08]

Patchset 13 (id:??) landed as
https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

[sof, 2015-12-14 07:38:26]

A revert of this CL (patchset #13 id:320001) has been created in
https://codereview.chromium.org/1527493002/ by sigbjornf@opera.com.

The reason for reverting is: Caused some UAFs,


http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/bu....

[yhirano, 2015-12-14 08:10:14]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

[yhirano, 2015-12-14 08:10:56]

On 2015/12/14 07:38:26, sof wrote:
> A revert of this CL (patchset #13 id:320001) has been created in
> https://codereview.chromium.org/1527493002/ by mailto:sigbjornf@opera.com.
> 
> The reason for reverting is: Caused some UAFs,
> 
> 
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/bu....

Thanks. I reopen the issue as it was reverted.

[yhirano, 2015-12-14 11:06:53]

I thought bind protects a RefCounted pointer by creating a RefPtr, but it was
wrong.
tyoshino@, can you take a look at PS14 again?

Thanks!

[tyoshino (SeeGerritForStatus), 2015-12-15 08:56:52]

On 2015/12/14 11:06:53, yhirano wrote:
> I thought bind protects a RefCounted pointer by creating a RefPtr, but it was
> wrong.
> tyoshino@, can you take a look at PS14 again?
> 
> Thanks!

Oh, I see. lgtm

[yhirano, 2015-12-15 08:58:31]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-12-15 08:58:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, domenic@chromium.org, bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/1506023003/#ps340001
(title: "Fix a uaf")

[commit-bot: I haz the power, 2015-12-15 08:58:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1506023003/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1506023003/340001

[commit-bot: I haz the power, 2015-12-15 12:04:38]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

[commit-bot: I haz the power, 2015-12-15 12:04:39]

Committed patchset #14 (id:340001)

[commit-bot: I haz the power, 2015-12-15 12:05:29]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

[commit-bot: I haz the power, 2015-12-15 12:05:30]

Patchset 14 (id:??) landed as
https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}

[Yuta Kitamura, 2015-12-16 03:42:41]

A revert of this CL (patchset #14 id:340001) has been created in
https://codereview.chromium.org/1528183004/ by yutak@chromium.org.

The reason for reverting is: Seems to have broken leak bots:
https://code.google.com/p/chromium/issues/detail?id=570168.

[yhirano, 2015-12-16 05:12:05]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

[yhirano, 2015-12-16 05:13:51]

On 2015/12/16 03:42:41, Yuta Kitamura wrote:
> A revert of this CL (patchset #14 id:340001) has been created in
> https://codereview.chromium.org/1528183004/ by mailto:yutak@chromium.org.
> 
> The reason for reverting is: Seems to have broken leak bots:
> https://code.google.com/p/chromium/issues/detail?id=570168.

As in the issue description, the leak doesn't affect stable features. I added
tests to LeakExpectation and will land this change again.
I will fix the leak in a separate CL.

[yhirano, 2015-12-16 05:14:00]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-12-16 05:14:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, domenic@chromium.org, bashi@chromium.org,
tyoshino@chromium.org
Link to the patchset: https://codereview.chromium.org/1506023003/#ps360001
(title: " ")

[commit-bot: I haz the power, 2015-12-16 05:15:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1506023003/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1506023003/360001

[yhirano, 2015-12-16 06:13:24]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2015-12-17 02:24:26]

On 2015/12/16 05:13:51, yhirano wrote:
> On 2015/12/16 03:42:41, Yuta Kitamura wrote:
> > A revert of this CL (patchset #14 id:340001) has been created in
> > https://codereview.chromium.org/1528183004/ by mailto:yutak@chromium.org.
> > 
> > The reason for reverting is: Seems to have broken leak bots:
> > https://code.google.com/p/chromium/issues/detail?id=570168.
> 
> As in the issue description, the leak doesn't affect stable features. I added
> tests to LeakExpectation and will land this change again.
> I will fix the leak in a separate CL.

I changed my mind: I made the leaked stream collectable when the execution
context is stopped.
I will fix the leak separately.

[yhirano, 2015-12-17 02:24:35]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-12-17 02:24:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, domenic@chromium.org, bashi@chromium.org,
tyoshino@chromium.org
Link to the patchset: https://codereview.chromium.org/1506023003/#ps380001
(title: " ")

[commit-bot: I haz the power, 2015-12-17 02:35:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1506023003/380001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1506023003/380001

[commit-bot: I haz the power, 2015-12-17 05:33:35]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

[commit-bot: I haz the power, 2015-12-17 05:33:37]

Committed patchset #16 (id:380001)

[commit-bot: I haz the power, 2015-12-17 05:34:19]

Description was changed from

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}
==========

to

==========
Response construction with a ReadableStream

This CL implements Response construction with a ReadableStream provided
by V8 Extras. The feature is behind a runtime enabled flag.

The implementation is not perfect.
 - ReadableStreamDataConsumerHandle should be thread-safe but is not.
 - ReadableStreamDataConsumerHandle may cause memory leaks.
But these problems don't bother stable users.

BUG=564479

Committed: https://crrev.com/6aa006ba0f0f8a60c20167ad009f5699e40b8ef2
Cr-Commit-Position: refs/heads/master@{#364968}

Committed: https://crrev.com/84d781133725f4170c0c21a99da1f35c9e17ca5d
Cr-Commit-Position: refs/heads/master@{#365229}

Committed: https://crrev.com/ed81e1bf7e1424c3470d164c73339936141f1f06
Cr-Commit-Position: refs/heads/master@{#365760}
==========

[commit-bot: I haz the power, 2015-12-17 05:34:20]

Patchset 16 (id:??) landed as
https://crrev.com/ed81e1bf7e1424c3470d164c73339936141f1f06
Cr-Commit-Position: refs/heads/master@{#365760}