[Webkit-unassigned] [Bug 199189] WebSockets: add support for sending blob messages when using web sockets platform APIs

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 1 01:07:19 PDT 2019


--- Comment #13 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 373095
  --> https://bugs.webkit.org/attachment.cgi?id=373095
Try to fix apple builds

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() synchronously.
> 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 doing.

I thought about that, but decided to keep the current behaviour. Before the blob is loaded we haven't buffered any data yet, so I think it makes sense to increase it after the blob loaded.

>> 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).

Instead of the template or in addition to?

>> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:277
>> +                    fail(makeString("Failed to load Blob: error code = ", errorCode.value()));
> I am not sure this error handling is good enough.
> If there is no test coverage for that case, we should file a bug.

It's not easy to work with the tests when most of them are failing and we this code path is not used in the bots. If I add a new test for this, we might need to change the old code too. In any case, we are not generating console messages yet.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190701/003ec91e/attachment.html>

More information about the webkit-unassigned mailing list