fix(user-timing): fix clearing marks/measures by name (#32120)

This fixes several tests in
[wpt/user-timing](https://wpt.fyi/results/user-timing?label=master&product=chrome%5Bexperimental%5D&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&product=servo&aligned)
by fixing some logic errors in how marks/measures are cleared (via
[`clearMarks`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/clearMarks)
and
[`clearMeasures`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/clearMeasures)).

There are two changes:

1. Fix the boolean logic in `clear_entries_by_name_and_type` so that,
when `clearMarks('foo')` or `clearMeasures('foo')` is called, the
presence of the entry name correctly filters based on existing entry
names.
2. Make the `entry_name` param a `DOMString` rather than an
`Option<DOMString>` since every API call has it as `Some` anyway, and
I'm not aware of any [Performance
APIs](https://developer.mozilla.org/en-US/docs/Web/API/Performance)
where you can clear all entries regardless of type. (This is not
strictly required for the fix, but I think it makes the code easier to
read.)

~~I also considered adding the expected WPT results using `mach
update-wpt`. But I'm not sure if you want these changes, since the
expectations are currently missing (i.e. `tests/wpt/meta/user-timing`
does not exist).~~ (_Update: added!_)

For the record, this PR fixes the following tests:

- `clearMarks.html.ini`
- `clearMeasures.html.ini`
- `clear_non_existent_mark.any.js.ini`
- `clear_non_existent_measure.any.js.ini`
- `clear_one_mark.any.js.ini`
- `clear_one_measure.any.js.ini`

~~In case you do want these meta files, here they are:
510e6146ba~~

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by
`[X]` when the step is complete, and replace `___` with appropriate
data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox
is checked, so that we can help you if you get stuck somewhere along the
way.-->

<!-- Pull requests that do not address these steps are welcome, but they
will require additional verification as part of the review process. -->
This commit is contained in:
Nolan Lawson 2024-04-22 02:16:05 -07:00 committed by GitHub
parent f9e154af55
commit 25b182c372
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 6 additions and 126 deletions

View file

@ -93,13 +93,11 @@ impl PerformanceEntryList {
pub fn clear_entries_by_name_and_type(
&mut self,
name: Option<DOMString>,
entry_type: Option<DOMString>,
entry_type: DOMString,
) {
self.entries.retain(|e| {
name.as_ref().map_or(true, |name_| *e.name() != *name_) &&
entry_type
.as_ref()
.map_or(true, |type_| *e.entry_type() != *type_)
*e.entry_type() != *entry_type ||
name.as_ref().map_or(false, |name_| *e.name() != *name_)
});
}
@ -479,7 +477,7 @@ impl PerformanceMethods for Performance {
fn ClearMarks(&self, mark_name: Option<DOMString>) {
self.buffer
.borrow_mut()
.clear_entries_by_name_and_type(mark_name, Some(DOMString::from("mark")));
.clear_entries_by_name_and_type(mark_name, DOMString::from("mark"));
}
// https://w3c.github.io/user-timing/#dom-performance-measure
@ -526,13 +524,13 @@ impl PerformanceMethods for Performance {
fn ClearMeasures(&self, measure_name: Option<DOMString>) {
self.buffer
.borrow_mut()
.clear_entries_by_name_and_type(measure_name, Some(DOMString::from("measure")));
.clear_entries_by_name_and_type(measure_name, DOMString::from("measure"));
}
// https://w3c.github.io/resource-timing/#dom-performance-clearresourcetimings
fn ClearResourceTimings(&self) {
self.buffer
.borrow_mut()
.clear_entries_by_name_and_type(None, Some(DOMString::from("resource")));
.clear_entries_by_name_and_type(None, DOMString::from("resource"));
self.resource_timing_buffer_current_size.set(0);
}