Bug 201302 - Abstract out LibWebRTCSocketClient so that rtc sockets can be implemented without libwebrtc sockets
Summary: Abstract out LibWebRTCSocketClient so that rtc sockets can be implemented wit...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-29 06:58 PDT by youenn fablet
Modified: 2019-09-01 23:21 PDT (History)
6 users (show)

See Also:


Attachments
Patch (16.47 KB, patch)
2019-08-29 07:02 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (14.27 KB, patch)
2019-08-29 08:18 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (14.25 KB, patch)
2019-09-01 13:11 PDT, 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 2019-08-29 06:58:10 PDT
Abstract out LibWebRTCSocketClient so that rtc sockets can be implemented without libwebrtc sockets
Comment 1 youenn fablet 2019-08-29 07:02:04 PDT
Created attachment 377579 [details]
Patch
Comment 2 youenn fablet 2019-08-29 08:18:32 PDT
Created attachment 377585 [details]
Patch
Comment 3 Alex Christensen 2019-08-30 13:44:10 PDT
Comment on attachment 377585 [details]
Patch

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

> Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:166
> +std::unique_ptr<NetworkRTCProvider::Socket> NetworkRTCProvider::takeSocket(uint64_t identifier)

A trailing return type here would remove the need for an extra NetworkRTCProvider::

> Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.h:74
> +        enum class Type { UDP, ServerTCP, ClientTCP, ServerConnectionTCP };

What's the difference between "ServerTCP" and "ServerConnectionTCP"?  Should "ServerTCP" be "ServerListeningTCP"?
Also, these can all fit in a uint8_t.
Comment 4 youenn fablet 2019-09-01 13:11:42 PDT
Created attachment 377828 [details]
Patch
Comment 5 youenn fablet 2019-09-01 13:14:52 PDT
(In reply to Alex Christensen from comment #3)
> Comment on attachment 377585 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377585&action=review
> 
> > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:166
> > +std::unique_ptr<NetworkRTCProvider::Socket> NetworkRTCProvider::takeSocket(uint64_t identifier)
> 
> A trailing return type here would remove the need for an extra
> NetworkRTCProvider::

In that case I do not really mind the extra NetworkRTCProvider prefix.

> 
> > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.h:74
> > +        enum class Type { UDP, ServerTCP, ClientTCP, ServerConnectionTCP };
> 
> What's the difference between "ServerTCP" and "ServerConnectionTCP"?  Should
> "ServerTCP" be "ServerListeningTCP"?

Right, the server TCP is listening and the ServerConnectionTCP is the connection created by a TCP client.
This matches more or less libwebrtc names.

> Also, these can all fit in a uint8_t.

OK
Comment 6 WebKit Commit Bot 2019-09-01 23:20:25 PDT
Comment on attachment 377828 [details]
Patch

Clearing flags on attachment: 377828

Committed r249376: <https://trac.webkit.org/changeset/249376>
Comment 7 WebKit Commit Bot 2019-09-01 23:20:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-09-01 23:21:16 PDT
<rdar://problem/54941595>