1590166 - Check if WindowGlobalChild::Create should call GetCrossOriginOpenerPolicy instead of ComputeCrossOriginOpenerPolicy

Status: RESOLVED FIXED

(Core :: DOM: Navigation, task, P2)

Description

[Valentin Gosu [:valentin] (he/him)]

https://phabricator.services.mozilla.com/D48715#inline-303287

Hmm, this looks like perhaps it should be updated to use the new cached GetCrossOriginOpenerPolicy getter. I'm not convinced this is doing the right thing.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1477743 is p3, so should this then.

Comment 2

[Neha Kochar [:neha]]

Valentin and Boris, could you review and decide if there are changes needed per Nika's comment on the previous review?

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, so if we're landing in WindowGlobalChild::Create we are quite far along from necko's point of view.

Is mComputedCrossOriginOpenerPolicy computed by then (assuming it will be computed at all)? It gets set by nsHttpChannel::ComputeCrossOriginOpenerPolicyMismatch, which is called in necko way before we ever get to this code. So the only way this can fail is if ComputeCrossOriginOpenerPolicyMismatch didn't compute it but HttpBaseChannel::ComputeCrossOriginOpenerPolicy would.

The reasons ComputeCrossOriginOpenerPolicyMismatch can fail to compute a policy are:

• Pref is disabled. Sees like we wouldn't want it in the window global child code either, then.
• This is not a toplevel document load. I'm not actually sure what we want here. Anne?
• There's no browsing context. That's clearly not an issue in WindowGlobalChild.

So the basic question is what the behavior here should be for toplevel vs non-toplevel documents...

Comment 4

[Anne (:annevk)]

Cross-Origin-Opener-Policy only applies to navigations of top-level browsing contexts. Hope that helps. (See also its processing model.)

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK. And now that I look again, this particular ComputeCrossOriginOpenerPolicy call in WindowGlobalChild::Create is only done if loadInfo->GetExternalContentPolicyType() == nsIContentPolicy::TYPE_DOCUMENT. So we're only dealing with toplevel loads here to start with.

So yes, I think this call should be GetCrossOriginOpenerPolicy instead. Someone should change that and make sure that tests pass, etc.

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5)

OK. And now that I look again, this particular ComputeCrossOriginOpenerPolicy call in WindowGlobalChild::Create is only done if loadInfo->GetExternalContentPolicyType() == nsIContentPolicy::TYPE_DOCUMENT. So we're only dealing with toplevel loads here to start with.

So yes, I think this call should be GetCrossOriginOpenerPolicy instead. Someone should change that and make sure that tests pass, etc.

Tom or Bas, are you the right engineer to work on this bug?

I see Tom is re-enabling SAB in bug 1594748 in Nightly, but I think Neha mentioned that Bas was working on SAB.

Is the current plan to re-enable SAB in 73 Nightly and then ride the trains in Firefox 74 or 75?

Comment 7

[Anne (:annevk)]

Valentin will likely work on this as he and Junior are responsible for COOP (and parts of COEP) and Valentin is back the soonest.

Re-enabling SAB is in effect several smaller projects:

1. Bas is working on bug 1563335 which is a requirement to have in beta quality to ship the remainder on release.
2. We need to make some design changes as the result of specification changes. These are minorish, but probably need some time to bake.
3. We can hopefully ship SharedArrayBuffer and atomics (without postMessage() support) sooner, I suspect in 75 as we want at least one cycle where it's enabled in beta (that'd be 74 with the current freeze as I understand it). Tracked in bug 1606624.
4. Shipping COOP/COEP (i.e., postMessage() support) can hopefully follow soon after provided 1 and 2 are done and bugs blocking bug 1477743 are addressed.

Hope that helps clarify the plans.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1590166 - WindowGlobalChild::Create should call GetCrossOriginOpenerPolicy instead of ComputeCrossOriginOpenerPolicy r=nika

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/513930129452
WindowGlobalChild::Create should call GetCrossOriginOpenerPolicy instead of ComputeCrossOriginOpenerPolicy r=bzbarsky

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/513930129452