776176 - e10s: propagate GetExtendedOrigin from child to parent process

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jason Duell]

Attachment: v1

Yet more exciting e10s boilerplate.  I've filed bug 776174 to centralize some of this code at some point.

Comment 1

[Jason Duell]

Attachment: bonus: nsNetUtil.h convenience accessors

Comment 2

[Jason Duell]

Attachment: patch2, v2: nsNetUtil accessors

I seem to forget to qref every time I post patches now.

Comment 3

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b672c3b20e58

Comment 4

[Ed Morley [:emorley]]

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

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 644540 [details] [diff] [review]
v1

Review of attachment 644540 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +679,5 @@
> +NS_IMETHODIMP
> +HttpChannelParent::GetExtendedOrigin(nsIURI *aUri, nsACString &aResult)
> +{
> +  aResult = mExtendedOrigin;
> +  return NS_OK;

Maybe I'm out here... but according [1] this is a completely wrong implementation.

How can you assume that aUri will always be just mURI of the real http channel?  And why do you need to carry the string up to the parent when you can always create it with ssm?


[1] http://hg.mozilla.org/mozilla-central/annotate/a26e751bfb54/docshell/base/nsDocShell.cpp#l11365

Comment 6

[Jason Duell]

Ah, yes, that is a bug.  I should at the very least fail if the URI passed-in is not the mURI of the channel.

It would be better to use the ssm to get this--biesi was not happy with making necko aware of the ssm--which is why I move the extendedOrigin string across from the child to the parent, but perhaps we should ask him again to reconsider.  Either that or we should move the logic that computes the extendedOrigin into nsNetUtil or somewhere else necko knows about (but Mounir didn't like that idea).   Honza what is your opinion?

Comment 7

[Jason Duell]

added bug 777419 for issue in comment 6

Comment 8

[Honza Bambas (:mayhemer)]

SSM is an xpcom service with a widely published interface, and these days everything slowly starts using everything (not that is would be good, though).

It opens space for my other idea: since I re-implement the same for offline cache update parent, the same is again duplicated.  We should have a class that implements nsILoadContext interface for parent process.  I.e. keeps all members that parent may know and also is serializable to carry through IPC as a single arg.  Classes that want to impl nsILoadContext on the parent may just simply forward to a member that refs this class object (gained in the IPC constructor).  That little class may live on other place then network, e.g. where docshell or ssm lives.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

How does this interact with bug 746280, especially bug 776652?

When we talked last week in SF, we had assumed we weren't going to attempt to have any security boundary between the child process and the parent process. Now, we are going to have such a security boundary, so does not make sense to pass the extended origin from the child to the parent, because the parent cannot trust the information that the child has sent it.

Comment 10

[Jason Duell]

I'm going to implement IPC::LoadContext in bug 776174 and that will change things so that we get the extendedOrigin in the parent.