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

tests: use origin-agent-cluster to actually test oopifs #13777

Merged
merged 5 commits into from
Apr 1, 2022

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark requested a review from a team as a code owner March 23, 2022 20:23
@connorjclark connorjclark requested review from adamraine and removed request for a team March 23, 2022 20:23
// All other scripts are filtered out because of our "OOPIF" filter (including anything
// that is just in another process, like a worker).
ScriptElements: [
{
src: 'http://localhost:10200/simple-script.js',
source: 'network',
},
{
Copy link
Collaborator Author

@connorjclark connorjclark Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't have ignored this in the original pr :)

@@ -110,6 +110,8 @@ class Server {
// see https://github.com/jshttp/mime-types/issues/66
if (contentType) headers['Content-Type'] = mime.contentType(contentType);

headers['origin-agent-cluster'] = ['?1'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put this behind our own url parameter like isolated_frame or something? This is non-default behavior that we only use for one test.

If not could you add this to L103.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple reasons why I didn't put this behind a query param:

  • chrome doesn't like it when you mix responses from the same origin with and without this header. I found I had to reset chrome to undo this "mix responses" state when navigating to these pages directly via yarn static-server, which was pretty confusing
  • At least for me, I always assumed iframing across different ports on localhost would be isolated. Probably best if just isolate by default for our smoke tests.

@devtools-bot devtools-bot merged commit 4e35cc0 into master Apr 1, 2022
@devtools-bot devtools-bot deleted the oopif-agent-cluster branch April 1, 2022 17:54
@connorjclark connorjclark mentioned this pull request May 11, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants