1485353 - Create PHttpTransaction IPC protocol

Status: RESOLVED FIXED

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

Description

[Honza Bambas (:mayhemer)]

This will be the basic class to send information and notifications between the parent process and the socket process.

There are few methods on nsHttpConnectionMgr that now take nsHttpTransaction.  We will add companion methods to those to take PHttpTransactionParent (I'll file a separate bug).

The PHttpTransactionParent should send down the necessary information, see nsHttpTransaction::Init method for the list of args it now accepts.

The PHttpTransactionChild will create and wrap the "real" nsHttpTransaction and let it live on the socket process.  It will also be responsible for notifications from the the data pipe (onstart/ondata/onstop) to the parent with necessary transaction properties serialization (like response headers, response code etc.)

At this early stage I would not care much about notification callbacks.

Transport event sink should be easy to impl.

Comment 1

[Junior [inactive]]

1. nsProxyInfo is not easy to serialize, and moving proxy resolution to socket process is to be determined.
   Would not support proxy now. Therefore we can serialize connectionInfo without mProxyInfo 

2. Will not handle security callback in the first version.

3. ResponseHead is already serializable. We can do the same thing to RequestHead.

4. EventTarget [2] for nsTransaction::Init is used to destruct at the right thread. We can remove it.

5. Should move the mTransactionPump to the HttpTransactionChild side. 

[1] https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/netwerk/protocol/http/nsHttpConnectionInfo.h#169

[2] https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/netwerk/protocol/http/nsHttpTransaction.cpp#245

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to Junior Hsu from comment #1)
> 1. nsProxyInfo is not easy to serialize, and moving proxy resolution to
> socket process is to be determined.
>    Would not support proxy now. Therefore we can serialize connectionInfo
> without mProxyInfo 
> 
> 2. Will not handle security callback in the first version.
> 
> 3. ResponseHead is already serializable. We can do the same thing to
> RequestHead.
> 
> 4. EventTarget [2] for nsTransaction::Init is used to destruct at the right
> thread. We can remove it.
> 
> 5. Should move the mTransactionPump to the HttpTransactionChild side. 
> 
> [1]
> https://searchfox.org/mozilla-central/rev/
> 55da592d85c2baf8d8818010c41d9738c97013d2/netwerk/protocol/http/
> nsHttpConnectionInfo.h#169
> 
> [2]
> https://searchfox.org/mozilla-central/rev/
> 55da592d85c2baf8d8818010c41d9738c97013d2/netwerk/protocol/http/
> nsHttpTransaction.cpp#245

Thanks!  This all sounds about right and fine.

The only bit I'm not sure about is why to move mTransactionPump to the HttpTransactionChild.  It would be great to not use a pump on the socket process at all EVENTUALLY to save dispatch from the socket thread to the main (or some other) thread, pump only works that way - async, AFAIK, but worth double checking.  Better would be to push the data from socket/h2-stream input streams directly to the appropriate IPC interface right on the socket thread.  But that of course, can be done in a followup.

Comment 3

[Junior [inactive]]

Attachment: Bug 1485353 - P1 create HttpTransactionParent/Child to pretend IPC

Comment 4

[Junior [inactive]]

Attachment: Bug 1485353 - P2 disable passing objects which is used by both processes

comment out those TODO codes
I can load bbc, cnn here. Sounds not bad, right?
I couldn't bear to run the test...

Comment 5

[Kershaw Chang [:kershaw]]

Attachment: Bug 1485353 - P3: Create PHttpTransacation.ipdl

This is just a WIP patch.
To enable socket process, please type: "MOZ_LOG=socketprocess:5 NECKO_DEDICATED_PROCESS=1 ./mach run".

Comment 6

[Junior [inactive]]

Attachment: Bug 1485353 - P4 facilitate opt-out remote nsHttpTransaction

create a nsAHttpTransactionShell to abstract the transaction operation in the parent process

I know it's not compile-able now.
However, is it possible to quickly check the setup of the shell interface and nsHttpChannel?
And xpcshell-test can't initialize the socket process hence I mark-out the |IsSocketProcessConnected| check.

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 9005781 [details]
Bug 1485353 - P1 create HttpTransactionParent/Child to pretend IPC

Honza Bambas (:mayhemer) has approved the revision.

Comment 8

[Honza Bambas (:mayhemer)]

Comment on attachment 9009806 [details]
Bug 1485353 - P2 disable passing objects which is used by both processes

Honza Bambas (:mayhemer) has approved the revision.

Comment 9

[Honza Bambas (:mayhemer)]

Comment on attachment 9010012 [details]
Bug 1485353 - P3: Create PHttpTransacation.ipdl

Honza Bambas (:mayhemer) has approved the revision.

Comment 10

[Honza Bambas (:mayhemer)]

Comment on attachment 9010494 [details]
Bug 1485353 - P4 facilitate opt-out remote nsHttpTransaction

Honza Bambas (:mayhemer) has approved the revision.

Comment 11

[Honza Bambas (:mayhemer)]

Junior, Kershaw, please let me know when this is finally ready to land (when you are done with any last updates to the patches), thanks.

Comment 12

[Junior [inactive]]

Attachment: Bug 1485353 - (rebased) P3 Create PHttpTransacation.ipdl (written by Kershaw D6182)

Rebase patch from central to larch, Original patch is D6182 written by Kershaw.

  1. Create PHttpTransacation.ipdl
  2. Implement necessary IPC messages, like OnStartRequest, OnDataAvailable, and OnStopRequest
  3. Add support to use OptionalIPCStream between parent and socket process
  4. Can load simple web page now, like example.com

Comment 13

[Junior [inactive]]

Attachment: Bug 1485353 - P5 reverse the env setting for NECKO_DEDICATED_PROCESS

Only specifying NECKO_DEDICATED_PROCESS=0 will disable socket process

Comment 14

[Kershaw Chang [:kershaw]]

(In reply to Honza Bambas (:mayhemer) from comment #11)
> Junior, Kershaw, please let me know when this is finally ready to land (when
> you are done with any last updates to the patches), thanks.

I think P3 is ready to land.

Comment 15

[Junior [inactive]]

We can land them after P4, P5 are reviewed.
I know P4 is reviewed once, but it would be better to have it seen again.

Comment 16

[Honza Bambas (:mayhemer)]

Comment on attachment 9010494 [details]
Bug 1485353 - P4 facilitate opt-out remote nsHttpTransaction

Honza Bambas (:mayhemer) has requested changes to the revision.

Comment 17

[Honza Bambas (:mayhemer)]

Comment on attachment 9011159 [details]
Bug 1485353 - P5 reverse the env setting for NECKO_DEDICATED_PROCESS

Honza Bambas (:mayhemer) has approved the revision.

Comment 18

[Honza Bambas (:mayhemer)]

Comment on attachment 9010494 [details]
Bug 1485353 - P4 facilitate opt-out remote nsHttpTransaction

Honza Bambas (:mayhemer) has approved the revision.

Comment 19

[Honza Bambas (:mayhemer)]

Fixed on larch:
https://hg.mozilla.org/projects/larch/rev/9fe4e62bf126e352a5c90b642036ccfb4b5c26d6
https://hg.mozilla.org/projects/larch/rev/48f1278159e8d6eca627f1d8b4a5090b686e3720
https://hg.mozilla.org/projects/larch/rev/8b4a0ada48e7985a72a2815390df1525e636a19e
https://hg.mozilla.org/projects/larch/rev/322a2074e4f0ced8ba6994789243b187046db1c0
https://hg.mozilla.org/projects/larch/rev/05c4fc918e06d535cd256d6e53243268caf54d20