Bug 181600

Created attachment 331422 [details] Patch PoC

youennf@gmail.com Could you please look if I'm implementing in correct way? Also there is a question what is correct way to test chunks handler functions? Currently I'm using part of the patch that is related to WebAssembly stream API, and check in debug if chunk handler is invoked.

(In reply to GSkachkov from comment #2) > youennf@gmail.com Could you please look if I'm implementing in correct way? > > Also there is a question what is correct way to test chunks handler > functions? I would start by using the chunk strategy in ServiceWorkerFetch.cpp. You can tweak LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/fetch-event-respond-with-readable-stream-worker.js to enqueue two chunks for instance.

Comment on attachment 331422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331422&action=review > Source/WebCore/Modules/fetch/FetchResponse.h:100 > + void consumeBodyByChunksFromReadableStream(ConsumeDataByChunksCallback&&); Instead of adding consumeBodyByChunksFromReadableStream, I would change the way consumeBodyFromReadableStream works. Currently, we are calling only once ConsumeDataCallback with an aggregated chunk. We could call it several times for each chunk. That would require to change the users of consumeBodyFromReadableStream: - ServiceWorkerFetch would send an IPC message for each callback call, the last being a didFinish message, the others sending chunks. - DOMCache would aggregate the content in a single body inside the callback. Given that, ConsumeDataCallback could be changed in something like a WTF::Function<void(ExceptionOr<Chunk>>, Chunk being a struct { const uint8_t*, size_t }. I would also tend to split this patch in two patches, one for handling ReadableStream bodies and one for handling loaded bodies.

<rdar://problem/36932547>

Created attachment 332593 [details] Patch

Comment on attachment 332593 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332593&action=review > Source/WebCore/Modules/cache/DOMCache.cpp:277 > + response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create()](auto&& result) mutable { I think it makes the code significantly easier to read to have the type of result here. > LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/fetch-event-respond-with-body-loaded-in-chunk-worker.js:6 > + event.respondWith(fetch("../../../fetch/api/resources/trickle.py?count=4&delay=50")); Does the delay do anything significant here?

Comment on attachment 332593 [details] Patch Thanks for the review. View in context: https://bugs.webkit.org/attachment.cgi?id=332593&action=review >> Source/WebCore/Modules/cache/DOMCache.cpp:277 >> + response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create()](auto&& result) mutable { > > I think it makes the code significantly easier to read to have the type of result here. OK, will add it back. >> LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/fetch-event-respond-with-body-loaded-in-chunk-worker.js:6 >> + event.respondWith(fetch("../../../fetch/api/resources/trickle.py?count=4&delay=50")); > > Does the delay do anything significant here? This is just a way to 'ensure' that network process will send response body as chunks to the service worker process. Then the service worker process will in turn send each chunk before response being finished loading.

Created attachment 332700 [details] Patch for landing

Comment on attachment 332700 [details] Patch for landing Clearing flags on attachment: 332700 Committed r227870: <https://trac.webkit.org/changeset/227870>

All reviewed patches have been landed. Closing bug.

Submitted web-platform-tests pull request: https://github.com/w3c/web-platform-tests/pull/9329