1544378 - [socket process] Implement nsIThreadRetargetableRequest for HttpTransactionParent

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Kershaw Chang [:kershaw]]

Currently, HttpTransactionParent doesn't implement nsIThreadRetargetableRequest methods.
I think this is some kind of optimization.

Comment 1

[Dragana Damjanovic [:dragana]]

nsIThreadRetargetableRequest is you as a interface to check if listeners allow OnDataAvailable off the main thread. We do not need it for HttpTransactionParent.

Comment 2

[Dragana Damjanovic [:dragana]]

(In reply to Dragana Damjanovic [:dragana] from comment #1)

nsIThreadRetargetableRequest is you as a interface to check if listeners allow OnDataAvailable off the main thread. We do not need it for HttpTransactionParent.

httpChannel listeners.

Comment 3

[Kershaw Chang [:kershaw]]

I mean we might have to implement RetargetDelivertTo like [1].
If e10s is disabled, I think we have to support run ODA off main thread.

[1] https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516affe4d28cb7fa220a/netwerk/base/nsInputStreamPump.cpp#695

Comment 4

[Mahak]

hey @Kershaw
Can I take this up ?
Could you elaborate more on this ?
Thanks
Mahak :)

Comment 5

[Kershaw Chang [:kershaw]]

(In reply to Mahak from comment #4)

hey @Kershaw
Can I take this up ?
Could you elaborate more on this ?
Thanks
Mahak :)

Thanks for offering this help.

Here are some steps for letting HttpTransactionParent support nsIThreadRetargetableRequest.

1. Let HttpTransactionParent derive nsIThreadRetargetableRequest and implement the required functions. See what we did in HttpChannelChild would be helpful.
2. Please refer to the implementation of RetargetDeliveryTo at here. We should just simply save the target in HttpTransactionParent.
3. Let's modify HttpTransactionParent::GetNeckoTarget to let it return the event target we saved in step 2.
4. I think the best way to test this is turn off e10s (set browser.tabs.remote.autostart to false) and enable http over socket process (set network.http.network_access_on_socket_process.enabled to true). Then, turn on http log and load some websites to see if nsHttpChannel::OnDataAvailable is really called off main thread.

Comment 6

[Mahak]

Attachment: Bug 1544378 - Implement nsIThreadRetargetableRequest for HttpTransactionParent.r?kershaw

Comment 7

[Kershaw Chang [:kershaw]]

Steal this from Mahak, since there is no activity for more than two months.

Mahak, I really appreciate your previous work. I need this bug to be fixed soon, so I'd like to take this.

Comment 8

[Kershaw Chang [:kershaw]]

(In reply to Dragana Damjanovic [:dragana] from comment #1)

nsIThreadRetargetableRequest is you as a interface to check if listeners allow OnDataAvailable off the main thread. We do not need it for HttpTransactionParent.

I've seen a lot warnings below when running mochitest with socket process on try. It seems that some fetch code in parent process needs to retarget ODA off main thread even if e10s is enabled.

[task 2020-06-11T13:42:34.454Z] 13:42:34     INFO - GECKO(1591) | [Parent 1591, Main Thread] WARNING: 'NS_FAILED(rr->RetargetDeliveryTo(sts))', file /builds/worker/checkouts/gecko/dom/fetch/FetchDriver.cpp, line 1201

Comment 9

[Kershaw Chang [:kershaw]]

Attachment: Bug 1544378 - Implement RetargetDeliveryTo for HttpTransactionParent

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d066dd84515
Implement RetargetDeliveryTo for HttpTransactionParent r=dragana

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/6d066dd84515