Closed Bug 1467223 Opened 6 years ago Closed 6 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Project Flags:
Fission Milestone M1
Tracking Flags:
Tracking Status
firefox66 --- fixed

People

(Reporter: relaas, Assigned: nika)

References

(Blocks 2 open bugs)

Details

Attachments

(7 files)

Bug 1467223 - Part 1: Move CrossProcessRedirect Message to PContent,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 2: Add BrowsingContextID to LoadInfo,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 3: Support uncached resources by using a redirectTo-like API for process swaps in necko,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 4: Add support for piping redirected channels through nsDocShell's loading functions,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 5: Perform parent-process interception for HTTP loads,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 6: Add basic test for process-changing POST loads,
(deleted), text/x-phabricator-request
Details
Bug 1467223 - Part 7: Renumber Continue* methods on nsHttpChannel,
(deleted), text/x-phabricator-request
Details
No description provided.
Thomas: Can we get details here? ready criteria/etc?
Flags: needinfo?(telin)
Priority: -- → P3
Flags: needinfo?(telin)
Summary: Modify the process-switching load code to support POST payloads on loads, and avoid calling into frontend JS. → Modify the process-switching load code to support POST payloads on loads
Blocks: 1490803
No longer blocks: 1490803
Assignee: nobody → nika
Blocks: fission
No longer depends on: 1445464, fission-history
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.
This is handy when performing process swaps, as it provides useful & important information to parent-process callers. Depends on D15608
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
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
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
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.
Flags: needinfo?(valentin.gosu)

(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

Flags: needinfo?(valentin.gosu)

What's the review priority for me here?

Depends on: 1520372

see c11

Flags: needinfo?(nika)
Attachment #9034062 - Attachment description: Bug 1467223 - Part 6: Add basic test for process-changing POST loads → Bug 1467223 - Part 6: Add basic test for process-changing POST loads,

(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 :-)

Flags: needinfo?(nika)

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

Pushed by nlayzell@mozilla.com: 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
Fission Milestone: --- → M1
Depends on: 1522839
Component: DOM → DOM: Core & HTML
No longer blocks: 1522640
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: