1639080 - First party isolation should be stored as a constant on the BrowsingContext

Status: NEW

(Core :: DOM: Security, task, P3)

Description

[Matt Woodrow (:mattwoodrow)]

We currently check the privacy.firstparty.isolate pref in multiple places, across all processes.

This can cause races where we compute state using one value of the pref, send it to another process, and then assert because the pref has changed.

One example is here: https://searchfox.org/mozilla-central/rev/9193635dca8cfdcb68f114306194ffc860456044/docshell/base/nsDocShell.cpp#9134

This runs in the parent process (called from DocumentLoadListener), but uses state computed from the content process (when the pref had a different value).

This shows up fairly reliably on Android in netwerk/tests/crashtests/1334468-1.html, once bug 1620679 lands.

That bug just changes the load timings a bit, which makes this more likely to happen.

I'm going to disable this test on Android right now, since it's just a testing issue, and would otherwise block fixing real fission bugs and enabling the crashtest suite.

Comment 1

[Chris Peterson [:cpeterson]]

Matt, is this problem specifically about the privacy.firstparty.isolate pref? Will the new Dynamic FPI feature (network.cookie.cookieBehavior = 5) also cause problems? The OriginAttributes::IsFirstPartyEnabled() function just reads the privacy.firstparty.isolate pref. I don't know if the function will need to be extended to include Dynamic FPI.

Dynamic FPI can be enabled through the about:preferences#privacy UI, but AFAIK the privacy.firstparty.isolate pref can only be set from about:config. If this race can only be triggered by users messing with about:config, then this bug doesn't need to block Fission.

Can this bug can non-Fission users with multiple e10s content processes?

Comment 2

[Matt Woodrow (:mattwoodrow)]

Yeah, this should only cause problems if users are changing the pref in the middle of a load (and it's just an assert, so only causes problems for debug builds).

I don't think this needs to block fission.