Bug 162892 – [Fetch API] Use ReadableStream pull to transfer binary data to stream when application needs it

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

When stream is not pulling data, it might be best to just store it in WebCore.
If there is a need for it, knowing how much data is stored in WebCore could allow us to request the network process to stop sending us data until we actually need it.

[youenn fablet]

Created attachment 290584 [details]
Patch

[Alex Christensen]

Comment on attachment 290584 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290584&action=review

> Source/WebCore/Modules/fetch/FetchResponse.cpp:290
> +    ASSERT(m_readableStreamSource);

This has two call sites, one of which checks m_readableStreamSource right before calling.  Would it hurt anything to move the check to here?

> Source/WebCore/Modules/fetch/FetchResponse.cpp:300
> +        if (!m_readableStreamSource->enqueue(body().consumer().takeAsArrayBuffer())) {

Should we assert m_readableStreamSource at the beginning of this function, or just if body.consumer.hasData?

[youenn fablet]

Comment on attachment 290584 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290584&action=review

>> Source/WebCore/Modules/fetch/FetchResponse.cpp:290
>> +    ASSERT(m_readableStreamSource);
> 
> This has two call sites, one of which checks m_readableStreamSource right before calling.  Would it hurt anything to move the check to here?

Yeah, not sure whether this is assert is all that useful.

This assert is just there in case we use closeStream() somewhere else in the future.
It does not hurt to have it and might help future debugging.
That said, closeStream() is a private method.

>> Source/WebCore/Modules/fetch/FetchResponse.cpp:300
>> +        if (!m_readableStreamSource->enqueue(body().consumer().takeAsArrayBuffer())) {
> 
> Should we assert m_readableStreamSource at the beginning of this function, or just if body.consumer.hasData?

Right, I'll add an assert at the beginning of this function.

[youenn fablet]

Created attachment 290798 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 290798 [details]
Patch for landing

Clearing flags on attachment: 290798

Committed r206857: <http://trac.webkit.org/changeset/206857>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.