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

Jump to bottom

[COOP] access reporting: test with redirects. #25711

Merged
merged 1 commit into from Sep 25, 2020
Merged

[COOP] access reporting: test with redirects. #25711

merged 1 commit into from Sep 25, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 23, 2020 •
edited

There are some properties added to reports depending on the redirects.
This adds tests so that we can verify what is reported.

body.{referrer,initialPopupURL} depends directly from the redirects.
body.{openeeURL,openerURL,otherDocumentURL} might depend in the future
from the redirects.

Bug: 1090273
Change-Id: I2ca80ee2664005bd156e4cbf2c08f1d2983b91b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424136
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810222}

wpt-pr-bot
wpt-pr-bot approved these changes Sep 23, 2020
View reviewed changes
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.

@ArthurSonzogni
Copy link
Member

wpt-chrome-dev-stability are failing, because of:
https://bugs.chromium.org/p/chromium/issues/detail?id=1098413#c16

This is a known issue. Here we are just updating the tests.
Could you please accept merging this patch, since this didn't regressed anything?

@ArthurSonzogni @chromium-wpt-export-bot
[COOP] access reporting: test with redirects.
d7f9009 
There are some properties added to reports depending on the redirects.
This adds tests so that we can verify what is reported.

body.{referrer,initialPopupURL} depends directly from the redirects.
body.{openeeURL,openerURL,otherDocumentURL} might depend in the future
from the redirects.

Bug: 1090273
Change-Id: I2ca80ee2664005bd156e4cbf2c08f1d2983b91b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424136
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810222}
@ArthurSonzogni
Copy link
Member

Hmm, I didn't expected wpt-firefox-nightly-stability to fail as well.
Firefox does not implement COOP reporting. So it is failing. However there are some timeout in function, and since Firefox fails all of them, it sometimes reach the full test timeout.

I am going to make the timeout of this function to be infinite by default.
Should I revert my patch or fix-forward? I am not fully sure how this works with two repository.

@ArthurSonzogni
Copy link
Member

I made:
https://chromium-review.googlesource.com/c/chromium/src/+/2428976

To fix the timeout issues on Firefox. This will fix wpt-firefox-nightly-stability.

@stephenmcgruer stephenmcgruer merged commit 2fef4d6 into master Sep 25, 2020
@stephenmcgruer stephenmcgruer deleted the chromium-export-cl-2424136 branch September 25, 2020 19:29
@stephenmcgruer
Copy link
Contributor

Thanks Arthur, I've admin-merged this PR since we'll pick up the fix from your follow-on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wpt-pr-bot wpt-pr-bot wpt-pr-bot approved these changes

Assignees
No one assigned
Labels
chromium-export html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@chromium-wpt-export-bot @ArthurSonzogni @stephenmcgruer @wpt-pr-bot