1467223 - Modify the process-switching load code to support POST payloads on loads

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Comment 1

[Marion Daly [:mdaly]]

Thomas: Can we get details here? ready criteria/etc?

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 1: Move CrossProcessRedirect Message to PContent,

This is needed because early in a content process's lifecycle, NeckoParent may
not have been created yet. This leads to issues when trying to redirect into a
fresh process which hasn't performed network loads yet. By sending the message
over PContent, we can be sure the APIs are available.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 2: Add BrowsingContextID to LoadInfo,

This is handy when performing process swaps, as it provides useful & important
information to parent-process callers.

Depends on D15608

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 3: Support uncached resources by using a redirectTo-like API for process swaps in necko,

With the old process selector service implementation, non-cached loads would skip the call into the process selector. 

While reading through the various places which I would need to add check calls,
I noticed that a similar mechanism already existed: the 'redirectTo' method on
nsIHttpChannel. This code piggybacks on that implementation to support
injecting process swaps.

(NOTE: Still WIP)

Depends on D15609

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 4: Add support for piping redirected channels through nsDocShell's loading functions,

This code largely skips the logic in load methods, and tries to simply get the
channel opened & connected to the correct listener ASAP, without breaking any
loading state.

(NOTE: Still WIP)

Depends on D15610

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 5: Perform parent-process interception for HTTP loads,

This will only happen if the pref is enabled, and works through the existing
mechanism for process switching loads.

This currently encounters an issue where, because the process changing logic
destroys the originating nsDocShell before the channel has been reconnected
into the new process. This causes the channel to be closed, and the load to not
succeed.

(NOTE: Still WIP)

Depends on D15611

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 6: Add basic test for process-changing POST loads,

Depends on D15612

Comment 8

[Nika Layzell [:nika] (ni? for response)]

As I mention in some of the above comments, I am running into issues due to the channel being closed by the original TabChild as it is shut down during the process swap. It seems as though the redirect process doesn't proceed far enough before the new TabParent is provided for the redirect to proceed smoothly.

Valentin, do you know what changes would need to be made to allow this to work? I have tried to figure out how the HTTP loading code works, but I am not yet confident in my understanding & don't know what I would need to set up.

Comment 9

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

(In reply to :Nika Layzell from comment #8)

As I mention in some of the above comments, I am running into issues due to
the channel being closed by the original TabChild as it is shut down during
the process swap. It seems as though the redirect process doesn't proceed
far enough before the new TabParent is provided for the redirect to proceed
smoothly.

Valentin, do you know what changes would need to be made to allow this to
work? I have tried to figure out how the HTTP loading code works, but I am
not yet confident in my understanding & don't know what I would need to set
up.

From what I recall, the old channel is deleted by HttpChannelParentListener::OnRedirectResult() [1]
This is the big-picture of how redirects work. [2]
I'm thinking we should delay destroying the old docshell/tabchild until the redirect is completed, no? Or maybe add a way of destroying the tabChild that doesn't close the parent-side of the channel, so it gets a chance to complete the redirect?

[1] https://searchfox.org/mozilla-central/rev/76fe4bb385348d3f45bbebcf69ba8c7283dfcec7/netwerk/protocol/http/HttpChannelParentListener.cpp#322
[2] https://gist.github.com/valenting/ce444c31bb8ef2f53b9acfc3f59feedb

Comment 11

[Honza Bambas (:mayhemer)]

What's the review priority for me here?

Comment 12

[Honza Bambas (:mayhemer)]

see c11

Comment 13

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1467223 - Part 7: Renumber Continue* methods on nsHttpChannel,

Depends on D15613

Comment 14

[Nika Layzell [:nika] (ni? for response)]

(In reply to Honza Bambas (:mayhemer) from comment #12)

see c11

This is fairly high priority with the new revision just posted. These patches now past at least some tests & I would like to start being able to use the new process changing capabilities. It'd also be really nice to have your input as to how exactly the interception should be performed, especially considering the limitations which I'm thinking currently exist. If you have a good solution for working around those issues please let me know :-)

Comment 15

[Honza Bambas (:mayhemer)]

OK, I will do this as the first thing the next week (Monday).

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0b61b6a4014
Part 1: Move CrossProcessRedirect Message to PContent, r=valentin
https://hg.mozilla.org/integration/autoland/rev/881438d34f09
Part 2: Add BrowsingContextID to LoadInfo, r=valentin
https://hg.mozilla.org/integration/autoland/rev/c320f8c70785
Part 3: Support uncached resources by using a redirectTo-like API for process swaps in necko, r=valentin,mayhemer
https://hg.mozilla.org/integration/autoland/rev/47630659e925
Part 4: Add support for piping redirected channels through nsDocShell's loading functions, r=qdot
https://hg.mozilla.org/integration/autoland/rev/ae82e6f22e29
Part 5: Perform parent-process interception for HTTP loads, r=qdot,valentin
https://hg.mozilla.org/integration/autoland/rev/909797e624a8
Part 6: Add basic test for process-changing POST loads, r=qdot
https://hg.mozilla.org/integration/autoland/rev/4e5e0c80bae2
Part 7: Renumber Continue* methods on nsHttpChannel, r=valentin,mayhemer

Comment 17

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/b0b61b6a4014
https://hg.mozilla.org/mozilla-central/rev/881438d34f09
https://hg.mozilla.org/mozilla-central/rev/c320f8c70785
https://hg.mozilla.org/mozilla-central/rev/47630659e925
https://hg.mozilla.org/mozilla-central/rev/ae82e6f22e29
https://hg.mozilla.org/mozilla-central/rev/909797e624a8
https://hg.mozilla.org/mozilla-central/rev/4e5e0c80bae2