Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

@ParisMeuleman
Copy link
Contributor

ParisMeuleman commented Jan 14, 2020

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

@ParisMeuleman ParisMeuleman force-pushed the ParisMeuleman:coop_add_openee_check branch from 5d90f71 to 5943d83 Jan 15, 2020
@ParisMeuleman ParisMeuleman requested a review from zcorpan Jan 15, 2020
@ParisMeuleman ParisMeuleman marked this pull request as ready for review Jan 15, 2020
@wpt-pr-bot wpt-pr-bot added the html label Jan 15, 2020
@ParisMeuleman ParisMeuleman requested review from annevk and hemeryar Jan 15, 2020
@ParisMeuleman ParisMeuleman force-pushed the ParisMeuleman:coop_add_openee_check branch from 1a640f8 to 96ad9c9 Jan 16, 2020
@annevk
Copy link
Member

annevk commented Jan 20, 2020

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
Copy link
Contributor Author

ParisMeuleman commented Jan 20, 2020

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
annevk approved these changes Jan 21, 2020
Copy link
Member

annevk left a comment

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 ParisMeuleman force-pushed the ParisMeuleman:coop_add_openee_check branch from bb8162c to 62e4052 Jan 21, 2020
@ParisMeuleman
Copy link
Contributor Author

ParisMeuleman commented Jan 21, 2020

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
Copy link
Member

annevk commented Jan 21, 2020

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.

@ParisMeuleman 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 ParisMeuleman force-pushed the ParisMeuleman:coop_add_openee_check branch from 758ad90 to 79c039c Jan 21, 2020
@ParisMeuleman ParisMeuleman merged commit 3624a87 into web-platform-tests:master Jan 21, 2020
10 checks passed
10 checks passed
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
Azure Pipelines Build #20200121.47 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Community-TC (pull_request) TaskGroup: success
Details
@ParisMeuleman ParisMeuleman deleted the ParisMeuleman:coop_add_openee_check branch Jan 21, 2020
chromium-wpt-export-bot added a commit that referenced this pull request Jan 23, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 27, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 27, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 27, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 27, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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}
chromium-wpt-export-bot added a commit that referenced this pull request Jan 29, 2020
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}
pull bot pushed a commit to FreddyZeng/chromium that referenced this pull request Jan 29, 2020
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}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 11, 2020
…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
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Feb 12, 2020
…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
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 12, 2020
…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
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Feb 12, 2020
…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
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Feb 12, 2020
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.