[webkit-reviews] review granted: [Bug 217457] Implement Blob.stream : [Attachment 410801] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 217457: Implement Blob.stream
https://bugs.webkit.org/show_bug.cgi?id=217457

Attachment 410801: Patch

https://bugs.webkit.org/attachment.cgi?id=410801&action=review




--- Comment #2 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 410801
  --> https://bugs.webkit.org/attachment.cgi?id=410801
Patch

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

> Source/WebCore/fileapi/Blob.cpp:266
> +	       controller().enqueue(m_loader->takeRawData());

This is fine as is for now.
In the future, if we have some piping operations, we might actually want to
enqueue only after the first pull operation.

> Source/WebCore/fileapi/Blob.cpp:275
> +	       controller().close();

Do we have a test case for this one?
I do not think we want to call close after error.
I would guess we could try something like: create a large blob. Try stream it
and synchronously kill the network process.

> Source/WebCore/fileapi/Blob.idl:46
> +    [NewObject, CallWith=ScriptExecutionContext, MayThrowException]
ReadableStream stream();

I think it makes sense to use CallWith=ScriptExecutionContext instead of the
blob context.
Do we have a test for that though? Something like frame A creates a blob that i
used by frame B to create a stream while frame A gets destroyed.
That also leads to the question whether Blob needs to be an ActiveDOMObject?