1521808 - Implement Cross-Origin-Opener-Policy

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

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

HTML Spec Issue: https://github.com/whatwg/html/issues/3740
Chrome Implementation Issue: https://bugs.chromium.org/p/chromium/issues/detail?id=922191
:annevk's more technical explainer: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e

The initial implementation of this feature will probably be easiest to implement using the in-flight process selection logic implemented in bug 1467223. A check would be added for a Cross-Origin-Opener-Policy mismatch which would cause an unconditional process swap, whether or not the source and destination remoteType values are the same.

Comment 1

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

Attachment: Bug 1521808 - Implement Cross-Origin-Opener-Policy header r=mayhemer!,nika!

Comment 2

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

Attachment: Bug 1521808 - Add xpcshell-test for CrossOriginOpenerPolicy r=mayhemer!,nika!,annevk!

Depends on D18119

Comment 3

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

Attachment: Bug 1521808 - Use topWindowPrincipal for CrossOriginOpenerPolicy check r=mayhemer!,nika!

• Adds nsIHttpChannelInternal.setTopWindowPrincipal used to pass the principal from the child to the parent

Depends on D18119

Comment 4

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

Attachment: Bug 1521808 - Implement process switching based on Cross-Opener-Origin-Policy header r=nika!

• New topLevel loads get the nsILoadInfo.openerPolicy of the current top level document
• Parsing the Cross-Opener-Origin-Policy of a channel will update mLoadInfo.openerPolicy and this value will get propagated to the child process.
• SessionStore now checks nsIHttpChannel.hasCrossOriginOpenerPolicyMismatch (preffed off) and performs a process switch if needed

Depends on D18246

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d5174a560fa
Implement Cross-Origin-Opener-Policy header r=nika,mayhemer
https://hg.mozilla.org/integration/autoland/rev/cc0a5c7dabb4
Use topWindowPrincipal for CrossOriginOpenerPolicy check r=mayhemer,nika
https://hg.mozilla.org/integration/autoland/rev/c8c151d92c03
Add xpcshell-test for CrossOriginOpenerPolicy r=mayhemer,nika,annevk
https://hg.mozilla.org/integration/autoland/rev/80aeea5f7abe
Implement process switching based on Cross-Opener-Origin-Policy header r=nika,qdot

Comment 6

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1521808) for build bustage. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=227895515&repo=autoland&lineNumber=12312

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=80aeea5f7abe0a5c98c5c2f8c9de7d152c2a82e2

Backout:
https://hg.mozilla.org/integration/autoland/rev/a791ece512e14ccdc7ebf95e1f28d49e80d372f5

Comment 7

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

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

Comment 8

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f452fe610ad28882dc6107216a7f8093d6d8776

Comment 9

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=31663f02e1cbd0ea70aa99f2f9361834d27908cf

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a9f30586704392a14621f817bcd520cdf3d1d41

Comment 11

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

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a08553c07886
Implement Cross-Origin-Opener-Policy header r=nika,mayhemer
https://hg.mozilla.org/integration/autoland/rev/94e513102b53
Use topWindowPrincipal for CrossOriginOpenerPolicy check r=mayhemer,nika
https://hg.mozilla.org/integration/autoland/rev/32ce09b2c33a
Add xpcshell-test for CrossOriginOpenerPolicy r=mayhemer,nika,annevk
https://hg.mozilla.org/integration/autoland/rev/42641f88d7da
Implement process switching based on Cross-Opener-Origin-Policy header r=nika,qdot

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/45aaded7cf63
Backed out 4 changesets for causing merge conflicts.

Comment 14

[Cosmin Sabou [:CosminS]]

Backed out for causing merge conflicts. https://irccloud.mozilla.com/file/AxR0gkMP/image.png

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/acf942062ea0
Implement Cross-Origin-Opener-Policy header r=nika,mayhemer
https://hg.mozilla.org/integration/autoland/rev/6c7b1d562420
Use topWindowPrincipal for CrossOriginOpenerPolicy check r=mayhemer,nika
https://hg.mozilla.org/integration/autoland/rev/a7dc0e4d0136
Add xpcshell-test for CrossOriginOpenerPolicy r=mayhemer,nika,annevk
https://hg.mozilla.org/integration/autoland/rev/ab75adc15a37
Implement process switching based on Cross-Opener-Origin-Policy header r=nika,qdot

Comment 16

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/acf942062ea0
https://hg.mozilla.org/mozilla-central/rev/6c7b1d562420
https://hg.mozilla.org/mozilla-central/rev/a7dc0e4d0136
https://hg.mozilla.org/mozilla-central/rev/ab75adc15a37

Comment 18

[Chris Mills [:cmills]]

Note to MDN writers:

I've added a note to the Fx67 rel notes about this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#HTTP

The header still needs documenting, and BCD filling in.

Comment 19

[Florian Scholz (Open Web Docs)]

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #18)

Note to MDN writers:

I've added a note to the Fx67 rel notes about this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#HTTP

The header still needs documenting, and BCD filling in.

I see there is a pref called "browser.tabs.remote.useCrossOriginOpenerPolicy". Is this behind that pref, or is the header actually shipping in Firefox 67?

Comment 20

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

(In reply to Florian Scholz [:fscholz] (MDN) from comment #19)

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #18)

Note to MDN writers:

I've added a note to the Fx67 rel notes about this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#HTTP

The header still needs documenting, and BCD filling in.

I see there is a pref called "browser.tabs.remote.useCrossOriginOpenerPolicy". Is this behind that pref, or is the header actually shipping in Firefox 67?

This header is behind the pref.

Comment 21

[Florian Scholz (Open Web Docs)]

Thanks, updating the release notes accordingly.

Comment 22

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

valentin, do you recall why the test uses rather magical setTimeout(, 20).
(I'm debugging bug 1528607 a bit, without being able to reproduce too often, and those setTimeouts without any comments look suspicious)

Comment 23

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

(In reply to Olli Pettay [:smaug] from comment #22)

valentin, do you recall why the test uses rather magical setTimeout(, 20).
(I'm debugging bug 1528607 a bit, without being able to reproduce too often, and those setTimeouts without any comments look suspicious)

It was because this code

currentPID = await ContentTask.spawn(browser, null, () => {
        return Services.appinfo.processID;
      });

would sometimes return the PID of the previous process, not the one in which we did the cross process redirect.
Waiting for a bit™ would let things stabilize and run in the correct process. I didn't get the chance to check why that was at the time.
I don't know how relevant that test is anymore, given that we've got a lot more WPT tests for COOP now.