1428447 - service workers should set the WorkerPrivate mLoadingPrincipal

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement)

Description

[Ben Kelly [:bkelly, not reviewing]]

In bug 1423507 we added an mLoadingPrincipal concept.  I think the initial intent was that this was not needed for service workers because in theory they load through cache storage.  There are times, however, when a cache API failure occurs and we still end up hitting normal script loader paths.

To make all this easier to understand and more reliable we should just set an mLoadingPrincipal for service workers as well.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Andrea, I think this is why we were getting those assertions.  I'm happy to keep the runtime checks for now, though.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Set a loading principal on ServiceWorker WorkerPrivate objects. r=asuth

Andrew, this fixes a worker script failure you were running into in your --verify test runs.  (Or at least I hit.)

Basically we have an mLoadingPrincipal concept for workers now.  We expect this to match mPrincipal in most cases when the worker loads and we check in a number of paths.  Service workers were not setting mLoadingPrincipal and mostly got away with it.

I think perhaps there is an issue with service workers where if we are adding/removing the same script quickly we might end up loading the service worker without its offline scripts available yet.  In this case it hits the normal ScriptLoader path and performs the mLoadingPrincipal check.

We want to be resilient to error, so we should support this.  To do so, lets just set the mLoadingPrincipal.  It also makes the code easier to reason about in general.

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

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8940325 [details] [diff] [review]
Set a loading principal on ServiceWorker WorkerPrivate objects. r=asuth

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

Makes sense!  (That the SW's own principal can be its loading principal.  Definitely more intuitive and avoids a bunch of annoying edge-cases.)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a922694f0e6
Set a loading principal on ServiceWorker WorkerPrivate objects. r=asuth

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5a922694f0e6