Closed Bug 1428447 Opened 4 years ago Closed 4 years ago

service workers should set the WorkerPrivate mLoadingPrincipal


(Core :: DOM: Service Workers, enhancement)

Not set



Tracking Status
firefox59 --- fixed


(Reporter: bkelly, Assigned: bkelly)




(1 file)

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.
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
Set a loading principal on ServiceWorker WorkerPrivate objects. r=asuth
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.