Closed Bug 1544378 Opened 2 years ago Closed 3 months ago

[socket process] Implement nsIThreadRetargetableRequest for HttpTransactionParent

Categories

(Core :: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: kershaw, Assigned: kershaw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

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

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

(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.

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

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

(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.
Assignee: nobody → mbansal
Status: NEW → ASSIGNED

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.

Assignee: mbansal → 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
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d066dd84515
Implement RetargetDeliveryTo for HttpTransactionParent r=dragana
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
You need to log in before you can comment on or make changes to this bug.