[webkit-reviews] review denied: [Bug 213168] Implement Blob.text and Blob.arrayBuffer : [Attachment 402103] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has denied Tetsuharu Ohzeki
<tetsuharu.ohzeki at gmail.com>'s request for review:
Bug 213168: Implement Blob.text and Blob.arrayBuffer
https://bugs.webkit.org/show_bug.cgi?id=213168

Attachment 402103: Patch

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




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

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

> Source/WebCore/ChangeLog:14
> +	   without `ReadableStream` implementation.

Since the goal is to go with supporting ReadableStream at some point, we should
probably extract existing functionality from FetchBodyOwner/FetchBoydConsumer.
Ideally, split FetchBodyOwner in two classes, one that will continue be the
base class from FetchRequest and FetchResponse, plus be an ActiveDOMObject.
And the other one which can be reused jointly with FetchBodyConsumer to
implement text/arrayBuffer/ReadableStream.

> Source/WebCore/fileapi/BlobPromiseLoader.cpp:65
> +    delete this;

This is a risky pattern.
I do not think we need BlobPromiseLoader to be an ActiveDOMObject.
It might be best to make Blob own the BlobPromiseLoader.