Bug 168233 - [WebRTC] libwebrtc socket factory is not assigning the right socket type
Summary: [WebRTC] libwebrtc socket factory is not assigning the right socket type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-13 10:08 PST by youenn fablet
Modified: 2017-02-13 10:39 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.63 KB, patch)
2017-02-13 10:09 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-02-13 10:08:02 PST
As spotted by Brent, libwebrtc socket factory is not assigning the right socket type.
Comment 1 youenn fablet 2017-02-13 10:09:06 PST
Created attachment 301358 [details]
Patch
Comment 2 Brent Fulgham 2017-02-13 10:10:45 PST
Comment on attachment 301358 [details]
Patch

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

It seems like we should have some kind of test for these code paths; I only stumbled upon this by chance.

> Source/WebKit2/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:64
> +    auto socket = std::make_unique<LibWebRTCSocket>(*this, ++s_uniqueSocketIdentifier, LibWebRTCSocket::Type::UDP, socketAddress, rtc::SocketAddress());

I approve this change! :-)

> Source/WebKit2/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:81
> +    auto socket = std::make_unique<LibWebRTCSocket>(*this, ++s_uniqueSocketIdentifier, LibWebRTCSocket::Type::ClientTCP, localSocketAddress, remoteSocketAddress);

Hey! You caught one I missed! :-)
Comment 3 youenn fablet 2017-02-13 10:29:22 PST
(In reply to comment #2)
> Comment on attachment 301358 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301358&action=review
> 
> It seems like we should have some kind of test for these code paths; I only
> stumbled upon this by chance.

I totally agree!
I would like to test the whole code path: WebKit2 WebProcess creating webrtc sockets and calling NetworkProcess to do that.
But then, the bots might not like that we do networking, although maybe 127.0.0.1 networking might work and be good enough for that purpose.
Comment 4 WebKit Commit Bot 2017-02-13 10:39:04 PST
Comment on attachment 301358 [details]
Patch

Clearing flags on attachment: 301358

Committed r212232: <http://trac.webkit.org/changeset/212232>
Comment 5 WebKit Commit Bot 2017-02-13 10:39:08 PST
All reviewed patches have been landed.  Closing bug.