When a user creates a New Tab with a non-default value for the userContextId (Bug 1179557), this value should be set during the docShell's creation. In this bug, we'll add the plumbing necessary for value to propagate from the UI to the originAttibutes dictionary in the child docShell for remote (e10s) tabs. Bug 1191460 will implement this for non-e10s tabs where nsFrameLoader has direct access to create the docShell. See Bug 1191418 for more information on userContextId.

WIP patch from Bug 1191460. Will require re-write after changes in Bug 1191740.

WIP

Minimized the patch & split out the NeckoParent changes for separate review

Comment on attachment 8698129 [details] [diff] [review] Contextual Identity working under e10s +nsDocShell::GetUserContextId(uint32_t* aUserContextId) Looks like we removed this from the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1191460#c48. Why are we adding it back in?

Comment on attachment 8698128 [details] [diff] [review] NeckoParent needs to propagate the userContextId dolske is not a necko peer, so he may not be the best person to flag here. https://wiki.mozilla.org/Modules/Core#Necko diff --git a/netwerk/ipc/NeckoParent.cpp b/netwerk/ipc/NeckoParent.cpp aAttrs = DocShellOriginAttributes(appId, inBrowserElement); aAttrs.mSignedPkg = tabContext.OriginAttributesRef().mSignedPkg; + aAttrs.mUserContextId = tabContext.OriginAttributesRef().mUserContextId; return nullptr; Can DocShellOriginAttributes take signedPkg and userContextID as parameters? Why does it only take appid and inBrowser? http://mxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.h#100 Needinfo'ing Jonas to find out.

Remove unneeded GetUserContextId, move setUserContextId into IDL so it is accessible from nsIDocShell

Rebased to current master

Comment on attachment 8699261 [details] [diff] [review] NeckoParent needs to propagate the userContextId Review of attachment 8699261 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/ipc/NeckoParent.cpp @@ +159,2 @@ > aAttrs.mSignedPkg = tabContext.OriginAttributesRef().mSignedPkg; > + aAttrs.mUserContextId = tabContext.OriginAttributesRef().mUserContextId; Can you make this aAttrs.mUserContextId = aSerialized.mOriginAttributes.mUserContextId; instead. Eventually we should simply do "aAttrs = aSerialize.mOriginAttributes". But I don't want to mess with the appid stuff for now since it's broken. But reading the values out of aSerialize will make it more clear that that's where we're heading. Can you also change mSignedPkg to be read from aSerialize while you're at it (since that's actually even required for correctness)

Comment on attachment 8698275 [details] [diff] [review] Contextual Identity working under e10s Review of attachment 8698275 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/TabChild.cpp @@ +837,5 @@ > } > + > + OriginAttributes attrs = OriginAttributesRef(); > + docShell->SetIsSignedPackage(attrs.mSignedPkg); > + docShell->SetUserContextId(attrs.mUserContextId); I hate that we're still forwarding these attribute-by-attribute. We should just do docShell->SetOriginAttributes(OriginAttributesRef()) here. But I suspect fixing that in a separate bug is safer.

Comment on attachment 8704848 [details] [diff] [review] Contextual Identity working under e10s Review of attachment 8704848 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsIDocShell.idl @@ +42,5 @@ > interface nsITabParent; > > typedef unsigned long nsLoadFlags; > > +[scriptable, builtinclass, uuid(32286b78-d35e-4f1b-a766-73d06060c29d)] Why this change? ::: dom/ipc/TabChild.cpp @@ +837,5 @@ > } > + > + OriginAttributes attrs = OriginAttributesRef(); > + docShell->SetIsSignedPackage(attrs.mSignedPkg); > + docShell->SetUserContextId(attrs.mUserContextId); (on correct patch now) I hate that we're still forwarding these attribute-by-attribute. We should just do docShell->SetOriginAttributes(OriginAttributesRef()) here. But I suspect fixing that in a separate bug is safer.

Consolidated the previous two patches, carried over r+ from tanvi and sicking on both

Attachment #8705263 [details] [diff] is needed to enable tests under e10s

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/312ccc646df3 for e10s failures in browser_sessionStoreContainer.js like https://treeherder.mozilla.org/logviewer.html#?job_id=19474894&repo=mozilla-inbound

Back out restore userContextId for session store. We still save the userContextId, but we don't restore it yet. We need this since having contextual identity in e10s is currently higher priority than contextual identity support for session restore.

This patch plus what I proposed for bug 1239420 make e10s + sessionRestore working. Kate, can you tell me why you removed the setUserContextId() in SessionStore.jsm?

(In reply to Andrea Marchesini (:baku) from comment #28) > Created attachment 8708487 [details] [diff] [review] > patch > > This patch plus what I proposed for bug 1239420 make e10s + sessionRestore > working. > > Kate, can you tell me why you removed the setUserContextId() in > SessionStore.jsm? In Kate's most recent patch, she was trying to revert part of your patch to bug https://bugzilla.mozilla.org/show_bug.cgi?id=1193854 in order to make i) containers work with e10s ii) but not work with session restore. Getting it to work with e10s was more important than getting session restore to work. I asked her instead to backout 1193854 and land her original patch (comment 23) that was backed out. But it looks like that is no longer necessary, since you fixed bug 1239420! Hence, Kate can push her patch from comment 23 to try to make sure everything is still green and reland her patch. Thanks!

(In reply to Tanvi Vyas - please needinfo [:tanvi] from comment #29) > But it looks like that is no longer necessary, since you fixed bug 1239420! Nevermind, it was just backed out. But perhaps baku will have it fixed again soon.

Comment on attachment 8708487 [details] [diff] [review] patch Review of attachment 8708487 [details] [diff] [review]: ----------------------------------------------------------------- Reverting the patch to previous, so this is not needed.

when we use promiseTabState, the setting of the state we want happens too late in the tab lifecycle, causing a crash. This works with the 1239420 patch applied.

This patch reverts back to depending on 1193854 and 1239420 to enable contextual identity under e10s.

Treeherder push with 1239420 patch applied. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e17f2100daf6

Comment on attachment 8708855 [details] [diff] [review] test works under e10s Leaving this review to baku since he wrote the original test.

Comment on attachment 8708855 [details] [diff] [review] test works under e10s Leaving this review to baku since he wrote the original test.

Comment on attachment 8708856 [details] [diff] [review] Contextual Identity working under e10s with 1239420 patch You can probably also carry over Jonas' r+, since the code here is code he has previously r+'ed.

https://hg.mozilla.org/integration/fx-team/rev/5567470c17b7 https://hg.mozilla.org/integration/fx-team/rev/49909767e030

https://hg.mozilla.org/mozilla-central/rev/5567470c17b7 https://hg.mozilla.org/mozilla-central/rev/49909767e030

[bugday-20160323] Status: RESOLVED,FIXED -> VERIFIED Comments: STR: Not clear. Developer specific testing Component: Name Firefox Version 46.0b4 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Developer specific testing. Actual Results: As expected