e10s: propagate GetExtendedOrigin from child to parent process

RESOLVED FIXED in mozilla17

Status

()

Core
Networking
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jduell, Assigned: jduell)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(blocking-kilimanjaro:?, blocking-basecamp:+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 644540 [details] [diff] [review]
v1

Yet more exciting e10s boilerplate.  I've filed bug 776174 to centralize some of this code at some point.
Attachment #644540 - Flags: review?(josh)

Updated

5 years ago
Attachment #644540 - Flags: review?(josh) → review+
(Assignee)

Comment 1

5 years ago
Created attachment 644549 [details] [diff] [review]
bonus: nsNetUtil.h convenience accessors
Assignee: nobody → jduell.mcbugs
Attachment #644549 - Flags: review?(josh)
(Assignee)

Comment 2

5 years ago
Created attachment 644551 [details] [diff] [review]
patch2, v2: nsNetUtil accessors

I seem to forget to qref every time I post patches now.
Attachment #644549 - Attachment is obsolete: true
Attachment #644549 - Flags: review?(josh)
Attachment #644551 - Flags: review?(josh)

Updated

5 years ago
Attachment #644551 - Flags: review?(josh) → review+
(Assignee)

Comment 3

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

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/b672c3b20e58
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
blocking-basecamp: ? → +
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
(Assignee)

Comment 6

5 years ago
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?
(Assignee)

Comment 7

5 years ago
added bug 777419 for issue in comment 6
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.
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.
(Assignee)

Comment 10

5 years ago
I'm going to implement IPC::LoadContext in bug 776174 and that will change things so that we get the extendedOrigin in the parent.
You need to log in before you can comment on or make changes to this bug.