Add cleanup code to web backend, mostly web-sys (#1715)

* web-sys: Impl. event listeners removal for canvas

* web-sys: Impl. media query listeners cleanup

* web: Emit WindowEvent::Destroyed after Window is dropped

* web-sys: Fix unload event closure being dropped early

* web: Impl. cleanup on ControlFlow::Exit

- Drops the Runner, which causes the event handler closure to be
  dropped.
- (web-sys only:) Remove event listeners from DOM.

* web: Do not remove canvas from DOM when dropping Window

The canvas was inserted by the user, so it should be up to the user
whether the canvas should be removed.

* Update changelog

src/platform_impl/web/web_sys/canvas.rs

@@ -1,4 +1,6 @@
 use super::event;
+use super::event_handle::EventListenerHandle;
+use super::media_query_handle::MediaQueryListHandle;
 use crate::dpi::{LogicalPosition, PhysicalPosition, PhysicalSize};
 use crate::error::OsError as RootOE;
 use crate::event::{ModifiersState, MouseButton, MouseScrollDelta, ScanCode, VirtualKeyCode};
@@ -18,14 +20,14 @@ mod pointer_handler;
 
 pub struct Canvas {
     common: Common,
-    on_focus: Option<Closure<dyn FnMut(FocusEvent)>>,
-    on_blur: Option<Closure<dyn FnMut(FocusEvent)>>,
-    on_keyboard_release: Option<Closure<dyn FnMut(KeyboardEvent)>>,
-    on_keyboard_press: Option<Closure<dyn FnMut(KeyboardEvent)>>,
-    on_received_character: Option<Closure<dyn FnMut(KeyboardEvent)>>,
-    on_mouse_wheel: Option<Closure<dyn FnMut(WheelEvent)>>,
-    on_fullscreen_change: Option<Closure<dyn FnMut(Event)>>,
-    on_dark_mode: Option<Closure<dyn FnMut(MediaQueryListEvent)>>,
+    on_focus: Option<EventListenerHandle<dyn FnMut(FocusEvent)>>,
+    on_blur: Option<EventListenerHandle<dyn FnMut(FocusEvent)>>,
+    on_keyboard_release: Option<EventListenerHandle<dyn FnMut(KeyboardEvent)>>,
+    on_keyboard_press: Option<EventListenerHandle<dyn FnMut(KeyboardEvent)>>,
+    on_received_character: Option<EventListenerHandle<dyn FnMut(KeyboardEvent)>>,
+    on_mouse_wheel: Option<EventListenerHandle<dyn FnMut(WheelEvent)>>,
+    on_fullscreen_change: Option<EventListenerHandle<dyn FnMut(Event)>>,
+    on_dark_mode: Option<MediaQueryListHandle>,
     mouse_state: MouseState,
 }
 
@@ -35,12 +37,6 @@ struct Common {
     wants_fullscreen: Rc<RefCell<bool>>,
 }
 
-impl Drop for Common {
-    fn drop(&mut self) {
-        self.raw.remove();
-    }
-}
-
 impl Canvas {
     pub fn create(attr: PlatformSpecificWindowBuilderAttributes) -> Result<Self, RootOE> {
         let canvas = match attr.canvas {
@@ -264,22 +260,12 @@ impl Canvas {
     where
         F: 'static + FnMut(bool),
     {
-        let window = web_sys::window().expect("Failed to obtain window");
-
-        self.on_dark_mode = window
-            .match_media("(prefers-color-scheme: dark)")
-            .ok()
-            .flatten()
-            .and_then(|media| {
-                let closure = Closure::wrap(Box::new(move |event: MediaQueryListEvent| {
-                    handler(event.matches())
-                }) as Box<dyn FnMut(_)>);
-
-                media
-                    .add_listener_with_opt_callback(Some(&closure.as_ref().unchecked_ref()))
-                    .map(|_| closure)
-                    .ok()
-            });
+        let closure =
+            Closure::wrap(
+                Box::new(move |event: MediaQueryListEvent| handler(event.matches()))
+                    as Box<dyn FnMut(_)>,
+            );
+        self.on_dark_mode = MediaQueryListHandle::new("(prefers-color-scheme: dark)", closure);
     }
 
     pub fn request_fullscreen(&self) {
@@ -289,10 +275,29 @@ impl Canvas {
     pub fn is_fullscreen(&self) -> bool {
         self.common.is_fullscreen()
     }
+
+    pub fn remove_listeners(&mut self) {
+        self.on_focus = None;
+        self.on_blur = None;
+        self.on_keyboard_release = None;
+        self.on_keyboard_press = None;
+        self.on_received_character = None;
+        self.on_mouse_wheel = None;
+        self.on_fullscreen_change = None;
+        self.on_dark_mode = None;
+        match &mut self.mouse_state {
+            MouseState::HasPointerEvent(h) => h.remove_listeners(),
+            MouseState::NoPointerEvent(h) => h.remove_listeners(),
+        }
+    }
 }
 
 impl Common {
-    fn add_event<E, F>(&self, event_name: &str, mut handler: F) -> Closure<dyn FnMut(E)>
+    fn add_event<E, F>(
+        &self,
+        event_name: &'static str,
+        mut handler: F,
+    ) -> EventListenerHandle<dyn FnMut(E)>
     where
         E: 'static + AsRef<web_sys::Event> + wasm_bindgen::convert::FromWasmAbi,
         F: 'static + FnMut(E),
@@ -307,17 +312,19 @@ impl Common {
             handler(event);
         }) as Box<dyn FnMut(E)>);
 
-        self.raw
-            .add_event_listener_with_callback(event_name, &closure.as_ref().unchecked_ref())
-            .expect("Failed to add event listener with callback");
+        let listener = EventListenerHandle::new(&self.raw, event_name, closure);
 
-        closure
+        listener
     }
 
     // The difference between add_event and add_user_event is that the latter has a special meaning
     // for browser security. A user event is a deliberate action by the user (like a mouse or key
     // press) and is the only time things like a fullscreen request may be successfully completed.)
-    fn add_user_event<E, F>(&self, event_name: &str, mut handler: F) -> Closure<dyn FnMut(E)>
+    fn add_user_event<E, F>(
+        &self,
+        event_name: &'static str,
+        mut handler: F,
+    ) -> EventListenerHandle<dyn FnMut(E)>
     where
         E: 'static + AsRef<web_sys::Event> + wasm_bindgen::convert::FromWasmAbi,
         F: 'static + FnMut(E),
@@ -343,9 +350,9 @@ impl Common {
     // handling to control event propagation.
     fn add_window_mouse_event<F>(
         &self,
-        event_name: &str,
+        event_name: &'static str,
         mut handler: F,
-    ) -> Closure<dyn FnMut(MouseEvent)>
+    ) -> EventListenerHandle<dyn FnMut(MouseEvent)>
     where
         F: 'static + FnMut(MouseEvent),
     {
@@ -364,15 +371,14 @@ impl Common {
             }
         }) as Box<dyn FnMut(_)>);
 
-        window
-            .add_event_listener_with_callback_and_add_event_listener_options(
-                event_name,
-                &closure.as_ref().unchecked_ref(),
-                AddEventListenerOptions::new().capture(true),
-            )
-            .expect("Failed to add event listener with callback and options");
+        let listener = EventListenerHandle::with_options(
+            &window,
+            event_name,
+            closure,
+            AddEventListenerOptions::new().capture(true),
+        );
 
-        closure
+        listener
     }
 
     pub fn request_fullscreen(&self) {

src/platform_impl/web/web_sys/canvas/mouse_handler.rs

@@ -1,19 +1,19 @@
 use super::event;
+use super::EventListenerHandle;
 use crate::dpi::PhysicalPosition;
 use crate::event::{ModifiersState, MouseButton};
 
 use std::cell::RefCell;
 use std::rc::Rc;
 
-use wasm_bindgen::closure::Closure;
 use web_sys::{EventTarget, MouseEvent};
 
 pub(super) struct MouseHandler {
-    on_mouse_leave: Option<Closure<dyn FnMut(MouseEvent)>>,
-    on_mouse_enter: Option<Closure<dyn FnMut(MouseEvent)>>,
-    on_mouse_move: Option<Closure<dyn FnMut(MouseEvent)>>,
-    on_mouse_press: Option<Closure<dyn FnMut(MouseEvent)>>,
-    on_mouse_release: Option<Closure<dyn FnMut(MouseEvent)>>,
+    on_mouse_leave: Option<EventListenerHandle<dyn FnMut(MouseEvent)>>,
+    on_mouse_enter: Option<EventListenerHandle<dyn FnMut(MouseEvent)>>,
+    on_mouse_move: Option<EventListenerHandle<dyn FnMut(MouseEvent)>>,
+    on_mouse_press: Option<EventListenerHandle<dyn FnMut(MouseEvent)>>,
+    on_mouse_release: Option<EventListenerHandle<dyn FnMut(MouseEvent)>>,
     on_mouse_leave_handler: Rc<RefCell<Option<Box<dyn FnMut(i32)>>>>,
     mouse_capture_state: Rc<RefCell<MouseCaptureState>>,
 }
@@ -200,4 +200,13 @@ impl MouseHandler {
             },
         ));
     }
+
+    pub fn remove_listeners(&mut self) {
+        self.on_mouse_leave = None;
+        self.on_mouse_enter = None;
+        self.on_mouse_move = None;
+        self.on_mouse_press = None;
+        self.on_mouse_release = None;
+        *self.on_mouse_leave_handler.borrow_mut() = None;
+    }
 }

src/platform_impl/web/web_sys/canvas/pointer_handler.rs

@@ -1,16 +1,16 @@
 use super::event;
+use super::EventListenerHandle;
 use crate::dpi::PhysicalPosition;
 use crate::event::{ModifiersState, MouseButton};
 
-use wasm_bindgen::closure::Closure;
 use web_sys::PointerEvent;
 
 pub(super) struct PointerHandler {
-    on_cursor_leave: Option<Closure<dyn FnMut(PointerEvent)>>,
-    on_cursor_enter: Option<Closure<dyn FnMut(PointerEvent)>>,
-    on_cursor_move: Option<Closure<dyn FnMut(PointerEvent)>>,
-    on_pointer_press: Option<Closure<dyn FnMut(PointerEvent)>>,
-    on_pointer_release: Option<Closure<dyn FnMut(PointerEvent)>>,
+    on_cursor_leave: Option<EventListenerHandle<dyn FnMut(PointerEvent)>>,
+    on_cursor_enter: Option<EventListenerHandle<dyn FnMut(PointerEvent)>>,
+    on_cursor_move: Option<EventListenerHandle<dyn FnMut(PointerEvent)>>,
+    on_pointer_press: Option<EventListenerHandle<dyn FnMut(PointerEvent)>>,
+    on_pointer_release: Option<EventListenerHandle<dyn FnMut(PointerEvent)>>,
 }
 
 impl PointerHandler {
@@ -100,4 +100,12 @@ impl PointerHandler {
             },
         ));
     }
