[Webkit-unassigned] [Bug 172630] Add support for wss:// websockets to wincairo

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=172630

Alex Christensen <achristensen at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #311335|review?, commit-queue?      |review-, commit-queue-
              Flags|                            |

--- Comment #2 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 311335
  --> https://bugs.webkit.org/attachment.cgi?id=311335
Patch

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

> Source/WebCore/ChangeLog:12
> +        Add support for wss:// websockets to wincairo

This is a great thing.  Sorry I hadn't noticed this patch before now.  Let's polish up this code and get it in.

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:50
> +static CString certificatePath()

This code exists already in CurlContext.cpp, and that's probably a better version because it correctly surrounds the CF code with #if USE(CF).  If we need to use it here, let's instead put String certificatePath() in a header somewhere and use the one copy of the code.

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:78
> +    , m_certificatePath(certificatePath())

This already exists on CurlContext::singleton(). There's no need for a SocketStreamHandleImpl to keep the path of the certificate file.

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:216
> +        callOnMainThread([this, ret] {
> +            // Check reference count to fix a crash.
> +            // When the call is invoked on the main thread after all other references are released,
> +            // the SocketStreamClient is already deleted. Accessing the SocketStreamClient will then cause a crash.
> +            if (refCount() > 1) {

This is bad.  If we need to prevent use-after-free bugs when calling something on the main thread, then we should capture protectedThis = makeRef(*this) to keep the SocketStreamHandleImpl alive.  I'm not sure this is a good design, though.  I see you're just moving code, but I think this should be improved.

-- 
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/20170705/8bcff42e/attachment-0001.html>