1446204 - Response body should be marked as used if used as stream

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Andrea Marchesini [:baku]]

Currently this is not true. We use an internal readableStream object to know if the stream is in use, but eventually that readableStream is released and the body becomes 'reusable'.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: stream_timeout.patch

Here a patch where the stream is marked as used at the first call of RequestDataCallback.

Comment 2

[Till Schneidereit [:till]]

Comment on attachment 8959380 [details] [diff] [review]
stream_timeout.patch

Review of attachment 8959380 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense to me, but note that I'm not a DOM reviewer. This seems simple enough that my r+ might be enough, but if you're unsure about that, perhaps get another review?

In any case, r=me with nits addressed.

::: dom/fetch/Fetch.h
@@ +245,5 @@
>  
> +  void
> +  MarkAsRead() override
> +  {
> +    mBodyUsed = true;

Should this have an assert that the stream's state is `eInitialize`?

::: dom/fetch/FetchStream.h
@@ +90,5 @@
>    // Common methods
>  
>    enum State {
> +    // This is the beginning state before any reading operation.
> +    eInitialize,

This name seems a bit confusing because "initialize" isn't really a state. Should it perhaps be eInitializing or eUninitialized? From the current name I'm not sure which of those would be more correct.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/947cf2126bb3
Response body should be marked as used if used as stream, r=till

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/947cf2126bb3