service workers should set the WorkerPrivate mLoadingPrincipal

RESOLVED FIXED in Firefox 59

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

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.
Andrea, I think this is why we were getting those assertions.  I'm happy to keep the runtime checks for now, though.
See Also: → 1426162
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
Assignee: nobody → bkelly
Attachment #8940325 - Flags: review?(bugmail)
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.)
Attachment #8940325 - Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a922694f0e6
Set a loading principal on ServiceWorker WorkerPrivate objects. r=asuth
https://hg.mozilla.org/mozilla-central/rev/5a922694f0e6
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.