deadbeef@: any thoughts or concerns?
My plan is to first convert from IPC to mojo, and later investigate onion souping it (I'm not sure what that would entail at the moment).
I don't think I have enough knowledge/context to give a meaningful answer; can you explain more about what your plans are? It sounds like you'd be replacing "ipc_network_manager" and "ipc_socket_factory" with Mojo equivalents, which I don't see the problem with as long as the functionality remains the same. But I don't know what your plans are around "onion souping".
From what I've briefly gathered, the "Onion Soup" plan is to move //content/renderer/ code into Blink. But //content/renderer/p2p is a dependency of webrtc, so won't that need to move first? (crbug.com/787254)
Sorry about the missing context. I don't have a detailed plan yet, but at a high level it is to delete p2p_messages.h and replace those messages with a couple of mojo interfaces that result in identical functionality. My first approach is going to be a direct mapping of the existing IPC messages to mojo, leaving the existing structure largely intact (i.e., where there is a Send() today there will be a call on an InterfacePtr, and where there is a message filter today the class will instead implement a mojo interface). After doing a direct conversion I'm sure other refactoring opportunities will become clear. I'd probably prefer to iterate in code review rather than writing a detailed plan in advance. WDYT?
I've removed onion souping from the bug for now because as you say, this probably depends on webrtc being onion souped first (and we can track that p2p may still need onion souping in the spreadsheet https://goo.gl/809bwy).
No need to be sorry; I just don't work in chromium often so I'm a little clueless.
Anyway, that sounds good to me. As for how it fits in with the net service, I believe that may be covered by this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=721401
So, adding xunjieli@ (owner of that bug). The trouble I imagine is that WebRTC adds some metadata with packets it sends (rtc::PacketOptions and numeric ID), and does WebRTC-specific things on top of the raw UDP/TCP sockets. xunjieli@, has this been discussed before? If so, is the plan to have a separate "p2p" service, or include the WebRTC socket classes in the net service, or something else?
Re #7:
WebRTC's usecase of //net falls into two cases:
- content::P2PSocketHostTcpBase
This uses ProxyResolvingClientSocket. ProxyResolvingClientSocket was a part of jingle glue, but we have since moved it to network service in r526510. I am currently fixing remaining issues in ProxyResolvingClientSocket. We plan to expose the functionalities through mojo. Note that WebRTC also uses this class to upgrade to TLS, which I plan to handle.
- content::P2PSocketHostUdp
This uses UDP socket APIs. I am currently adding mojo UDP socket APIs that will be exposed by the network service. watk@ or deadbeef@, if you can help to convert content::P2PSocketHostUdp to mojo UDP socket APIs (once my CL lands), that will be great.
If you know any WebRTC's use of network service that doesn't fall into these two, please let me know. I only did a code search, so I might miss something.
We are not including WebRTC socket classes (other than ProxyResolvingClientSocket) in the network service. I am not sure about other questions.
+jam, mmenke@ who might know more.
My last question was mostly about where P2PSocketHostTcpBase and P2PSocketHostUdp themselves (content/browser/renderer_host/p2p) will end up; if not the net service, I assume they'd be in a "p2p" service, running in the same process such that no extra copies of packets are needed.
I'm switching teams, and this task is bigger than I estimated, so I won't be able to complete this unfortunately. :( I'll assign it to you deadbeef@ to do you what you will with this bug.
Here's what I've done so far. It could be a useful starting point for whoever picks this up next. I was starting with a direct translation of the existing IPC, so it doesn't look much like a real mojo interface yet.
https://chromium-review.googlesource.com/c/chromium/src/+/879981
Comment 1 by w...@chromium.org
, Jan 9 2018