Change HttpChannelChild to open IPDL as part of construction (i.e. earlier than AsyncOpen)

REOPENED
Assigned to

Status

()

defect
P3
normal
REOPENED
3 years ago
6 months ago

People

(Reporter: mayhemer, Assigned: valentin)

Tracking

(Depends on 1 bug)

Trunk
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 affected, firefox65 affected)

Details

(Whiteboard: [necko-next])

Attachments

(1 attachment)

Reporter

Description

3 years ago
Because adding a document load channel to the load group triggers (among others) webnavigarion::onstatechange (bubbles up through docshell) and consumers want to call methods of nsIChannel on the request (our child channel) that is passed along those notifications.

We can either queue some of them (like suspend/resume) or simply open the parent channel a bit sooner.

Jason, can you find an owner?  You can freely bounce back to me.
Blocks: 1093535
Whiteboard: [necko-active] → [necko-backlog]
Reporter

Updated

3 years ago
Blocks: 1263793
Reporter

Comment 1

3 years ago
We have several bugs that need the IPC available sooner than before AsyncOpen.

Jason, can you find an assignee?  Again, I can work on this if no one else is available.
Flags: needinfo?(jduell.mcbugs)
Whiteboard: [necko-backlog] → [necko-active]
Reporter

Comment 2

3 years ago
Having this on-demand, like first method invoked that needs IPC will trigger the IPC ctor, will do.  We also then need to split out again AsyncOpen IPC method, as we used to have.
Assignee: jduell.mcbugs → valentin.gosu

Updated

3 years ago
Flags: needinfo?(jduell.mcbugs)

Comment 3

3 years ago
I think we should just construct the IPDL channel at the same time as the object is created, i.e. use the default IPDL-generated constructor instead of the one that takes a preconstructed object.

Updated

3 years ago
Summary: Http channel: Make IPC available at the time of AsyncOpen adds itself to its load group → Change HttpChannelChild to open IPDL as part of construction (i.e. earlier than AsyncOpen)
Depends on: 1124971
Bulk priority update: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1

Updated

2 years ago
Assignee: valentin.gosu → nobody
Priority: P1 → P2
Whiteboard: [necko-active] → [necko-next]
Valentin,

Since the code in HttpChannelChild has changed a lot, do you think we still need this bug?
Flags: needinfo?(valentin.gosu)
Not sure. I think the requirements in comment 0, but since things still work without it, I guess it's not _that_ critical.
P3, unless someone thinks otherwise.
Flags: needinfo?(valentin.gosu)
Priority: P2 → P3
Assignee

Updated

6 months ago
Assignee: nobody → valentin.gosu
Since we need the loadInfo to set up the IPDL connection, we move the logic to
do so from HttpChannelChild::AsyncOpen to HttpChannelChild::SetLoadInfo.
It would have been nicer to do so in HttpChannelChild::Init, but
I ran into issues with view-source channels, which required an ugly hack.

Also note that sometimes we call SetLoadInfo(mLoadInfo). This is because for
redirected service workers we sometimes close the IPDL channel and reopen it.
In this case we'd have a non-null mLoadInfo, but no IPDL channel - so we
need to call SetLoadInfo again to reopen the IPC connection.

Also note that RemoteChannelExists() preserves the existing contract - it is
true between asyncOpen and onStopRequest - but the name is slightly off, as
the channel has already been open by the time we call asyncOpen. We will fix
this in a follow-up.

Comment 14

6 months ago
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d15f0ac561d9
Change HttpChannelChild to open IPDL earlier than AsyncOpen r=kershaw,dragana

Comment 15

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d15f0ac561d9
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Depends on: 1510715
Assignee

Updated

6 months ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
You need to log in before you can comment on or make changes to this bug.