Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. #21161

ParisMeuleman · 2020-01-14T13:56:06Z

This adds am assertion to verify the windowProxy object is closed.

html/cross-origin-opener-policy/resources/postback.html

html/cross-origin-opener-policy/resources/common.js

annevk · 2020-01-20T13:31:41Z

Let me know when you think this is ready for review again as it's not immediately apparent. I think you still need to find some occurrences as I'm pretty sure that at least on master this also happens in html/cross-origin-embedder-policy.

ParisMeuleman · 2020-01-20T15:18:09Z

Let me know when you think this is ready for review again as it's not immediately apparent. I think you still need to find some occurrences as I'm pretty sure that at least on master this also happens in html/cross-origin-embedder-policy.

I had a check on COEP cases, and I don't think we can have them close the popup:

the popup is openned with "noopener"
the popup navigates, which breaks the script-closable condition: "if it is a top-level browsing context whose session history contains only one Document."
This occurs in both none.https.html and requrie-corp.https.html.

I also expect coop-navigated-history-popup.https.html to be non-closable for the above reasons.

annevk

Assuming this doesn't affect how browsers fail/pass the tests, LGTM.

(Optional nits: "window proxy" -> "browsing context" (the WindowProxy object makes it observable), if( ... ) -> if (...).)

ParisMeuleman · 2020-01-21T10:29:44Z

Assuming this doesn't affect how browsers fail/pass the tests, LGTM.

(Optional nits: "window proxy" -> "browsing context" (the WindowProxy object makes it observable), if( ... ) -> if (...).)

Thanks!, regarding the first nit, I did the update but I'm not sure it is accurate (or at least that my comments after the change are still accurate): Isn't the point of this that the bc (browsing context) is no longer observable by the opener, rather than the bc being closed?

annevk · 2020-01-21T10:36:21Z

The browsing context is closed and replaced by a new browsing context. Browsing context and WindowProxy are 1:1 and we're not changing that.

A spec-problem is that we might need something higher-level than a browsing context to keep track of transitions such as these. E.g., a "browsing session" or some such.

…ered the relationship bothways

… channel

…o test opener's window proxy on some more tests

This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2) and (3). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at #21161). Bug:922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492

This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at #21161) and (2) needs to be updated with some other state in DidCommit. Bug:922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492

This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at #21161) and (2) needs to be updated with some other state in DidCommit. Bug: 922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985013 Commit-Queue: Arthur Hemery <ahemery@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736344}

This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at web-platform-tests/wpt#21161) and (2) needs to be updated with some other state in DidCommit. Bug: 922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985013 Commit-Queue: Arthur Hemery <ahemery@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736344}

…for COOP., a=testonly Automatic update from web-platform-tests [Security] Implement the context switch for COOP. This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at web-platform-tests/wpt#21161) and (2) needs to be updated with some other state in DidCommit. Bug: 922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985013 Commit-Queue: Arthur Hemery <ahemery@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736344} -- wpt-commits: 2ae8e39b7116213662bcc5357394d4418651211c wpt-pr: 21191

…for COOP., a=testonly Automatic update from web-platform-tests [Security] Implement the context switch for COOP. This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman at web-platform-tests/wpt#21161) and (2) needs to be updated with some other state in DidCommit. Bug: 922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985013 Commit-Queue: Arthur Hemery <ahemerychromium.org> Reviewed-by: Arthur Sonzogni <arthursonzognichromium.org> Cr-Commit-Position: refs/heads/master{#736344} -- wpt-commits: 2ae8e39b7116213662bcc5357394d4418651211c wpt-pr: 21191 UltraBlame original commit: b3450e9c5b09afa339b5ff59675c8b2f2c9be817

…for COOP., a=testonly Automatic update from web-platform-tests [Security] Implement the context switch for COOP. This CL lands the Cross-Origin-Opener-Policy "changes to navigation", as per: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e In practice, it forces a BrowsingInstance switch when we have a COOP mismatch. This is necessary but not sufficient, as we want to ensure the following behavior: (1) window.open() returned WindowProxy is closed when we know that we need to switch. (2) opener is null in opened popups. (3) window.name is empty in opened popups. This CL solves (2). Currently (1) is untested and will follow in a different patch (tests under development by pmeuleman@ at web-platform-tests/wpt#21161) and (2) needs to be updated with some other state in DidCommit. Bug: 922191 Change-Id: I7d5169f7d53bfc28ea01a1a03c7658e0645bc492 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985013 Commit-Queue: Arthur Hemery <ahemery@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736344} -- wpt-commits: 2ae8e39b7116213662bcc5357394d4418651211c wpt-pr: 21191

ParisMeuleman force-pushed the coop_add_openee_check branch from 5d90f71 to 5943d83 Compare January 15, 2020 10:34

ParisMeuleman requested a review from zcorpan January 15, 2020 10:35

ParisMeuleman marked this pull request as ready for review January 15, 2020 10:35

wpt-pr-bot added the html label Jan 15, 2020

wpt-pr-bot assigned zqzhang Jan 15, 2020

ParisMeuleman requested review from annevk and hemeryar January 15, 2020 10:37

annevk reviewed Jan 15, 2020

View reviewed changes

html/cross-origin-opener-policy/resources/postback.html Show resolved Hide resolved

html/cross-origin-opener-policy/resources/common.js Show resolved Hide resolved

ParisMeuleman force-pushed the coop_add_openee_check branch from 1a640f8 to 96ad9c9 Compare January 16, 2020 16:42

wpt-pr-bot requested review from arturjanc, domenic, foolip, jdm, jgraham, jugglinmike, mikewest, valenting and zqzhang January 16, 2020 16:42

annevk approved these changes Jan 21, 2020

View reviewed changes

ParisMeuleman force-pushed the coop_add_openee_check branch from bb8162c to 62e4052 Compare January 21, 2020 10:25

ParisMeuleman changed the title ~~Verify that COOP severed the relationship on the Opener's WindowProxy~~ Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. Jan 21, 2020

ParisMeuleman added 5 commits January 21, 2020 14:34

Add an assertion on the Opener's WindowProxy, verifying that COOP sev…

e844c58

…ered the relationship bothways

Remove defunct comment

6cb1001

Trigger the popup closure during the test cleanup using the broadcast…

bdca5f6

… channel

Close the popup from the top level opened document in COOP tests, als…

1313fb6

…o test opener's window proxy on some more tests

Fix typos and style

0d1165a

Fix failing cleanup in blob-popup.https.html

79c039c

ParisMeuleman force-pushed the coop_add_openee_check branch from 758ad90 to 79c039c Compare January 21, 2020 13:34

ParisMeuleman merged commit 3624a87 into web-platform-tests:master Jan 21, 2020

ParisMeuleman deleted the coop_add_openee_check branch January 21, 2020 15:22

chromium-wpt-export-bot mentioned this pull request Jan 23, 2020

[Security] Implement the context switch for COOP. #21191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. #21161

Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. #21161

ParisMeuleman commented Jan 14, 2020

annevk commented Jan 20, 2020

ParisMeuleman commented Jan 20, 2020

annevk left a comment

ParisMeuleman commented Jan 21, 2020

annevk commented Jan 21, 2020

Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. #21161

Verify that COOP severed the relationship on the Opener's side, with the popup's initial Browsing context closed. #21161

Conversation

ParisMeuleman commented Jan 14, 2020

annevk commented Jan 20, 2020

ParisMeuleman commented Jan 20, 2020

annevk left a comment

Choose a reason for hiding this comment

ParisMeuleman commented Jan 21, 2020

annevk commented Jan 21, 2020