[Webkit-unassigned] [Bug 199151] [SOUP] Use libsoup WebSockets API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 26 22:06:59 PDT 2019


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

r=me as well.
Some comments below.

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

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.h:38
> +class WebSocketTask {

Some small code is shared between the soup and cocoa versions.
Maybe code should be shared.

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:164
> +void WebSocketChannel::didConnect(const String& subprotocol)


> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:173
> +        enqueueTask([this, subprotocol] {

WTFMove() and mutable

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:179
> +    m_subprotocol = subprotocol;


> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:237
> +void WebSocketChannel::didFail(const String& reason)


> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:243
> +        enqueueTask([this, reason] {

WTFMove and mutable

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:249
> +    // FIXME: do something with reason.

reason might not be a great name since it relates to didClose.
Maybe errorMessage might be better.

There is a preexisting issue as well in the sense we might usually need to call didClose after didFail to trigger the onclose event.
This is not really consistent with other code where didFail is final.
Maybe we should rename didFail to didReceiveMessageError.

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/20190627/c4268252/attachment.html>

More information about the webkit-unassigned mailing list