1169210 - Can't call content service worker from chrome

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Alexandre Poirot [:ochameau]]

Bug 1153407 is hitting this exception while writing tests, mochitest browser, used for testing devtools. We have to test the devtools by using chrome, but then we can't easily call the content service worker.
We are not trying to use chrome service worker but hit this exception just because the caller is chrome.
http://mxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#1052

Couldn't we just prevent exposing navigator.serviceWorker on chrome?
Or only hit this assertion when accessing navigator.serviceWorker on a chrome document?
Ideally we shouldn't prevent service worker to work when the *caller* is chrome, but only when the *document* is chrome!
Or we could just make it work on chrome docs as weel if that's any easier ;)

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

Nikhil, what could we do here please? Thanks!

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Alexandre Poirot [:ochameau] from comment #0)
> Bug 1153407 is hitting this exception while writing tests, mochitest
> browser, used for testing devtools. We have to test the devtools by using
> chrome, but then we can't easily call the content service worker.
> We are not trying to use chrome service worker but hit this exception just
> because the caller is chrome.
> http://mxr.mozilla.org/mozilla-central/source/dom/workers/
> ServiceWorkerManager.cpp#1052
> 
> Couldn't we just prevent exposing navigator.serviceWorker on chrome?
> Or only hit this assertion when accessing navigator.serviceWorker on a
> chrome document?
> Ideally we shouldn't prevent service worker to work when the *caller* is
> chrome, but only when the *document* is chrome!
> Or we could just make it work on chrome docs as weel if that's any easier ;)

Please remove that assert and add an assert that the window's document's principal is not the system principal.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP

> Please remove that assert and add an assert that the window's document's
> principal is not the system principal.

Still not tested, but Is that what you meant please? Thanks!

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP

This is the correct patch. Sorry.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8612839 [details] [diff] [review]
WIP

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

::: dom/workers/ServiceWorkerManager.cpp
@@ +1208,5 @@
> +  nsCOMPtr<nsIPrincipal> documentPrincipal = doc->NodePrincipal();
> +  MOZ_ASSERT(documentPrincipal);
> +
> +  PrincipalInfo info;
> +  nsresult rv = PrincipalToPrincipalInfo(documentPrincipal, &info);

I think you just want:

  MOZ_ASSERT(!nsContentUtils::IsSystemPrincipal(doc->NodePrincipal()));

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8612839 [details] [diff] [review]
WIP

(In reply to Ben Kelly [:bkelly] from comment #5)
> Comment on attachment 8612839 [details] [diff] [review]
> WIP
> 
> Review of attachment 8612839 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/workers/ServiceWorkerManager.cpp
> @@ +1208,5 @@
> > +  nsCOMPtr<nsIPrincipal> documentPrincipal = doc->NodePrincipal();
> > +  MOZ_ASSERT(documentPrincipal);
> > +
> > +  PrincipalInfo info;
> > +  nsresult rv = PrincipalToPrincipalInfo(documentPrincipal, &info);
> 
> I think you just want:
> 
>   MOZ_ASSERT(!nsContentUtils::IsSystemPrincipal(doc->NodePrincipal()));

Thanks Ben, much easier!

Comment 7

[José Antonio Olivera Ortega [:jaoo]]

Attachment: MozReview Request: Bug 1169210 - Can't call content service worker from chrome. r=nsm

Bug 1169210 - Can't call content service worker from chrome. r=nsm

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8612915 [details]
MozReview Request: Bug 1169210 - Can't call content service worker from chrome. r=nsm

Bug 1169210 - Can't call content service worker from chrome. r=nsm

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8612915 [details]
MozReview Request: Bug 1169210 - Can't call content service worker from chrome. r=nsm

Bug 1169210 - Can't call content service worker from chrome. r=nsm

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8612915 [details]
MozReview Request: Bug 1169210 - Can't call content service worker from chrome. r=nsm

https://reviewboard.mozilla.org/r/9669/#review8427

Ship It!

::: dom/workers/ServiceWorkerManager.cpp:1060
(Diff revision 2)
> +  // XXXnsm Don't allow service workers to register when the *document* is

No longer XXXnsm ;)

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8612915 [details]
MozReview Request: Bug 1169210 - Can't call content service worker from chrome. r=nsm

Bug 1169210 - Can't call content service worker from chrome. r=nsm

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd24d251ec85

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b56e05dc5fa

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9b56e05dc5fa