1423507 - Data URL Shared Workers must be shared when the parent origins match

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect, P2)

Description

[Andrea Marchesini [:baku]]

This is a follow up for bug 1419166.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: part 1 - loadingPrincipal in WorkerLoadInfo

Comment 2

[Andrea Marchesini [:baku]]

Attachment: part 2 - WPT

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8934893 [details] [diff] [review]
part 2 - WPT

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

::: testing/web-platform/tests/workers/SharedWorker_dataUrl.html
@@ +16,5 @@
> +    let count = 0;
> +    onmessage = e => {
> +      assert_equals(e.data, 1);
> +      if (++count == 2) {
> +        resolve(true);

Please add a comment about the expectations here.  Something like, "The shared workers should be separate and only see a count of 1.  We still wait for both workers to respond."

@@ +39,5 @@
> +  return new Promise(function(resolve) {
> +    let count = 0;
> +    onmessage = e => {
> +      assert_equals(e.data, ++count);
> +      if (count == 2) {

Again, please add a comment.  Maybe something like "The shared worker should be shared and therefore send an incrementing count."

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8934892 [details] [diff] [review]
part 1 - loadingPrincipal in WorkerLoadInfo

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

Thanks for the comments and extra validation.  Looks good!

::: dom/workers/RuntimeService.cpp
@@ +2481,5 @@
>      nsCString scriptSpec;
>      nsresult rv = aLoadInfo->mResolvedScriptURI->GetSpec(scriptSpec);
>      NS_ENSURE_SUCCESS(rv, rv);
>  
> +    MOZ_ASSERT(aLoadInfo->mPrincipal && aLoadInfo->mLoadingPrincipal);

MOZ_DIAGNOSTIC_ASSERT please.

::: dom/workers/ScriptLoader.cpp
@@ +1833,5 @@
>      // before doing anything else.  Normally we do this in the WorkerPrivate
>      // Constructor, but we can't do so off the main thread when creating
>      // a nested worker.  So do it here instead.
> +    mLoadInfo.mLoadingPrincipal = mWorkerPrivate->GetPrincipal();
> +    MOZ_ASSERT(mLoadInfo.mLoadingPrincipal);

MOZ_DIAGNOSTIC_ASSERT

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/774f1b75e350
Data URL Shared Workers must be shared when the parent origins match, r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b63cb1aa01e
Data URL Shared Workers must be shared when the parent origins match - WPT, r=bkelly

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/774f1b75e350
https://hg.mozilla.org/mozilla-central/rev/6b63cb1aa01e

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

[Tracking Requested - why for this release]:

Since bug 1419166 was uplifted to 58 we might want to uplift this as well.  Or we should decide to back bug 1419166 out from 58.

Note, if this is uplifted we also need to uplift these bugs to fix follow-on assertion issues:

bug 1424336
bug 1426162
bug 1428447

Comment 9

[Julien Cristau [:jcristau]]

bug 1419166 was backed out of 58.