<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [Curl] WebSocket platform part is not implemented."
href="https://bugs.webkit.org/show_bug.cgi?id=144628#c12">Comment # 12</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [Curl] WebSocket platform part is not implemented."
href="https://bugs.webkit.org/show_bug.cgi?id=144628">bug 144628</a>
from <span class="vcard"><a class="email" href="mailto:peavo@outlook.com" title="peavo@outlook.com">peavo@outlook.com</a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=144628#c10">comment #10</a>)
<span class="quote">> Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=252729&action=diff" name="attach_252729" title="Patch">attachment 252729</a> <a href="attachment.cgi?id=252729&action=edit" title="Patch">[details]</a></span>
> Patch
> </span >
Thank you for your patience :)
<span class="quote">> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=252729&action=review">https://bugs.webkit.org/attachment.cgi?id=252729&action=review</a>
>
> lets not use this unnecessary std::unique_ptr
>
> > Source/WebCore/platform/network/curl/SocketStreamHandle.h:48
> > +#if PLATFORM(WIN)
> > +#include <windows.h>
> > +#include <winsock2.h>
> > +#endif
> > +
> > +#include <curl/curl.h>
> > +
> > +#include <mutex>
> > +
> > +#include <wtf/Deque.h>
> > #include <wtf/RefCounted.h>
> > +#include <wtf/Threading.h>
>
> These aren’t sorted in the normal WebKit project way.
> </span >
winsock2.h is needed before curl.h is included, so I kept this.
<span class="quote">>
> > Source/WebCore/platform/network/curl/SocketStreamHandleCurl.cpp:51
> > + startThread();
>
> Should probably ASSERT(isMainThread()) here.
>
> But also, why do we need to start the thread? I don’t think we do.
> </span >
startThread() seems to be needed here, many tests timed out without it.
<span class="quote">>
> > Source/WebCore/platform/network/curl/SocketStreamHandleCurl.cpp:70
> > + std::unique_ptr<const char[]> copy(new const char[length]);
> > + memcpy(const_cast<char*>(copy.get()), data, length);
>
> Should be able to allocate a std::unique_ptr<char[]> and then pass it to a
> function expecting a std::unique_ptr<const char[]> and have it be converted.
> We’d avoid a const_cast that way.</span >
>
MSVC didn't want to convert this, I got the error:
error C2664: 'std::unique_ptr<const char [],std::default_delete<_Ty>>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' :
cannot convert argument 1 from 'std::unique_ptr<char [],std::default_delete<_Ty>>' to 'std::unique_ptr<const char [],std::default_delete<_Ty>> &&'
<span class="quote">>
> > Source/WebCore/platform/network/curl/SocketStreamHandleCurl.cpp:200
> > + if (refCount() > 1)
> > + didOpenSocket();
>
> This check of refCount is peculiar. Is this an important optimization?
> </span >
I'm also not too happy with this :) I added it to fix a crash where the call was invoked on the main thread after all other references were released, and the SocketStreamClient was deleted. Accessing the SocketStreamClient in didOpenSocket() then caused a crash.
I had to add a SocketData constructor since m_sendData.append(SocketData { WTF::move(copy), length }) didn't compile with MSVC. The error I got was:
error C2440 : '<function-style-cast>' : cannot convert from 'initializer-list' to 'WebCore::SocketStreamHandle::SocketData'
1> No constructor could take the source type, or constructor overload resolution was ambiguous
I also had to add a move constructor, otherwise I got an error stating that
Deque.h (line 422) was trying to reference the deleted unique_ptr copy constructor.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>