1509471 - Support bug 1203503 (HTTP proxy connection with authentication for WebRTC) also with the socket process

Status: RESOLVED DUPLICATE of bug 1545827

(Core :: Networking: HTTP, enhancement, P3)

Description

[Honza Bambas (:mayhemer)]

In that bug we are providing raw sockets tunneled via a proxy CONNECT for general use, in this case specifically for WebRTC consumers.  

The total lack of comments in [1] drives me crazy.  I don't fully understand how that works and what, if anything, has to be adopted when we have the socket process.

Byron, you have reviewed that patch.  Can you please describe what exactly the `patch 2` [1] does?  I want to understand how it works and how the code paths look like to make the decision what needs to be updated to make this work with SPI (socket process isolation). 

Also, we need to set a priority on this.  It's kinda sad to fix that proxy bug and then break it again with the SPI work :)  But it's also a corner case.

Thanks.


[1] https://bugzilla.mozilla.org/page.cgi?id=splinter.html&ignore=&bug=1203503&attachment=9025366

Comment 1

[Byron Campen [:bwc]]

Part 2 creates a new IPDL interface called PWebrtcProxyChannel that the webrtc implementation can use to send/receive data via a http proxy. Parent and child implementations are written (WebrtcProxyChannelParent/Child). I am not totally sure why WebrtcProxyChannel and WebrtcProxyChannelParent weren't a single class to begin with, but this gives us the necessary flexibility to run an isolated socket process or not (depending on prefs).

On the mtransport side of things, we create a new subclass of NrSocket called NrSocketProxy that will use this new IPDL interface to send/receive packets. It is configured with an NrSocketProxyConfig object.

Sadly, the headers generated by idpl cannot be included by mtransport, because of typedef conflicts. So, WebrtcProxyChannelWrapper uses the PImpl idiom to hide WebrtcProxyChannelChild (and the headers it must include to get its base-class) from mtransport. It also handles the main/STS thread dispatching between WebrtcProxyChannelChild and mtransport. Think of this class as the mediator between the ipdl child code and mtransport.

Because we now have a handful of classes that need to implement the same child interface _without_ inheriting from PWebrtcProxyChannelChild, we create a base-class for those functions (WebrtcProxyChannelCallback). This also makes unit-testing a little easier.


The questions I have wrt socket process isolation are as follows:

Do we still need to use IPC from the socket process to the parent for this proxy channel stuff, or can we open the channel and use it directly in the socket process? If we still need to IPC to the parent, is that going to be a separate interface because the "child" side is now the socket process? (Remember; we will need to be able to operate in the old way for a while, and use prefs to switch over.)

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to Byron Campen [:bwc] from comment #1)
> The questions I have wrt socket process isolation are as follows:
> 
> Do we still need to use IPC from the socket process to the parent for this
> proxy channel stuff, or can we open the channel and use it directly in the
> socket process? 

You will definitely need to go to the parent process.  The channel has a lot of things to do on the parent process (proxy authentication, the proxy resolution, tun of small stuff that simply can work only on the parent process.)  And the created socket will live on the parent process too!  Hence, PWebrtcProxyChannel will IMO be used as is now.

If you need the socket to live on the _socket process_ (and not proxied via the existing WebrtcProxyChannel code to the parent process) we have to finish the http protocol separation work to at least some extent and use only it for the WebRTC CONNECT case...  this is way more work.

> If we still need to IPC to the parent, is that going to be a
> separate interface because the "child" side is now the socket process?

As said above, the socket process will be in the same position as a content process in this model.

> (Remember; we will need to be able to operate in the old way for a while,
> and use prefs to switch over.)

If SPI-WebRTC is off, then it will go the existing and now working path content process <-> parent process.

Comment 3

[Byron Campen [:bwc]]

(In reply to Honza Bambas (:mayhemer) from comment #2)
> (In reply to Byron Campen [:bwc] from comment #1)
> > The questions I have wrt socket process isolation are as follows:
> > 
> > Do we still need to use IPC from the socket process to the parent for this
> > proxy channel stuff, or can we open the channel and use it directly in the
> > socket process? 
> 
> You will definitely need to go to the parent process.  The channel has a lot
> of things to do on the parent process (proxy authentication, the proxy
> resolution, tun of small stuff that simply can work only on the parent
> process.)  And the created socket will live on the parent process too! 
> Hence, PWebrtcProxyChannel will IMO be used as is now.

   So you're saying that the socket process is similar enough to a content process that it can use the PWebrtcProxyChannel interface as the "child"? If so, that's great. Makes things easier.

> If you need the socket to live on the _socket process_ (and not proxied via
> the existing WebrtcProxyChannel code to the parent process) we have to
> finish the http protocol separation work to at least some extent and use
> only it for the WebRTC CONNECT case...  this is way more work.

   Ok. Given that we need to continue supporting mtransport-in-content for a while, this is fine. Once we're ready to stop supporting mtransport-in-content, we can revisit this.

Comment 4

[Dragana Damjanovic [:dragana]]

Form our conversation yesterday this is the bug you wanted to take a look