1815934 - Early Hints: Verify in Parent that channel connecting from content process is the same as the channel in EarlyHintPreloader

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Comment 1

[Manuel Bucher [:manuel]]

One way of doing this would be to store the EarlyHintRegistrar in the ContentParent instead of a global one. Maybe we can remove the need of the EarlyHintRegistrar by storing the EarlyHintsService in ContentProcess.

That would have the benefits that we could clear up the EarlyHint preloads when the tab gets closed no matter where in the process we are in (reducing the need for the ParentConnectTimout timer).

Comment 2

[Manuel Bucher [:manuel]]

Roughly the plan:

• Have a structure like OngoingEarlyHints that gets stored in ContentParent, right before or within the DocumentLoadListener::RedirectToRealChannel. Essentially moving the ownership of OngoingEarlyHints from DocumentLoadListener to ContentParent. This allows us to cancel all Early Hints when the tab gets closed (and the ContentParent destructed). I either reuse the OngoingEarlyHint class or create a new one that can't add further preloads.
• The earlyHintPreloadId will be the index in the array of OngoingEarlyHints holding the EarlyHintPreloaders.
• When the HttpChannelParent connects back, the EarlyHintPreloader gets removed from the array, moving the ownership to the new HttpChannelParent. (similar to how the EarlyHintPreloader gets removed from the EarlyHintRegistrar now.

Comment 3

[Manuel Bucher [:manuel]]

A problem I currently have implementing checks is, that when Registering the EarlyHintPreloaders

https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/netwerk/ipc/DocumentLoadListener.cpp#2143

mEarlyHintsService.RegisterLinksAndGetConnectArgs(ehArgs);

the ContentParent isn't available for loads initiated in the parent:

https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/netwerk/ipc/DocumentLoadListener.cpp#964-965,1019,1025

Therefore I can neither store the EarlyHintRegistrar in the ContentParent nor pass the processId of the ContentParent to the EarlyHintPreloaders for later verfication.

Also the mParentChannelListener->GetBrowsingContext()->OwnerProcessId() == 0 in DocumentLoadListener during that stage.

Comment 4

[Manuel Bucher [:manuel]]

Attachment: WIP: Bug 1815934 - Separate EarlyHintRegistrar by ContentParent r=#necko

To verify in Parent that channel connecting from content process is the same as the channel in EarlyHintPreloader

Comment 5

[Manuel Bucher [:manuel]]

Attachment: Bug 1815934 - Add timer back r=#necko

This commit is planned to be squashed with the previous one.

Depends on D171557

Comment 6

[Manuel Bucher [:manuel]]

Attachment: WIP: Bug 1815934 - Separate EarlyHintRegistrar by ContentParent

To verify in Parent that channel connecting from content process is the same as the channel in EarlyHintPreloader

Comment 7

[Manuel Bucher [:manuel]]

Attachment: Bug 1815934 - Only allow correct ContentParent to connect to EarlyHintPreloader r=#necko

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8f21cb15dfe
Only allow correct ContentParent to connect to EarlyHintPreloader r=necko-reviewers,kershaw

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d8f21cb15dfe