[webkit-reviews] review denied: [Bug 112314] Add support of sending blob data for RTCDataChannel. : [Attachment 193800] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Li Yin <li.yin at intel.com>'s request
for review:
Bug 112314: Add support of sending blob data for RTCDataChannel.
https://bugs.webkit.org/show_bug.cgi?id=112314

Attachment 193800: Patch
https://bugs.webkit.org/attachment.cgi?id=193800&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193800&action=review


> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:178
> +    m_outgoingBlobQueue.append(Blob::create(data->url(), data->type(),
data->size()));

Why do we need to create a new blob here?

> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:229
> +	   OwnPtr<Vector<char> > binaryData = adoptPtr(new
Vector<char>(dataLength));
> +	   memcpy(binaryData->data(), data, dataLength);
> +	   RefPtr<RawData> rawData = RawData::create();
> +	   binaryData->swap(*rawData->mutableData());
> +	   OwnPtr<BlobData> blobData = BlobData::create();
> +	   blobData->appendData(rawData.release(), 0,
BlobDataItem::toEndOfFile);
> +	   RefPtr<Blob> blob = Blob::create(blobData.release(), dataLength);

This all seems really low-level for this file.	Does Blob not know how to
create itself from a sequence of bytes?

The benefit of using a blob is that you don't need to bring all the data into
memory.  If you're going to bring all the data into memory anyway, you might as
well use an array buffer.  To have a high-quality blob implementation, we'd
need to pass an opaque handle to the data from its source all the way here.