Bug 217457

Summary: Implement Blob.stream
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, esprehn+autocc, ews-watchlist, kondapallykalyan, mek, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Alex Christensen 2020-10-07 16:47:01 PDT
Implement Blob.stream
Comment 1 Alex Christensen 2020-10-07 16:47:54 PDT
Created attachment 410801 [details]
Patch
Comment 2 youenn fablet 2020-10-08 00:51:10 PDT
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?
Comment 3 Alex Christensen 2020-10-08 10:13:06 PDT
Created attachment 410858 [details]
Patch
Comment 4 Alex Christensen 2020-10-08 10:45:00 PDT
(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.
Comment 5 Alex Christensen 2020-10-08 16:28:52 PDT
http://trac.webkit.org/r268228
Comment 6 Radar WebKit Bug Importer 2020-10-08 16:29:16 PDT
<rdar://problem/70116459>
Comment 7 Alex Christensen 2020-10-12 12:36:49 PDT
*** Bug 196258 has been marked as a duplicate of this bug. ***