Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add nested-context resource-timing tests #13823
Add nested-context resource-timing tests #13823
Changes from 11 commits
a9d38f6
9c407d1
8670ca6
4f31657
760b9d3
2abb55d
06075fb
2c63ca2
8937681
8f0c173
69dd2aa
7f004db
50b4cd2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a followup, it might be good to have not-samesite tests here too, just to test the cross-process case. hostinfo does not expose NOTSAMESITE_HOST at the moment, so that would need to get fixed too... I filed #15819 on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this test is already cross-origin between localhost and some other host, or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, @bzbarsky was asking for cross-origin and cross-site tests (so using hosts that have different domains)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right but this test is both cross-origin and cross-site if runs from localhost. Maybe what's missing is same-site cross-origin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test does not normally run "from localhost". It runs from one of the defined test hosts in the test configuration. Specifically, from whatever
{{host}}
expands to. Depends on the harness what that is, but it's generally notlocalhost
, but something more likeweb-platform.test
orwpt.live
or whatnot, depending on the test runner configuration.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And for details, see the
get_host_info
function's implementation and ORIGINAL_HOST and REMOTE_HOST and NOTSAMESITE_HOST in there. It has provisions for what to do if running fromlocalhost
, but also provisions for what to do in other cases. So yes, when running fromlocalhost
the test is testing the cross-site case and can't test the same-site cross-origin case at all.... which is why you should not be running these tests from localhost: a bunch of them will fail due to the lack of an available same-site cross-origin host in that setup.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. I wasn't aware of these details. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might have been good to document somewhere the tight coupling between this and the exact test-serialization behavior we get from using promise_test for everything, thus ensuring that we never touch sessionStorage from two different tests in a racy way.