-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Description
The testcase here is a little complex:
a.html:
<script>
setTimeout(() => {
console.log("in first setTimeout");
setTimeout(() => console.log("in second setTimeout"), 2000);
}, 2000);
window.addEventListener("load", () => {
setTimeout(() => location.href = "b.html", 0);
}, {once: true});
</script>b.html:
<script>
window.addEventListener("load", () => {
setTimeout(() => history.back(), 0);
}, {once: true});
</script>In Firefox, this logs:
in first setTimeout
in second setTimeout
In Servo, this logs:
in first setTimeout
When we navigate away from a page, we suspend any timers scheduled for that page:
servo/components/script/timers.rs
Lines 259 to 268 in 41b1a87
| pub(crate) fn suspend(&self) { | |
| // Suspend is idempotent: do nothing if the timers are already suspended. | |
| if self.suspended_since.get().is_some() { | |
| return warn!("Suspending an already suspended timer."); | |
| } | |
| debug!("Suspending timers."); | |
| self.suspended_since.set(Some(Instant::now())); | |
| self.invalidate_expected_event_id(); | |
| } |
When we traverse history to a suspended page, we resume the timers for that page:
servo/components/script/timers.rs
Lines 270 to 283 in 41b1a87
| pub(crate) fn resume(&self) { | |
| // Resume is idempotent: do nothing if the timers are already resumed. | |
| let additional_offset = match self.suspended_since.get() { | |
| Some(suspended_since) => Instant::now() - suspended_since, | |
| None => return warn!("Resuming an already resumed timer."), | |
| }; | |
| debug!("Resuming timers."); | |
| self.suspension_offset | |
| .set(self.suspension_offset.get() + additional_offset); | |
| self.suspended_since.set(None); | |
| self.schedule_timer_call(); | |
| } |
To ensure that we don't just fire all the timers that would have elapsed while the page was suspended, we use the suspension_offset value to add an offset when determining whether a timer is ready to fire or not:
servo/components/script/timers.rs
Lines 241 to 247 in 41b1a87
fn base_time(&self) -> Instant { let offset = self.suspension_offset.get(); match self.suspended_since.get() { Some(suspend_time) => suspend_time - offset, None => Instant::now() - offset, } } servo/components/script/timers.rs
Lines 204 to 210 in 41b1a87
let base_time = self.base_time(); // Since the event id was the expected one, at least one timer should be due. if base_time < self.timers.borrow().back().unwrap().scheduled_for { warn!("Unexpected timing!"); return; }
This calculation works for all timers that were already scheduled when the page was suspended, but it's incorrect for new timers that are scheduled after the page is resumed. The testcase fails that condition and prints the warning log and the scheduled timer never fires, since the scheduling mechanism thinks that it already ran.
This is the underlying cause for at least one of the bfcache tests timing out with #39421.