This bug was filed from the Socorro interface and is report bp-37e16330-9499-4a6e-8621-42aa12170215. ============================================================= We are hitting the diagnostic assertion I added in bug 1337543 verifying that we don't get a principal with CSP. I'm not sure what path is leading to this, though.

About 40 crashes here so far. I need to look at it.

This just adds a few more assertions to try to narrow down what is happening in nightly channel. I'd like to land this before the weekend so I have results on monday if possible.

These assertions don't trigger anything in tests locally, but lets check try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=28191ae0f30fe7167b91dd51e820369a165238cb

So, I think maybe I should just make ServiceWorkerPrivate::SpawnWorkerIfNeeded() create a pristine principle without any CSP. This is easier than trying to make sure we never pass the registration principal somewhere that CSP gets added.

This moves the place where we sanitize the principal from registration creation to spawning the ServiceWorker thread. We just can't guarantee the registration principal doesn't get passed somewhere where it gets polluted with CSP. I thought I would be able to remove override the principal after loading the data from the CacheStorage, but I can't. It turns out our referrer implementation depends on getting the script URL out of the principal on worker thread. This is terrible, but the current implementation. Therefore keep overriding the principal with the final result principal.

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

Comment on attachment 8838716 [details] [diff] [review] Create a pristine principal in ServiceWorkerPrivate::SpawnWorkerIfNeeded(). r=baku I meant to flag for review here before. Please see comment 5 for the description.

Comment on attachment 8838716 [details] [diff] [review] Create a pristine principal in ServiceWorkerPrivate::SpawnWorkerIfNeeded(). r=baku Review of attachment 8838716 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerPrivate.cpp @@ +1736,5 @@ > return rv; > } > > + nsCOMPtr<nsIURI> uri; > + rv = mInfo->GetPrincipal()->GetURI(getter_AddRefs(uri)); GetPrincipal() sounds like it can return nullptr. Maybe would be nice to rename it _or_ to check the return value before using it.

#7 Windows topcrash in Nightly 20170217030229, with 44 occurrences. I'm looking forward to seeing the patch land! :)

(In reply to Andrea Marchesini [:baku] from comment #8) > GetPrincipal() sounds like it can return nullptr. Maybe would be nice to > rename it _or_ to check the return value before using it. I'll file a follow-up for this.

Updated to remove the principal sanitizing from ServiceWorkerManager::CreateNewRegistration() which is no longer necessary. I meant to include that here.

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/59e30ba1b38f Create a pristine principal in ServiceWorkerPrivate::SpawnWorkerIfNeeded(). r=baku

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d3feeb492d8f Create a pristine principal in ServiceWorkerPrivate::SpawnWorkerIfNeeded(). r=baku

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1fa61f7d1186 Backed out changeset 59e30ba1b38f on a CLOSED TREE

Note, the comment 14 backout was for comment 12 landing. Comment 13 is the re-landing.

Last crash seems to be in build 20170220030205. Overall graph of crash shows downward trend. This seems to have fixed it.