Shared Workers should be segregated by userContextId in the OriginAttributes.

From the perspective of implementation, this bug might be identical to bug 1268726 (isolate shared worker by first party domain). Once we resolve this bug by making Shared Worker OriginAttributes aware, bug 1268726 should be resolved as well. However, we still file independent bugs to track this feature for two different projects (Containers and Tor integration) in case they have various requirements or priorities, etc.

Comment on attachment 8781043 [details] Bug 1294336 - Segregating shared workers by the originAttributes. https://reviewboard.mozilla.org/r/71556/#review69070 ::: dom/workers/RuntimeService.cpp:244 (Diff revision 1) > - bool aPrivateBrowsing, nsCString& aKey) > + bool aPrivateBrowsing, nsIPrincipal* aPrincipal, > + nsCString& aKey) > { > + MOZ_ASSERT(aPrincipal); > + > + nsCString originSuffix; nsAutoCString ::: dom/workers/RuntimeService.cpp:245 (Diff revision 1) > + nsCString& aKey) > { > + MOZ_ASSERT(aPrincipal); > + > + nsCString originSuffix; > + nsresult rv; nsresult rv = ...

I think this is the right way to do the origin attributes isolation because the private browsing flag is in the origin attributes already (I think).

If we go with my patch and remove the IsInPrivateBrowsing from the cache key, we can probably remove it completely from the worker info struct altogether. If we do that, then we need to figure out what to do with the CacheCreator class: https://dxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#353 That's the only caller of the function that isn't related to the creation of the SharedWorker cache key.

I've added two manual test cases that illustrate that shared workers can propagate changes through different containers. Test Case #1: * open https://arthuredelstein.github.io/tordemos/sharedworker-parent.html in a personal container * the page will generate and display a random number within the iframe * right click on the second link (listed below) within the personal container and select "Open New Link in New Container Tab -> Work" ** https://arthurtordemos.github.io/tordemos/sharedworker-parent.html The second tab (Work) will read the random number from the first container/tab (Personal), and will display the same generated number. Test Case #2: * open https://whatwg.org/demos/workers/multiviewer/page.html in a personal container * click on the "open a new viewer" button which will open a new viewer within the personal container (Viewer 0 created) * open a new work container via "File -> New Container Tab -> Work" * load https://whatwg.org/demos/workers/multiviewer/viewer.html in the work container (Viewer 1 created) * open a new shopping container via "File -> New Container Tab -> Shopping" * load https://whatwg.org/demos/workers/multiviewer/viewer.html in the shopping contaier (Viewer 2 created) * switch into the personal container that has "Viewer 0" and click on "Set 1" * take a look at the other Viewers within the different containers and you'll notice that the change has propagated throughout the different containers * switch into the personal container that has "Viewer 0" and type something into the "Public Chat" textbox and hit "Enter" * take a look at the other Viewers within the different containers and you'll notice that the change has propagated throughout the different containers

Comment on attachment 8781339 [details] [diff] [review] Bug_1268726.patch I think it is a good idea for using the originAttirbutes instead of the nsIPrincipal in the GenerateSharedWorkerKey(). And yes, we should fix the private browsing flags here. For the CacheCreator, I think we could still get the mPrivateBrowsingId from the principal of the workerPrivate. We only have to modify the WorkerPrivate.IsInPrivateBrowsing() to check the originAttributes of the workerPrivate's principal.

Hi Tim, Since I'm blocked on your test framework bug, can I take this bug and push it forward to review. It's mostly done. I'll just confirm whether or not we can get rid of the IsInPrivateBrowsing bool and accessor. Once I know the answer to that, I'll update the patch and submit it for review. --dave

I'm ok with that. Please take this.