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] Access reporting with 2 reporters. #25596

Merged
merged 1 commit into from Sep 21, 2020
Merged

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 17, 2020

A regression has been introduced by [1]:

[COOP] access reporting: Clear old CoopAccessMonitor.
https://chromium-review.googlesource.com/c/chromium/src/+/2408752

Instead of accumulating the CoopAccessMonitor for the same pair of
(accessing, accessed) window, it limited it to one and kept only the
most recent one.

This is an issue, because both the accessing and the accessed window
might have a reporter defined. We must keep both for supporting sending
reports toward both.

This patch adds regressions tests.

Bug: 1090273
Change-Id: I2a3aa7f26458db820d3383245c60c5f3f917acd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414308
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808323}

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.

A regression has been introduced by [1]:
```
[COOP] access reporting: Clear old CoopAccessMonitor.
https://chromium-review.googlesource.com/c/chromium/src/+/2408752
```

Instead of accumulating the CoopAccessMonitor for the same pair of
(accessing, accessed) window, it limited it to one and kept only the
most recent one.

This is an issue, because both the accessing and the accessed window
might have a reporter defined. We must keep both for supporting sending
reports toward both.

This patch adds regressions tests.

Bug: 1090273
Change-Id: I2a3aa7f26458db820d3383245c60c5f3f917acd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414308
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808323}
@Hexcles Hexcles closed this Sep 18, 2020
@Hexcles Hexcles reopened this Sep 18, 2020
@Hexcles Hexcles merged commit 9d8250c into master Sep 21, 2020
@Hexcles Hexcles deleted the chromium-export-cl-2414308 branch September 21, 2020 17:56
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

4 participants