[webkit-reviews] review denied: [Bug 172630] [Curl] Add support for wss:// websockets : [Attachment 345403] Fix style

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 19 21:08:14 PDT 2018


Fujii Hironori <Hironori.Fujii at sony.com> has denied Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 172630: [Curl] Add support for wss:// websockets
https://bugs.webkit.org/show_bug.cgi?id=172630

Attachment 345403: Fix style

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




--- Comment #18 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 345403
  --> https://bugs.webkit.org/attachment.cgi?id=345403
Fix style

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

> Source/WebCore/ChangeLog:8
> +	   To support secure WebSocket connection, it is almost rewriten by
using existing

rewriten → rewritten
https://eow.alc.co.jp/search?q=rewriten
https://eow.alc.co.jp/search?q=rewritten

> Source/WebCore/ChangeLog:11
> +	   Enable exsisting tests to cover this implementation.

I want to check regressions caused by landing this patch.
Please, unskip the test cases by unreviewed gardening commit beforehand.
And, land this patch after waiting for at least one LayoutTests result on
BuildBots.

> Source/WebCore/platform/network/curl/SocketStreamHandleImpl.h:92
> +    std::array<uint8_t, kLocalSendBufferSize> m_writeBuffer;

std::array has size() constepre method. You can replace kLocalSendBufferSize
with m_writeBuffer.size() if you like.
std::array<uint8_t, 16 * 1024> m_writeBuffer;

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:90
> +    memcpy(&m_writeBuffer[0], data, length);

memcpy(m_writeBuffer.data(), data, length);

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:140
> +	       auto bytesSent = socket.send(data, state.remainingSize());

auto bytesSent = socket.send(&m_writeBuffer[state.offset],
state.remainingSize());

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:184
> +	       m_client.didFailSocketStream(*this,
SocketStreamError(static_cast<int>(errorCode), { }, localizedDescription));

You don't need to include localizedDescription into the lambda and pass.
m_client.didFailSocketStream(*this,
SocketStreamError(static_cast<int>(errorCode), { },
CurlHandle::errorDescription(errorCode))


More information about the webkit-reviews mailing list