Bug 159804

Summary:

[Fetch API] Response constructor should be able to take a ReadableStream as body

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebCore Misc.

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue, darin, sam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

159808, 160200

Bug Blocks:

151937

Attachments:

Description	Flags
WIP	none
Patch	none
Rebasing	none
Rebasing	none
Fixing build	none
Fixing build	none
Fixing Debug build	none
Fixing according review	none
Patch for landing	none
Removing WebCoreJSBuiltins.cpp from CMake WebCore_DERIVED_SOURCES	none

youenn fablet

Reported 2016-07-15 02:55:47 PDT

As per https://fetch.spec.whatwg.org/#responsebodyinit

Attachments
WIP (5.98 KB, patch) 2016-07-15 07:43 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (63.85 KB, patch) 2016-07-25 08:23 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Rebasing (63.82 KB, patch) 2016-07-25 08:26 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Rebasing (62.98 KB, patch) 2016-07-25 08:30 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Fixing build (63.00 KB, patch) 2016-07-25 09:30 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Fixing build (62.98 KB, patch) 2016-07-25 09:53 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Fixing Debug build (62.91 KB, patch) 2016-07-25 10:21 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Fixing according review (63.41 KB, patch) 2016-07-25 13:32 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (58.61 KB, patch) 2016-07-26 08:34 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Removing WebCoreJSBuiltins.cpp from CMake WebCore_DERIVED_SOURCES (59.29 KB, patch) 2016-07-26 10:39 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2016-07-15 07:43:55 PDT

Created attachment 283761 [details] WIP

youenn fablet

Comment 2 2016-07-25 08:23:53 PDT

Created attachment 284484 [details] Patch

youenn fablet

Comment 3 2016-07-25 08:26:54 PDT

Created attachment 284485 [details] Rebasing

WebKit Commit Bot

Comment 4 2016-07-25 08:28:23 PDT

Attachment 284485 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 5 2016-07-25 08:30:00 PDT

Created attachment 284486 [details] Rebasing

WebKit Commit Bot

Comment 6 2016-07-25 08:31:05 PDT

Attachment 284486 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 7 2016-07-25 09:30:36 PDT

Created attachment 284492 [details] Fixing build

WebKit Commit Bot

Comment 8 2016-07-25 09:31:57 PDT

Attachment 284492 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 9 2016-07-25 09:53:24 PDT

Created attachment 284493 [details] Fixing build

WebKit Commit Bot

Comment 10 2016-07-25 10:01:24 PDT

Attachment 284493 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 11 2016-07-25 10:21:31 PDT

Created attachment 284498 [details] Fixing Debug build

WebKit Commit Bot

Comment 12 2016-07-25 10:25:02 PDT

Attachment 284498 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 13 2016-07-25 12:02:26 PDT

Comment on attachment 284498 [details] Fixing Debug build View in context: https://bugs.webkit.org/attachment.cgi?id=284498&action=review > Source/WebCore/Modules/fetch/FetchResponse.js:115 > + return @consumeStream(this, 2); These numbers should at least have a name. > Source/WebCore/bindings/js/WebCoreBuiltinNames.h:56 > + macro(cloneForJS) \ These are out of alphabetical order.

Alex Christensen

Comment 14 2016-07-25 12:15:03 PDT

Comment on attachment 284498 [details] Fixing Debug build View in context: https://bugs.webkit.org/attachment.cgi?id=284498&action=review > Source/WebCore/ChangeLog:15 > + Clients of FetchLoader needs to oass a FetchBodyConsumer to the FetchLoader to do the data adaptation at loader creation time. pass > Source/WebCore/Modules/fetch/FetchLoader.h:66 > + FetchBodyConsumer* m_consumer; Do the FetchLoader and FetchBodyConsumer have the same lifetime?

youenn fablet

Comment 15 2016-07-25 12:38:40 PDT

