Add another body function returning a V8 Extra ReadableStream to Response

Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832
Committed: https://crrev.com/2b12939638f2fe7c5eabcc0dae0cac479651d9ba
Cr-Commit-Position: refs/heads/master@{#385377}

[yhirano, 2016-03-25 07:55:00]

Description was changed from

==========
Add another body function returning a V8 Extra ReadableStream to Response

todo

BUG=596832
==========

to

==========
Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832
==========

[yhirano, 2016-03-25 08:57:18]

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

[yhirano, 2016-03-30 12:02:49]

ping

[tyoshino (SeeGerritForStatus), 2016-04-04 09:21:30]

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:23: #include "wtf/RefPtr.h"
OwnPtr.h
WebDataConsumerHandle.h

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:107:
DECLARE_EAGER_FINALIZATION_OPERATOR_NEW();
Could you please point me to some doc by which I can quickly learn these macros?
There I couldn't find any doc about eager finalization.

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:131: const auto kNone =
WebDataConsumerHandle::FlagNone;
kNone is used just once. how about just putting the RHS?

[yhirano, 2016-04-04 09:28:19]

yhirano@chromium.org changed reviewers:
+ sigbjornf@opera.com

[yhirano, 2016-04-04 09:28:20]

+sof for DECLARE_EAGER_FINALIZATION_OPERATOR_NEW.

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:23: #include "wtf/RefPtr.h"
On 2016/04/04 09:21:30, tyoshino wrote:
> OwnPtr.h
> WebDataConsumerHandle.h

Done.

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:107:
DECLARE_EAGER_FINALIZATION_OPERATOR_NEW();
On 2016/04/04 09:21:30, tyoshino wrote:
> Could you please point me to some doc by which I can quickly learn these
macros?
> There I couldn't find any doc about eager finalization.

I don't know much about it either. I got an assertion error and the comments
suggested using this macro.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/1827373002/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/Body.cpp:131: const auto kNone =
WebDataConsumerHandle::FlagNone;
On 2016/04/04 09:21:30, tyoshino wrote:
> kNone is used just once. how about just putting the RHS?

Done.

[sof, 2016-04-04 11:26:32]

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Body.cpp:109:
DECLARE_EAGER_FINALIZATION_OPERATOR_NEW();
this derived class looks fine & macro is needed as the GC mixin instance it
derives from declares its own override of new(), but as non-eagerly finalized.

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Body.cpp:133: size_t available;
Don't you risk running into "potentially uninitialized" warnings by leaving this
as uninitialized?

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.idl (right):

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.idl:32:
[RuntimeEnabled=ResponseBodyWithV8ExtraStream, CallWith=ScriptState] any
v8ExtraStreamBody();
Why is it a method and not a readonly attribute?

[yhirano, 2016-04-04 11:42:32]

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Body.cpp:133: size_t available;
On 2016/04/04 11:26:32, sof wrote:
> Don't you risk running into "potentially uninitialized" warnings by leaving
this
> as uninitialized?

Done.

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.idl (right):

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.idl:32:
[RuntimeEnabled=ResponseBodyWithV8ExtraStream, CallWith=ScriptState] any
v8ExtraStreamBody();
On 2016/04/04 11:26:32, sof wrote:
> Why is it a method and not a readonly attribute?

Calling this function is destructive, i.e. locks & disturbs |body|, so I would
like to make it a function. Anyway, this interface is not planned to be shipped.

[sof, 2016-04-04 11:47:23]

lgtm

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/fetch/Response.idl (right):

https://codereview.chromium.org/1827373002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/fetch/Response.idl:32:
[RuntimeEnabled=ResponseBodyWithV8ExtraStream, CallWith=ScriptState] any
v8ExtraStreamBody();
On 2016/04/04 11:42:32, yhirano wrote:
> On 2016/04/04 11:26:32, sof wrote:
> > Why is it a method and not a readonly attribute?
> 
> Calling this function is destructive, i.e. locks & disturbs |body|, so I would
> like to make it a function. Anyway, this interface is not planned to be
shipped.

ok, thanks for clarifying.

[tyoshino (SeeGerritForStatus), 2016-04-05 05:46:26]

lgtm

[yhirano, 2016-04-06 01:28:04]

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

[yhirano, 2016-04-06 01:28:06]

+haraken for Source/platform/RuntimeEnabledFeatures.in.

[haraken, 2016-04-06 01:36:38]

On 2016/04/06 01:28:06, yhirano wrote:
> +haraken for Source/platform/RuntimeEnabledFeatures.in.

LGTM

[yhirano, 2016-04-06 01:43:29]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2016-04-06 01:43:56]

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

[commit-bot: I haz the power, 2016-04-06 03:24:46]

Description was changed from

==========
Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832
==========

to

==========
Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832
==========

[commit-bot: I haz the power, 2016-04-06 03:24:47]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2016-04-06 03:25:33]

Description was changed from

==========
Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832
==========

to

==========
Add another body function returning a V8 Extra ReadableStream to Response

This CL adds a function to get body as a ReadableStream implemented with
V8 Extra. The function is behind the flag.

BUG=596832

Committed: https://crrev.com/2b12939638f2fe7c5eabcc0dae0cac479651d9ba
Cr-Commit-Position: refs/heads/master@{#385377}
==========

[commit-bot: I haz the power, 2016-04-06 03:25:35]

Patchset 8 (id:??) landed as
https://crrev.com/2b12939638f2fe7c5eabcc0dae0cac479651d9ba
Cr-Commit-Position: refs/heads/master@{#385377}