1428130 - remove cookie permission preload after service worker code checks storage access in parent process.

Status: NEW

(Core :: DOM: Service Workers, enhancement, P3)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently nsDocShell does its own 3rd-party URL, private browsing, etc checks in order to block service worker interception when storage is not allowed.  Ideally it should use the nsContentUtils::StorageAllowed*() methods instead.

Unfortunately this is not possible currently since nsDocShell sometimes needs this before nsPermissionManager has data in the child process.

After we move service worker interception to the parent process we should make sure it uses our proper StorageAllowed code instead of its own side implementation.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

We changed plans and are going to preload the cookie permission.  Lets use this bug to remove the cookie permission preload once service worker begins checking storage access in the parent process.

Comment 2

[Johann Hofmann [:johannh]]

With the blocking bug resolved, can we remove this code again? https://searchfox.org/mozilla-central/rev/ba4fab1cc2f1c9c4e07cdb71542b8d441707c577/extensions/permissions/nsPermissionManager.cpp#120-125

Comment 3

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

Not until parent-intercept has stuck. It's only on beta right now and there's some concerns over the performance regressions. Bug 1496997 is where we'll declare victory and start ripping things out.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

We might want to look into removing this code again now, as I believe parent process interception is always on nowadays.

Comment 5

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

(In reply to Johann Hofmann [:johannh] from comment #2)

With the blocking bug resolved, can we remove this code again? https://searchfox.org/mozilla-central/rev/ba4fab1cc2f1c9c4e07cdb71542b8d441707c577/extensions/permissions/nsPermissionManager.cpp#120-125

This can be removed now if you want to remove it. Other work to this end is happening in bug 1496997.

Comment 6

[Johann Hofmann [:johannh]]

Unfortunately we're probably using the preload for other things these days, but I lost track of some of the details. Paul is looking into it.

Comment 7

[Emma Zühlcke [:emz]]

Preloading of cookie permissions is still a dependency of "cookie" permission OriginAttributes stripping. See comment here: https://searchfox.org/mozilla-central/rev/763b888847ac7291248b62ef3ab99da6ba2332d6/extensions/permissions/PermissionManager.cpp#144-145,148-151
Ideally we wouldn't need to do stripping at all, because we had UI that allowed users to adjust these permissions on a per private browsing / user context level.
Looks like we'll have to keep the preloading for now.