Closed
Bug 776176
Opened 12 years ago
Closed 12 years ago
e10s: propagate GetExtendedOrigin from child to parent process
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
Attachment #644540 -
Flags: review?(josh) → review+
Assignee | ||
Comment 1•12 years ago
|
||
Assignee: nobody → jduell.mcbugs
Attachment #644549 -
Flags: review?(josh)
Assignee | ||
Comment 2•12 years ago
|
||
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•12 years ago
|
Attachment #644551 -
Flags: review?(josh) → review+
Assignee | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b672c3b20e58
Comment 4•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b672c3b20e58
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Updated•12 years ago
|
blocking-basecamp: ? → +
![]() |
||
Comment 5•12 years ago
|
||
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•12 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?
![]() |
||
Comment 8•12 years ago
|
||
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•12 years ago
|
||
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•12 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.
Description
•