...
Created attachment 454068 [details] WIP Patch
The entry point is in ThreadableWebSocketChannel::create
Created attachment 454069 [details] WIP Patch
Created attachment 454282 [details] WIP Patch
Created attachment 454391 [details] Patch
Comment on attachment 454391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454391&action=review Looks mostly good. Thanks for upstreaming. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:46 > + if (request.url().protocolIs("wss") && WebCore::DeprecatedGlobalSettings::allowsAnySSLCertificate()) > + WebCore::CurlContext::singleton().sslHandle().setIgnoreSSLErrors(true); It would be great if we could do this per task or at least per domain. That's for a different patch, though. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:55 > +WebSocketTask::~WebSocketTask() > +{ > + destructStream(); > +} This might be a sign that the stream should be a smart-pointer-like object that cleans up when destroyed. It also might not. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:96 > +void WebSocketTask::cancel() > +{ > + > +} > + > +void WebSocketTask::resume() > +{ > + > +} Do these need to do anything? > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:119 > + handshakeMessage.remove(handshakeMessage.length() - 2, 2); // Remove the terminating '\r\n' > + handshakeMessage.append("Cookie: "); > + handshakeMessage.append(cookieHeaderField); > + handshakeMessage.append("\r\n\r\n"); This does 4 unnecessary string allocations and copies. If handshakeMessage were a StringBuilder or even a Vector<char> it would be more efficient. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:125 > + auto buffer = makeUniqueArray<uint8_t>(handshakeData.length()); > + memcpy(buffer.get(), handshakeData.data(), handshakeData.length()); This is another copy and allocation that might be able to be removed. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:156 > + auto frameResult = receiveFrames([this](WebCore::WebSocketFrame::OpCode opCode, const uint8_t* data, size_t length) { It would probably be worth capturing a WeakPtr<WebSocketTask> and returning early if it's null to be safe against using this after it has been freed. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:422 > + callOnMainRunLoop([this, code, reason] { Ditto, this may be deallocated by the time the lambda executes, which would be bad. > Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.h:64 > + enum class State { nit : uint8_t
<rdar://problem/90287981>
Pull request: https://github.com/webkit/webkit/pull/1071
Committed r294932 (251043@main): <https://commits.webkit.org/251043@main> Reviewed commits have been landed. Closing PR #1071 and removing active labels.
> Committed r294932 (251043@main): <https://commits.webkit.org/251043@main> WinCairo WK1 is crashing. Bug 241087 – [curl] REGRESSION(251043@main): "ASSERTION FAILED: channel" in WebCore::ThreadableWebSocketChannel::create