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

[COOP] Fix reporting-observer.html #27893

Merged
merged 1 commit into from
Mar 5, 2021
Merged

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 4, 2021

Stop using a WebLock for "send()", this isn't:

  • supported in cross-origin iframe. (causing this test to fail)
  • supported in Firefox (still experimental)
  • supported in Safari.

Instead use pure JS. This is a copypaste of the same method for COEP
credentialless, which has proven to be not flaky:
/wpt/html/cross-origin-embedder-policy/credentialless/resources/dispatcher.js

Bug: 1133342
Change-Id: Ib5c4eb6c76bb576bf1a21d44cb9bde1a63284d18
Fixed: 1133342
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2735014
Reviewed-by: Pâris Meuleman <[email protected]>
Commit-Queue: Arthur Sonzogni <[email protected]>
Cr-Commit-Position: refs/heads/master@{#859726}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

Stop using a WebLock for "send()", this isn't:
- supported in cross-origin iframe. (causing this test to fail)
- supported in Firefox (still experimental)
- supported in Safari.

Instead use pure JS. This is a copypaste of the same method for COEP
credentialless, which has proven to be not flaky:
/wpt/html/cross-origin-embedder-policy/credentialless/resources/dispatcher.js

Bug: 1133342
Change-Id: Ib5c4eb6c76bb576bf1a21d44cb9bde1a63284d18
Fixed: 1133342
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2735014
Reviewed-by: Pâris Meuleman <[email protected]>
Commit-Queue: Arthur Sonzogni <[email protected]>
Cr-Commit-Position: refs/heads/master@{#859726}
@ArthurSonzogni
Copy link
Member

I modified a common helper files used by every tests, and there was existing stability issues.
My patch doesn't regress anything.

See this previous answer for more details:
#25834 (comment)

@stephenmcgruer could you admin merge?
(Sorry for asking you this, every time I am modifying this file)

@stephenmcgruer stephenmcgruer merged commit 3ddc8d8 into master Mar 5, 2021
@stephenmcgruer stephenmcgruer deleted the chromium-export-cl-2735014 branch March 5, 2021 18:22
@stephenmcgruer
Copy link
Contributor

@stephenmcgruer could you admin merge?
(Sorry for asking you this, every time I am modifying this file)

Done, and apologies for missing this until today. Always happy to merge these, no problem about asking :)

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.

4 participants