As we will use originAttribute to wrap appId/inBrowser in Bug 1163254, so ContentPrincipalInfo should leverage that instead of handling appId/isBrowser by itself.

So far I think following changes are needed: 1. Bug 1188776 and Bug 1165272 need to be done first. 2. dom/base/nsJSEnvironment.cpp for updating structured clone of Principal. 3. dom/cache/CacheStorage.cpp for checking appId is unknown. 4. dom/cache/DBSchema.cpp for using OriginAttributes and constructing ContentPrincipalInfo. 5. dom/workers/ServiceWorkerRegistrar.cpp for reading/writing originAttributes into serviceworker.txt and test case in dom/workers/test/gtest/TestReadWrite.cpp. 6. updating ipc/glue/BackgroundUtils.cpp and ipc/glue/PBackgroundSharedTypes.ipdlh

(In reply to Yoshi Huang[:allstars.chh] from comment #1) > 4. dom/cache/DBSchema.cpp for using OriginAttributes and constructing > ContentPrincipalInfo. The serialized origin attributes are stored in the database. If this changes we need a migration written to avoid corrupting the data. > 5. dom/workers/ServiceWorkerRegistrar.cpp for reading/writing > originAttributes into serviceworker.txt > and test case in dom/workers/test/gtest/TestReadWrite.cpp. Same thing. This needs to migrate existing data on disk.

(In reply to Ben Kelly [:bkelly] from comment #2) > (In reply to Yoshi Huang[:allstars.chh] from comment #1) > > 4. dom/cache/DBSchema.cpp for using OriginAttributes and constructing > > ContentPrincipalInfo. > > The serialized origin attributes are stored in the database. If this > changes we need a migration written to avoid corrupting the data. > I won't change the serialized origin. I will only change how we get OriginAttributes in InsertEntry and how we construct ContentPrincipalInfo in ReadResponse. Just change the const > > 5. dom/workers/ServiceWorkerRegistrar.cpp for reading/writing > > originAttributes into serviceworker.txt > > and test case in dom/workers/test/gtest/TestReadWrite.cpp. > > Same thing. This needs to migrate existing data on disk. Dimi will fix this in Bug 1189235.

Comment on attachment 8651732 [details] [diff] [review] Patch. Review of attachment 8651732 [details] [diff] [review]: ----------------------------------------------------------------- This looks great - thank you Yoshi! ::: ipc/glue/BackgroundUtils.cpp @@ +80,3 @@ > rv = secMan->GetSimpleCodebasePrincipal(uri, getter_AddRefs(principal)); > } else { > + principal = BasePrincipal::CreateCodebasePrincipal(uri, const_cast<OriginAttributes&>(info.attrs())); instead of the const_cast, can you just make CreateCodebasePrincipal accept const OriginAttributes& ?

I found the Mn-e10s will always have UNEXPECTED_ERROR after applying my patch on try server, although my local run is green. Will spend more time to check this. https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&tochange=487c57e377c2&fromchange=dbd6e7b9a9e1

rebase

Comment on attachment 8665230 [details] [diff] [review] Patch v2. Review of attachment 8665230 [details] [diff] [review]: ----------------------------------------------------------------- re-send r? again for Bug 1203916 and Bug 1163254 have been landed. https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d6d069d4198

Comment on attachment 8665230 [details] [diff] [review] Patch v2. Review of attachment 8665230 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks Yoshi!

I had to back this out for causing crashes: https://hg.mozilla.org/mozilla-central/rev/ccee6614fd9d

I think we should do bug 1209843 instead.

(In reply to Wes Kocher (:KWierso) from comment #12) > I had to back this out for causing crashes: > https://hg.mozilla.org/mozilla-central/rev/ccee6614fd9d Could this be the reason for crashes as reported in bug 1209508?

Comment on attachment 8665230 [details] [diff] [review] Patch v2. Review of attachment 8665230 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/glue/BackgroundUtils.cpp @@ +202,5 @@ > bool isUnknownAppId; > rv = aPrincipal->GetUnknownAppId(&isUnknownAppId); > if (NS_WARN_IF(NS_FAILED(rv))) { > return rv; > } line 202 ~ line 206 are dead code, will remove this as well.

rebase https://hg.mozilla.org/try/pushloghtml?changeset=8483e1db11c1