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

[Reporting API] Track limit of 100 reports by report type #4957

Conversation

brentfulgham
Copy link
Contributor

@brentfulgham brentfulgham commented Oct 4, 2022

f2b771d

[Reporting API] Track limit of 100 reports by report type
https://bugs.webkit.org/show_bug.cgi?id=244369
<rdar://problem/99463125>

Reviewed by Chris Dumez.

Track the count of reports by type so we can limit enforcement of the 100-report
cap to each type.

* LayoutTests/TestExpectations: Update to log to stderr.
* LayoutTests/imported/w3c/web-platform-tests/reporting/reporting-api-honors-limits.https.sub-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/reporting/reporting-api-honors-limits.https.sub.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/reporting/reporting-api-honors-limits.https.sub.html.sub.headers: Added.
* Source/WTF/wtf/Deque.h:
(WTF::inlineCapacity>::removeFirstMatching): Added.
* Source/WebCore/Modules/reporting/ReportingObserver.cpp:
(WebCore::ReportingObserver::appendQueuedReportIfCorrectType): Only enqueue the set of reports when the report count is one. The full
set will get read when the Task starts draining the existing queue.
* Source/WebCore/Modules/reporting/ReportingScope.cpp:
(WebCore::ReportingScope::clearReports):
(WebCore::ReportingScope::notifyReportObservers): Increment per-report count. Only
purge matching reports once we exceed the limit.
* Source/WebCore/Modules/reporting/ReportingScope.h:

Canonical link: https://commits.webkit.org/255307@main

b5a4046

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios   πŸ§ͺ api-mac   πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv   πŸ§ͺ mac-wk1 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch   πŸ§ͺ mac-AS-debug-wk2 ❌ πŸ›  jsc-mips
βœ… πŸ›  watch-sim βœ… πŸ§ͺ mac-wk2-stress ❌ πŸ§ͺ jsc-mips-tests

@brentfulgham brentfulgham self-assigned this Oct 4, 2022
@brentfulgham brentfulgham added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Oct 4, 2022
@brentfulgham
Copy link
Contributor Author

The GTK failure appears to be an infrastructure issue (unable to checkout a repo).

@brentfulgham brentfulgham force-pushed the eng/Reporting-API-Track-limit-of-100-reports-by-report-type branch from 27ede85 to 36dddbd Compare October 6, 2022 16:38
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM

@brentfulgham brentfulgham force-pushed the eng/Reporting-API-Track-limit-of-100-reports-by-report-type branch from 36dddbd to fbc4a07 Compare October 6, 2022 17:10
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 6, 2022
@brentfulgham brentfulgham removed the merging-blocked Applied to prevent a change from being merged label Oct 7, 2022
@brentfulgham brentfulgham force-pushed the eng/Reporting-API-Track-limit-of-100-reports-by-report-type branch from fbc4a07 to b5a4046 Compare October 7, 2022 21:14
@brentfulgham
Copy link
Contributor Author

I have adjusted the test a few times to bring it in line with other WPT's, and to make sure it works cleanly under WKTR and SafariDriver. I'll land as soon as the tests actually run to confirm I don't need to skip on any ports.

@brentfulgham brentfulgham added the merge-queue Applied to send a pull request to merge-queue label Oct 8, 2022
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Reporting-API-Track-limit-of-100-reports-by-report-type branch from b5a4046 to f2b771d Compare October 8, 2022 03:53
@webkit-commit-queue
Copy link
Collaborator

Committed 255307@main (f2b771d): https://commits.webkit.org/255307@main

Reviewed commits have been landed. Closing PR #4957 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
5 participants