Skip to content

Commit

Permalink
Start implementing a "callback registry" so closures aren't leaked
Browse files Browse the repository at this point in the history
  • Loading branch information
@blm768
blm768 committed Jun 8, 2019
1 parent fe963e4 commit 4f7e75d
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 32 deletions.
84 changes: 56 additions & 28 deletions src/platform_impl/web_sys/event_loop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::collections::VecDeque;
use std::marker::PhantomData;
use std::rc::Rc;
use wasm_bindgen::{prelude::*, JsCast};
use web_sys::{EventTarget, FocusEvent, HtmlCanvasElement, KeyboardEvent, PointerEvent, WheelEvent};
use web_sys::{EventTarget, FocusEvent, KeyboardEvent, PointerEvent, WheelEvent};
use window::WindowId as RootWI;

#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand All @@ -27,6 +27,7 @@ impl DeviceId {

pub struct EventLoop<T: 'static> {
elw: RootELW<T>,
callbacks: CallbackRegistry<T>,
}

pub struct EventLoopWindowTarget<T: 'static> {
Expand Down Expand Up @@ -71,12 +72,13 @@ struct EventLoopRunner<T> {

impl<T> EventLoop<T> {
pub fn new() -> Self {
EventLoop {
elw: RootELW {
p: EventLoopWindowTarget::new(),
_marker: PhantomData,
},
}
let elw = RootELW {
p: EventLoopWindowTarget::new(),
_marker: PhantomData,
};
let document = document();
let callbacks = CallbackRegistry::new(elw.p.runner.clone(), document.unchecked_into());
EventLoop { elw, callbacks }
}

pub fn available_monitors(&self) -> VecDequeIter<MonitorHandle> {
Expand All @@ -87,7 +89,7 @@ impl<T> EventLoop<T> {
MonitorHandle
}

pub fn run<F>(self, mut event_handler: F) -> !
pub fn run<F>(mut self, mut event_handler: F) -> !
where
F: 'static + FnMut(Event<T>, &RootELW<T>, &mut ControlFlow),
{
Expand All @@ -99,20 +101,19 @@ impl<T> EventLoop<T> {
};
runner.set_listener(Box::new(move |evt, ctrl| event_handler(evt, &relw, ctrl)));

let document = &document();
add_event(&runner, document, "blur", |elrs, _: FocusEvent| {
self.callbacks.add_event("blur", |elrs, _: FocusEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::Focused(false),
});
});
add_event(&runner, document, "focus", |elrs, _: FocusEvent| {
self.callbacks.add_event("focus", |elrs, _: FocusEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::Focused(true),
});
});
add_event(&runner, document, "keydown", |elrs, event: KeyboardEvent| {
self.callbacks.add_event("keydown", |elrs, event: KeyboardEvent| {
let key = event.key();
let mut characters = key.chars();
let first = characters.next();
Expand All @@ -136,7 +137,7 @@ impl<T> EventLoop<T> {
},
});
});
add_event(&runner, document, "keyup", |elrs, event: KeyboardEvent| {
self.callbacks.add_event("keyup", |elrs, event: KeyboardEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::KeyboardInput {
Expand Down Expand Up @@ -169,24 +170,43 @@ impl<T> EventLoop<T> {
}
}

pub fn register<T: 'static>(elrs: &EventLoopRunnerShared<T>, canvas: &HtmlCanvasElement) {
add_event(elrs, canvas, "pointerout", |elrs, event: PointerEvent| {
struct SavedCallback {
pub event: &'static str,
pub callback: Box<dyn AsRef<JsValue>>,
}

/**
* Manages the lifetime of the closures that are used to bind Web events to `winit` events
*/
pub struct CallbackRegistry<T: 'static> {
runner: EventLoopRunnerShared<T>,
event_target: EventTarget,
callbacks: Vec<SavedCallback>,
}

impl<T: 'static> CallbackRegistry<T> {
pub fn new(runner: EventLoopRunnerShared<T>, event_target: EventTarget) -> Self {
CallbackRegistry { runner, event_target, callbacks: Vec::new() }
}

pub fn register_window_events(&mut self) {
self.add_event("pointerout", |elrs, event: PointerEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::CursorLeft {
device_id: RootDI(DeviceId(event.pointer_id())),
},
});
});
add_event(elrs, canvas, "pointerover", |elrs, event: PointerEvent| {
self.add_event("pointerover", |elrs, event: PointerEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::CursorEntered {
device_id: RootDI(DeviceId(event.pointer_id())),
},
});
});
add_event(elrs, canvas, "pointermove", |elrs, event: PointerEvent| {
self.add_event("pointermove", |elrs, event: PointerEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::CursorMoved {
Expand All @@ -199,7 +219,7 @@ pub fn register<T: 'static>(elrs: &EventLoopRunnerShared<T>, canvas: &HtmlCanvas
},
});
});
add_event(elrs, canvas, "pointerup", |elrs, event: PointerEvent| {
self.add_event("pointerup", |elrs, event: PointerEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::MouseInput {
Expand All @@ -210,7 +230,7 @@ pub fn register<T: 'static>(elrs: &EventLoopRunnerShared<T>, canvas: &HtmlCanvas
},
});
});
add_event(elrs, canvas, "pointerdown", |elrs, event: PointerEvent| {
self.add_event("pointerdown", |elrs, event: PointerEvent| {
elrs.send_event(Event::WindowEvent {
window_id: RootWI(WindowId),
event: WindowEvent::MouseInput {
Expand All @@ -221,7 +241,7 @@ pub fn register<T: 'static>(elrs: &EventLoopRunnerShared<T>, canvas: &HtmlCanvas
},
});
});
add_event(elrs, canvas, "wheel", |elrs, event: WheelEvent| {
self.add_event("wheel", |elrs, event: WheelEvent| {
let x = event.delta_x();
let y = event.delta_y();
let delta = match event.delta_mode() {
Expand All @@ -241,18 +261,17 @@ pub fn register<T: 'static>(elrs: &EventLoopRunnerShared<T>, canvas: &HtmlCanvas
});
}

fn add_event<T: 'static, E, F>(
elrs: &EventLoopRunnerShared<T>,
target: &EventTarget,
event: &str,
fn add_event<E, F>(
&mut self,
event: &'static str,
mut handler: F,
) where
E: AsRef<web_sys::Event> + wasm_bindgen::convert::FromWasmAbi + 'static,
F: FnMut(&EventLoopRunnerShared<T>, E) + 'static,
{
let elrs = elrs.clone();
let elrs = self.runner.clone();

let closure = Closure::wrap(Box::new(move |event: E| {
let callback = Closure::wrap(Box::new(move |event: E| {
// Don't capture the event if the events loop has been destroyed
match &*elrs.runner.borrow() {
Some(ref runner) if runner.control == ControlFlow::Exit => return,
Expand All @@ -267,8 +286,17 @@ fn add_event<T: 'static, E, F>(
handler(&elrs, event);
}) as Box<dyn FnMut(E)>);

target.add_event_listener_with_callback(event, &closure.as_ref().unchecked_ref());
closure.forget(); // TODO: don't leak this.
self.event_target.add_event_listener_with_callback(event, &callback.as_ref().unchecked_ref());
self.callbacks.push(SavedCallback { event, callback: Box::new(callback) });
}
}

impl<T: 'static> Drop for CallbackRegistry<T> {
fn drop(&mut self) {
for callback in self.callbacks.iter() {
self.event_target.remove_event_listener_with_callback(callback.event, callback.callback.as_ref().as_ref().unchecked_ref());
}
}
}

impl<T> ELRShared<T> {
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/web_sys/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod events;
mod window;

pub use self::event_loop::{
register, DeviceId, EventLoop, EventLoopProxy, EventLoopRunnerShared, EventLoopWindowTarget,
CallbackRegistry, DeviceId, EventLoop, EventLoopProxy, EventLoopRunnerShared, EventLoopWindowTarget,
};
pub use self::events::{
button_mapping, keyboard_modifiers_state, mouse_button, mouse_modifiers_state, scancode,
Expand Down
10 changes: 7 additions & 3 deletions src/platform_impl/web_sys/window.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use platform::web_sys::WindowExtWebSys;
use platform_impl::platform::{document, window};
use monitor::{MonitorHandle as RootMH};
use window::{CursorIcon, Window as RootWindow, WindowAttributes, WindowId as RootWI};
use super::{EventLoopWindowTarget, OsError, register};
use super::{CallbackRegistry, EventLoopWindowTarget, OsError};
use std::any::Any;
use std::collections::VecDeque;
use std::collections::vec_deque::IntoIter as VecDequeIter;
use std::cell::RefCell;
Expand Down Expand Up @@ -51,6 +52,7 @@ pub struct Window {
pub(crate) redraw: Box<dyn Fn()>,
previous_pointer: RefCell<&'static str>,
position: RefCell<LogicalPosition>,
callbacks: Box<dyn Any>,
}

impl Window {
Expand All @@ -64,7 +66,8 @@ impl Window {
.ok_or_else(|| os_error!(OsError("Failed to find body node".to_owned())))?
.append_child(&canvas).map_err(|_| os_error!(OsError("Failed to append canvas".to_owned())))?;

register(&target.runner, &canvas);
let mut callbacks = CallbackRegistry::new(target.runner.clone(), canvas.clone().unchecked_into());
callbacks.register_window_events();

let runner = target.runner.clone();
let redraw = Box::new(move || {
Expand All @@ -85,7 +88,8 @@ impl Window {
position: RefCell::new(LogicalPosition {
x: 0.0,
y: 0.0
})
}),
callbacks: Box::new(callbacks),
};

if let Some(inner_size) = attr.inner_size {
Expand Down

0 comments on commit 4f7e75d

Please sign in to comment.