Auto merge of #25590 - pshaughn:perfobs, r=jdm

Make performance observers take "type" and "buffered" options without panicking.

<!-- Please describe your changes on the following line: -->
I updated the observe() method to align with spec, and fixed the borrow duration bug @jdm pointed out in #25589 so it wouldn't cause a hard crash. Some tests go from failing to passing, but others go from early failing to later timeout; performance observers still aren't doing the right thing in all cases.

---
<!-- 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
- [X] These changes fix #24781 and fix #25589 and fix #23225

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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:
bors-servo 2020-02-12 11:09:52 -05:00 committed by GitHub
commit ed9b584344
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 171 additions and 205 deletions

View file

@ -185,22 +185,12 @@ impl Performance {
/// Add a PerformanceObserver to the list of observers with a set of
/// observed entry types.
pub fn add_observer(
pub fn add_multiple_type_observer(
&self,
observer: &DOMPerformanceObserver,
entry_types: Vec<DOMString>,
buffered: bool,
) {
if buffered {
let buffer = self.buffer.borrow();
let mut new_entries = entry_types
.iter()
.flat_map(|e| buffer.get_entries_by_name_and_type(None, Some(e.clone())))
.collect::<DOMPerformanceEntryList>();
let mut obs_entries = observer.entries();
obs_entries.append(&mut new_entries);
observer.set_entries(obs_entries);
}
let mut observers = self.observers.borrow_mut();
match observers.iter().position(|o| *o.observer == *observer) {
// If the observer is already in the list, we only update the observed
@ -214,6 +204,46 @@ impl Performance {
};
}
pub fn add_single_type_observer(
&self,
observer: &DOMPerformanceObserver,
entry_type: &DOMString,
buffered: bool,
) {
if buffered {
let buffer = self.buffer.borrow();
let mut new_entries =
buffer.get_entries_by_name_and_type(None, Some(entry_type.clone()));
if new_entries.len() > 0 {
let mut obs_entries = observer.entries();
obs_entries.append(&mut new_entries);
observer.set_entries(obs_entries);
}
if !self.pending_notification_observers_task.get() {
self.pending_notification_observers_task.set(true);
let task_source = self.global().performance_timeline_task_source();
task_source.queue_notification(&self.global());
}
}
let mut observers = self.observers.borrow_mut();
match observers.iter().position(|o| *o.observer == *observer) {
// If the observer is already in the list, we only update
// the observed entry types.
Some(p) => {
// Append the type if not already present, otherwise do nothing
if !observers[p].entry_types.contains(entry_type) {
observers[p].entry_types.push(entry_type.clone())
}
},
// Otherwise, we create and insert the new PerformanceObserver.
None => observers.push(PerformanceObserver {
observer: DomRoot::from_ref(observer),
entry_types: vec![entry_type.clone()],
}),
};
}
/// Remove a PerformanceObserver from the list of observers.
pub fn remove_observer(&self, observer: &DOMPerformanceObserver) {
let mut observers = self.observers.borrow_mut();
@ -287,18 +317,13 @@ impl Performance {
// Step 7.2.
// We have to operate over a copy of the performance observers to avoid
// the risk of an observer's callback modifying the list of registered
// observers.
// observers. This is a shallow copy, so observers can
// disconnect themselves by using the argument of their own callback.
let observers: Vec<DomRoot<DOMPerformanceObserver>> = self
.observers
.borrow()
.iter()
.map(|o| {
DOMPerformanceObserver::new(
&self.global(),
o.observer.callback(),
o.observer.entries(),
)
})
.map(|o| DomRoot::from_ref(&*o.observer))
.collect();
// Step 7.3.