[webkit-reviews] review granted: [Bug 199189] WebSockets: add support for sending blob messages when using web sockets platform APIs : [Attachment 373095] Try to fix apple builds

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 28 11:35:02 PDT 2019

youenn fablet <youennf at gmail.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 199189: WebSockets: add support for sending blob messages when using web
sockets platform APIs

Attachment 373095: Try to fix apple builds


--- Comment #12 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 373095
  --> https://bugs.webkit.org/attachment.cgi?id=373095
Try to fix apple builds


Since we might want the same mechanism for DataChannel, I wonder though whether
we should not have a more generic mechanism.
Something like a Queue where we can push String/ArrayBuffer/Blob.
The queue could be query whether empty or not.
It would take a Function callback to notify that a Variant<String, ArrayBuffer>
is ready to be sent.

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

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:253
> +

As per spec, I think we should increase the buffered amount by blob.size()
And decrement it when receiving the IPC message.
This does not seem to be like this in the previous implementation though.
It might be nice to have a test for that and check what other browsers are

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:264
> +		   sendMessage(Messages::NetworkSocketChannel::SendData {
IPC::DataReference { reinterpret_cast<const uint8_t*>(binaryData.data()),
binaryData.size() } }, binaryData.size());

Maybe we should have routines for these sendMessage(text) sendMessage(binary).

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:277
> +		       fail(makeString("Failed to load Blob: error code = ",

I am not sure this error handling is good enough.
If there is no test coverage for that case, we should file a bug.

More information about the webkit-reviews mailing list