(In reply to comment #13) > Comment on attachment 284498 [details] > Fixing Debug build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284498&action=review > > > Source/WebCore/Modules/fetch/FetchResponse.js:115 > > + return @consumeStream(this, 2); > > These numbers should at least have a name. What do you mean? Something like the status added for ReadableStream: @readableStreamClosed. We can do that to ensure the values and enum class remain in sync. Is it worth it? > > > Source/WebCore/bindings/js/WebCoreBuiltinNames.h:56 > > + macro(cloneForJS) \ > > These are out of alphabetical order. OK (In reply to comment #14) > Comment on attachment 284498 [details] > Fixing Debug build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284498&action=review > > > Source/WebCore/ChangeLog:15 > > + Clients of FetchLoader needs to oass a FetchBodyConsumer to the FetchLoader to do the data adaptation at loader creation time. > > pass OK >Source/WebCore/Modules/fetch/FetchLoader.h:66 > > + FetchBodyConsumer* m_consumer; > > Do the FetchLoader and FetchBodyConsumer have the same lifetime? FetchBodyConsumer has the safe lifetime as FetchLoaderClient. FWIW, it might be good to retire FetchLoader and move the code in its clients (FetchBodyOwner for blob loading and FetchResponse for fetch loads).

Alex Christensen

Comment 16 2016-07-25 12:43:35 PDT

(In reply to comment #15) > (In reply to comment #13) > > Comment on attachment 284498 [details] > > Fixing Debug build > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=284498&action=review > > > > > Source/WebCore/Modules/fetch/FetchResponse.js:115 > > > + return @consumeStream(this, 2); > > > > These numbers should at least have a name. > > What do you mean? We should at least have something like let meaningfulName = 2;

youenn fablet

Comment 17 2016-07-25 13:32:56 PDT

Created attachment 284522 [details] Fixing according review

WebKit Commit Bot

Comment 18 2016-07-25 13:34:31 PDT

Attachment 284522 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 19 2016-07-25 13:35:21 PDT

(In reply to comment #17) > Created attachment 284522 [details] > Fixing according review Thanks for the review. I integrated the different points you mentioned.

Alex Christensen

Comment 20 2016-07-25 15:59:57 PDT

Comment on attachment 284522 [details] Fixing according review View in context: https://bugs.webkit.org/attachment.cgi?id=284522&action=review Test failure might be unrelated. Please double check before landing. > Source/WebCore/Modules/fetch/FetchResponse.js:137 > + return @consumeStream(this, 3); jsonConsumerType

youenn fablet

Comment 21 2016-07-26 08:34:36 PDT

Created attachment 284592 [details] Patch for landing

youenn fablet

Comment 22 2016-07-26 08:58:42 PDT

> > Source/WebCore/Modules/fetch/FetchResponse.js:137 > > + return @consumeStream(this, 3); > > jsonConsumerType Done

WebKit Commit Bot

Comment 23 2016-07-26 09:04:13 PDT

Comment on attachment 284592 [details] Patch for landing Clearing flags on attachment: 284592 Committed r203719: <http://trac.webkit.org/changeset/203719>

WebKit Commit Bot

Comment 24 2016-07-26 09:04:17 PDT

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 25 2016-07-26 10:12:14 PDT

Re-opened since this is blocked by bug 160200

youenn fablet

Comment 26 2016-07-26 10:15:58 PDT

(In reply to comment #25) > Re-opened since this is blocked by bug 160200 WebCoreJSBuiltins.cpp is added in CMakeLists.txt while it does not need to.

youenn fablet

Comment 27 2016-07-26 10:39:09 PDT

Created attachment 284603 [details] Removing WebCoreJSBuiltins.cpp from CMake WebCore_DERIVED_SOURCES

WebKit Commit Bot

Comment 28 2016-07-26 10:41:14 PDT

Attachment 284603 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/Modules/fetch/FetchBodyConsumer.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 29 2016-07-26 23:39:27 PDT

Comment on attachment 284603 [details] Removing WebCoreJSBuiltins.cpp from CMake WebCore_DERIVED_SOURCES Clearing flags on attachment: 284603 Committed r203767: <http://trac.webkit.org/changeset/203767>

WebKit Commit Bot

Comment 30 2016-07-26 23:39:30 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.