Bug 161137

Summary:

[Fetch API] Ensure response cloning works when data is loading

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebCore Misc.

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, cdumez, commit-queue, esprehn+autocc, kondapallykalyan, sam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

151937

Attachments:

Description	Flags
Patch	none
Improving test style	none
Patch for landing	none

youenn fablet

Reported 2016-08-24 06:52:45 PDT

When data is loading, cloning is not working as FetchBody contains no data.

Attachments
Patch (12.79 KB, patch) 2016-08-24 06:59 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Improving test style (12.70 KB, patch) 2016-08-25 07:18 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (12.69 KB, patch) 2016-08-28 23:20 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2016-08-24 06:59:37 PDT

Created attachment 286852 [details] Patch

youenn fablet

Comment 2 2016-08-25 07:18:54 PDT

Created attachment 286971 [details] Improving test style

Darin Adler

Comment 3 2016-08-27 17:12:37 PDT

Comment on attachment 286971 [details] Improving test style View in context: https://bugs.webkit.org/attachment.cgi?id=286971&action=review > Source/WebCore/Modules/fetch/FetchResponse.cpp:140 > if (m_response.m_readableStreamSource) { > - m_response.m_readableStreamSource->close(); > - m_response.m_readableStreamSource = nullptr; > + // Let's close the stream except if we have data to enqueue. > + if (m_response.m_body.type() != FetchBody::Type::Loaded) { > + m_response.m_readableStreamSource->close(); > + m_response.m_readableStreamSource = nullptr; > + } > } In the case where we don’t close the stream here, what closes the stream? Why not use && instead of nested if statements?

youenn fablet

Comment 4 2016-08-28 23:18:11 PDT

(In reply to comment #3) > Comment on attachment 286971 [details] > Improving test style > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286971&action=review > > > Source/WebCore/Modules/fetch/FetchResponse.cpp:140 > > if (m_response.m_readableStreamSource) { > > - m_response.m_readableStreamSource->close(); > > - m_response.m_readableStreamSource = nullptr; > > + // Let's close the stream except if we have data to enqueue. > > + if (m_response.m_body.type() != FetchBody::Type::Loaded) { > > + m_response.m_readableStreamSource->close(); > > + m_response.m_readableStreamSource = nullptr; > > + } > > } > > In the case where we don’t close the stream here, what closes the stream? When type is Loaded, FetchBody has some data, which means the data was not enqueued in the stream. On the first read request, FetchBody::consumeAsStream will enqueue the data and close the stream. I'll update the comment. Overall, there is some complexity added by the idea of enqueuing data only on the first read call. Maybe we should simplify this and enqueue data as soon as the stream is created. > Why not use && instead of nested if statements? Right, I'll fix that.

youenn fablet

Comment 5 2016-08-28 23:20:35 PDT

Created attachment 287249 [details] Patch for landing

WebKit Commit Bot

Comment 6 2016-08-28 23:51:32 PDT

Comment on attachment 287249 [details] Patch for landing Clearing flags on attachment: 287249 Committed r205110: <http://trac.webkit.org/changeset/205110>

WebKit Commit Bot

Comment 7 2016-08-28 23:51:36 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.