Bug 144869 - [Streams API] ReadableStream reader should not be disposable when having pending promises
Summary: [Streams API] ReadableStream reader should not be disposable when having pend...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-11 06:31 PDT by youenn fablet
Modified: 2015-05-12 00:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.52 KB, patch)
2015-05-11 12:23 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (7.51 KB, patch)
2015-05-11 23:20 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2015-05-11 06:31:09 PDT
Currently, ReadableJSStream::Reader is disposed as soon as the JS scripts loose reference to it.
Once disposed, the stream is released, which forbids resolving promises when the stream is changing, like going to close state.
This happens even if the closed promise is actually set. 
The reader should be kept alive as long as there are pending promises to the reader and the stream can change of state.
Comment 1 youenn fablet 2015-05-11 12:23:49 PDT
Created attachment 252883 [details]
Patch
Comment 2 Darin Adler 2015-05-11 17:21:49 PDT
Comment on attachment 252883 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252883&action=review

> Source/WebCore/Modules/streams/ReadableStreamReader.cpp:80
> +    RefPtr<ReadableStreamReader> protect(this);

We should use Ref for this now, not RefPtr.

> Source/WebCore/bindings/js/ReadableJSStream.cpp:182
> +    Ref<Reader> reader =  adoptRef(*new Reader(stream));

Extra space on this line.
Comment 3 youenn fablet 2015-05-11 23:20:37 PDT
Created attachment 252942 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2015-05-12 00:17:04 PDT
Comment on attachment 252942 [details]
Patch for landing

Clearing flags on attachment: 252942

Committed r184159: <http://trac.webkit.org/changeset/184159>
Comment 5 WebKit Commit Bot 2015-05-12 00:17:09 PDT
All reviewed patches have been landed.  Closing bug.