[fetch] Do not collect all data when streaming requests.

[brendandahl]

Previously, when EMSCRIPTEN_FETCH_STREAM_DATA was used, we would collect all the streamed data chunks and create the full response when finished in the XHR polyfill. The full response is not used by the C fetch API, so we can avoid storing it when streaming is enabled.

[brendandahl]

See #25383 (comment)

[brendandahl]

/cc @inolen

[kripken]

Is this a breaking change?

[brendandahl]

Is this a breaking change?

No, the old code that used the firefox onprogress events wouldn't pass all the data when finished to the C API. There's a test for it in

emscripten/test/fetch/test_fetch_stream_file.cpp

Line 22 in 2747b2a

assert(fetch->data == 0); // The data was streamed via onprogress, no bytes available here.

[kripken]

But I'm still confused, the description says

Now we leave it up to the user to process the data and collect it as needed.

What if the user doesn't do that properly - could existing code not break?

[brendandahl]

Let me reword that... I was thinking of it from the perspective of the FetchXHR polyfill not the C API. The polyfill behavior does change (we no longer collect all the bytes), but this isn't visible to anyone using the emscripten C API.

[inolen]

Hey @brendandahl thanks! This looks great for not allocating the entire response when streaming. I was going to post a patch in that comment thread, but I got a bit distracted and haven't had time to run tests on my changes.

There are two other issues with the code right now that I noticed when using this with thousands of files. These aren't directly related to your new code, so I can always open a new PR if desired -

1.) saveResponseAndStatus leaks the allocated data when using the streaming mode. Since xhr.response is NULL, ptr / ptrLen remain 0, and it then overwrites ptr with 0 leaking the allocated data.
2.) the realloc's feel unnecessary (although functional) if the new buffer is smaller than the old.

Here was my current WIP state -
https://gist.github.com/inolen/8bfcb72c58061512475c69b84f75524d

[brendandahl]

I haven't looked into the other issues you mentioned yet. A separate PR would be good. We should probably run some of the fetch tests with lsan.

[kripken]

Now I see, thanks!