Summary: | [Fetch API] Use ReadableStream pull to transfer binary data to stream when application needs it | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
youenn fablet
2016-10-04 01:49:42 PDT
Created attachment 290584 [details]
Patch
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? 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. Created attachment 290798 [details]
Patch for landing
Comment on attachment 290798 [details] Patch for landing Clearing flags on attachment: 290798 Committed r206857: <http://trac.webkit.org/changeset/206857> All reviewed patches have been landed. Closing bug. |