Closed Bug 1446204 Opened 2 years ago Closed 2 years ago

Response body should be marked as used if used as stream

Categories

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

58 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file)

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'.
Here a patch where the stream is marked as used at the first call of RequestDataCallback.
Attachment #8959380 - Flags: review?(till)
Blocks: 1445587
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.
Attachment #8959380 - Flags: review?(till) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/947cf2126bb3
Response body should be marked as used if used as stream, r=till
https://hg.mozilla.org/mozilla-central/rev/947cf2126bb3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.