+
+    pub fn remove_listeners(&mut self) {
+        self.on_cursor_leave = None;
+        self.on_cursor_enter = None;
+        self.on_cursor_move = None;
+        self.on_pointer_press = None;
+        self.on_pointer_release = None;
+    }
 }

src/platform_impl/web/web_sys/event_handle.rs

@@ -0,0 +1,65 @@
+use wasm_bindgen::{prelude::Closure, JsCast};
+use web_sys::{AddEventListenerOptions, EventTarget};
+
+pub(super) struct EventListenerHandle<T: ?Sized> {
+    target: EventTarget,
+    event_type: &'static str,
+    listener: Closure<T>,
+}
+
+impl<T: ?Sized> EventListenerHandle<T> {
+    pub fn new<U>(target: &U, event_type: &'static str, listener: Closure<T>) -> Self
+    where
+        U: Clone + Into<EventTarget>,
+    {
+        let target = target.clone().into();
+        target
+            .add_event_listener_with_callback(event_type, listener.as_ref().unchecked_ref())
+            .expect("Failed to add event listener");
+        EventListenerHandle {
+            target,
+            event_type,
+            listener,
+        }
+    }
+
+    pub fn with_options<U>(
+        target: &U,
+        event_type: &'static str,
+        listener: Closure<T>,
+        options: &AddEventListenerOptions,
+    ) -> Self
+    where
+        U: Clone + Into<EventTarget>,
+    {
+        let target = target.clone().into();
+        target
+            .add_event_listener_with_callback_and_add_event_listener_options(
+                event_type,
+                listener.as_ref().unchecked_ref(),
+                options,
+            )
+            .expect("Failed to add event listener");
+        EventListenerHandle {
+            target,
+            event_type,
+            listener,
+        }
+    }
+}
+
+impl<T: ?Sized> Drop for EventListenerHandle<T> {
+    fn drop(&mut self) {
+        self.target
+            .remove_event_listener_with_callback(
+                self.event_type,
+                self.listener.as_ref().unchecked_ref(),
+            )
+            .unwrap_or_else(|e| {
+                web_sys::console::error_2(
+                    &format!("Error removing event listener {}", self.event_type).into(),
+                    &e,
+                )
+            });
+    }
+}

src/platform_impl/web/web_sys/media_query_handle.rs

@@ -0,0 +1,56 @@
+use wasm_bindgen::{prelude::Closure, JsCast};
+use web_sys::{MediaQueryList, MediaQueryListEvent};
+
+pub(super) struct MediaQueryListHandle {
+    mql: MediaQueryList,
+    listener: Option<Closure<dyn FnMut(MediaQueryListEvent)>>,
+}
+
+impl MediaQueryListHandle {
+    pub fn new(
+        media_query: &str,
+        listener: Closure<dyn FnMut(MediaQueryListEvent)>,
+    ) -> Option<Self> {
+        let window = web_sys::window().expect("Failed to obtain window");
+        let mql = window
+            .match_media(media_query)
+            .ok()
+            .flatten()
+            .and_then(|mql| {
+                mql.add_listener_with_opt_callback(Some(&listener.as_ref().unchecked_ref()))
+                    .map(|_| mql)
+                    .ok()
+            });
+        mql.map(|mql| Self {
+            mql,
+            listener: Some(listener),
+        })
+    }
+
+    pub fn mql(&self) -> &MediaQueryList {
+        &self.mql
+    }
+
+    /// Removes the listener and returns the original listener closure, which
+    /// can be reused.
+    pub fn remove(mut self) -> Closure<dyn FnMut(MediaQueryListEvent)> {
+        let listener = self.listener.take().unwrap_or_else(|| unreachable!());
+        remove_listener(&self.mql, &listener);
+        listener
+    }
+}
+
+impl Drop for MediaQueryListHandle {
+    fn drop(&mut self) {
+        if let Some(listener) = self.listener.take() {
+            remove_listener(&self.mql, &listener);
+        }
+    }
+}
+
+fn remove_listener(mql: &MediaQueryList, listener: &Closure<dyn FnMut(MediaQueryListEvent)>) {
+    mql.remove_listener_with_opt_callback(Some(listener.as_ref().unchecked_ref()))
+        .unwrap_or_else(|e| {
+            web_sys::console::error_2(&"Error removing media query listener".into(), &e)
+        });
+}

src/platform_impl/web/web_sys/mod.rs

@@ -1,5 +1,7 @@
 mod canvas;
 mod event;
+mod event_handle;
+mod media_query_handle;
 mod scaling;
 mod timeout;
 
@@ -10,7 +12,7 @@ pub use self::timeout::{AnimationFrameRequest, Timeout};
 use crate::dpi::{LogicalSize, Size};
 use crate::platform::web::WindowExtWebSys;
 use crate::window::Window;
-use wasm_bindgen::{closure::Closure, JsCast};
+use wasm_bindgen::closure::Closure;
 use web_sys::{window, BeforeUnloadEvent, Element, HtmlCanvasElement};
 
 pub fn throw(msg: &str) {
@@ -24,16 +26,21 @@ pub fn exit_fullscreen() {
     document.exit_fullscreen();
 }
 
-pub fn on_unload(mut handler: impl FnMut() + 'static) {
+pub struct UnloadEventHandle {
+    _listener: event_handle::EventListenerHandle<dyn FnMut(BeforeUnloadEvent)>,
+}
+
+pub fn on_unload(mut handler: impl FnMut() + 'static) -> UnloadEventHandle {
     let window = web_sys::window().expect("Failed to obtain window");
 
     let closure = Closure::wrap(
         Box::new(move |_: BeforeUnloadEvent| handler()) as Box<dyn FnMut(BeforeUnloadEvent)>
     );
 
-    window
-        .add_event_listener_with_callback("beforeunload", &closure.as_ref().unchecked_ref())
-        .expect("Failed to add close listener");
+    let listener = event_handle::EventListenerHandle::new(&window, "beforeunload", closure);
+    UnloadEventHandle {
+        _listener: listener,
+    }
 }
 
 impl WindowExtWebSys for Window {

src/platform_impl/web/web_sys/scaling.rs

@@ -1,8 +1,9 @@
 use super::super::ScaleChangeArgs;
+use super::media_query_handle::MediaQueryListHandle;
 
 use std::{cell::RefCell, rc::Rc};
-use wasm_bindgen::{prelude::Closure, JsCast};
-use web_sys::{MediaQueryList, MediaQueryListEvent};
+use wasm_bindgen::prelude::Closure;
+use web_sys::MediaQueryListEvent;
 
 pub struct ScaleChangeDetector(Rc<RefCell<ScaleChangeDetectorInternal>>);
 
@@ -19,8 +20,7 @@ impl ScaleChangeDetector {
 /// changes of the `devicePixelRatio`.
 struct ScaleChangeDetectorInternal {
     callback: Box<dyn FnMut(ScaleChangeArgs)>,
-    closure: Option<Closure<dyn FnMut(MediaQueryListEvent)>>,
-    mql: Option<MediaQueryList>,
+    mql: Option<MediaQueryListHandle>,
     last_scale: f64,
 }
 
@@ -32,27 +32,28 @@ impl ScaleChangeDetectorInternal {
         let current_scale = super::scale_factor();
         let new_self = Rc::new(RefCell::new(Self {
             callback: Box::new(handler),
-            closure: None,
             mql: None,
             last_scale: current_scale,
         }));
 
-        let cloned_self = new_self.clone();
+        let weak_self = Rc::downgrade(&new_self);
         let closure = Closure::wrap(Box::new(move |event: MediaQueryListEvent| {
-            cloned_self.borrow_mut().handler(event)
+            if let Some(rc_self) = weak_self.upgrade() {
+                rc_self.borrow_mut().handler(event);
+            }
         }) as Box<dyn FnMut(_)>);
 
-        let mql = Self::create_mql(&closure);
+        let mql = Self::create_mql(closure);
         {
             let mut borrowed_self = new_self.borrow_mut();
-            borrowed_self.closure = Some(closure);
             borrowed_self.mql = mql;
         }
         new_self
     }
 
-    fn create_mql(closure: &Closure<dyn FnMut(MediaQueryListEvent)>) -> Option<MediaQueryList> {
-        let window = web_sys::window().expect("Failed to obtain window");
+    fn create_mql(
+        closure: Closure<dyn FnMut(MediaQueryListEvent)>,
+    ) -> Option<MediaQueryListHandle> {
         let current_scale = super::scale_factor();
         // This media query initially matches the current `devicePixelRatio`.
         // We add 0.0001 to the lower and upper bounds such that it won't fail
@@ -62,30 +63,20 @@ impl ScaleChangeDetectorInternal {
             current_scale - 0.0001,
             current_scale + 0.0001,
         );
-        window
-            .match_media(&media_query)
-            .ok()
-            .flatten()
-            .and_then(|mql| {
-                assert_eq!(mql.matches(), true);
-                mql.add_listener_with_opt_callback(Some(&closure.as_ref().unchecked_ref()))
-                    .map(|_| mql)
-                    .ok()
-            })
+        let mql = MediaQueryListHandle::new(&media_query, closure);
+        if let Some(mql) = &mql {
+            assert_eq!(mql.mql().matches(), true);
+        }
+        mql
     }
 
     fn handler(&mut self, event: MediaQueryListEvent) {
         assert_eq!(event.matches(), false);
-        let closure = self
-            .closure
-            .as_ref()
-            .expect("DevicePixelRatioChangeDetector::closure should not be None");
         let mql = self
             .mql
             .take()
             .expect("DevicePixelRatioChangeDetector::mql should not be None");
-        mql.remove_listener_with_opt_callback(Some(closure.as_ref().unchecked_ref()))
-            .expect("Failed to remove listener from MediaQueryList");
+        let closure = mql.remove();
         let new_scale = super::scale_factor();
         (self.callback)(ScaleChangeArgs {
             old_scale: self.last_scale,
@@ -96,15 +87,3 @@ impl ScaleChangeDetectorInternal {
         self.last_scale = new_scale;
     }
 }
-
-impl Drop for ScaleChangeDetectorInternal {
-    fn drop(&mut self) {
-        match (self.closure.as_ref(), self.mql.as_ref()) {
-            (Some(closure), Some(mql)) => {
-                let _ =
-                    mql.remove_listener_with_opt_callback(Some(closure.as_ref().unchecked_ref()));
-            }
-            _ => {}
-        }
-    }
-}