Auto merge of #23574 - asajeffrey:glutin-reduce-borrow-duration, r=paulrouget

Don't process events while borrowing the event loop

<!-- Please describe your changes on the following line: -->

At the moment, the glutin embedder holds onto a borrow of the events loop while processing events, which is dangerous if any of the event handlers end up using the events loop reentrantly. (This caused a panic while integrating https://github.com/servo/rust-webvr/pull/84.)

---
<!-- 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 do not require tests because this is low-level embedding code

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23574)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-07-01 10:01:46 -04:00 committed by GitHub
commit 18477d1e1e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -69,11 +69,8 @@ impl App {
mem::replace(&mut *self.event_queue.borrow_mut(), Vec::new()) mem::replace(&mut *self.event_queue.borrow_mut(), Vec::new())
} }
fn has_events(&self) -> bool { // This function decides whether the event should be handled during `run_forever`.
!self.event_queue.borrow().is_empty() || self.window.has_events() fn winit_event_to_servo_event(&self, event: glutin::Event) -> glutin::ControlFlow {
}
fn winit_event_to_servo_event(&self, event: glutin::Event) {
match event { match event {
// App level events // App level events
glutin::Event::Suspended(suspended) => { glutin::Event::Suspended(suspended) => {
@ -94,36 +91,43 @@ impl App {
if Some(window_id) != self.window.id() { if Some(window_id) != self.window.id() {
warn!("Got an event from unknown window"); warn!("Got an event from unknown window");
} else { } else {
// Resize events need to be handled during run_forever
let cont = if let glutin::WindowEvent::Resized(_) = event {
glutin::ControlFlow::Continue
} else {
glutin::ControlFlow::Break
};
self.window.winit_event_to_servo_event(event); self.window.winit_event_to_servo_event(event);
return cont;
} }
}, },
} }
glutin::ControlFlow::Break
} }
fn run_loop(self) { fn run_loop(self) {
let mut stop = false;
loop { loop {
let mut events_loop = self.events_loop.borrow_mut(); if !self.window.is_animating() || self.suspended.get() {
if self.window.is_animating() && !self.suspended.get() { // If there's no animations running then we block on the window event loop.
// We block on compositing (self.handle_events() ends up calling swap_buffers) self.events_loop.borrow_mut().run_forever(|e| {
events_loop.poll_events(|e| { let cont = self.winit_event_to_servo_event(e);
self.winit_event_to_servo_event(e); if cont == glutin::ControlFlow::Continue {
}); // Note we need to be careful to make sure that any events
stop = self.handle_events(); // that are handled during run_forever aren't re-entrant,
} else { // since we are handling them while holding onto a mutable borrow
// We block on winit's event loop (window events) // of the events loop
events_loop.run_forever(|e| { self.handle_events();
self.winit_event_to_servo_event(e);
if self.has_events() && !self.suspended.get() {
stop = self.handle_events();
}
if stop || self.window.is_animating() && !self.suspended.get() {
glutin::ControlFlow::Break
} else {
glutin::ControlFlow::Continue
} }
cont
}); });
} }
// Grab any other events that may have happened
self.events_loop.borrow_mut().poll_events(|e| {
self.winit_event_to_servo_event(e);
});
// If animations are running, we block on compositing
// (self.handle_events() ends up calling swap_buffers)
let stop = self.handle_events();
if stop { if stop {
break; break;
} }