Implement Blob.stream

[Alex Christensen]

Implement Blob.stream

[Alex Christensen]

Created attachment 410801 [details]
Patch

[youenn fablet]

Comment on attachment 410801 [details]
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?

[Alex Christensen]

Created attachment 410858 [details]
Patch

[Alex Christensen]

(In reply to youenn fablet from comment #2)
> Comment on attachment 410801 [details]
> 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.
You're right.  The test showed that we don't want this close call.

> > 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.
This is indeed fine as it is.  We want the ScriptExecutionContext of the current Blob/ReadableStream.  I added a test.
> That also leads to the question whether Blob needs to be an ActiveDOMObject?
I don't think so.

[Alex Christensen]

http://trac.webkit.org/r268228

[Radar WebKit Bug Importer]

<rdar://problem/70116459>

[Alex Christensen]

*** Bug 196258 has been marked as a duplicate of this bug. ***