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

CHANGELOG.md

@@ -35,6 +35,11 @@
 - Deprecate the stdweb backend, to be removed in a future release
 - **Breaking:** Prefixed virtual key codes `Add`, `Multiply`, `Divide`, `Decimal`, and `Subtract` with `Numpad`.
 - Added `Asterisk` and `Plus` virtual key codes.
+- On Web (web-sys only), the `Event::LoopDestroyed` event is correctly emitted when leaving the page.
+- On Web, the `WindowEvent::Destroyed` event now gets emitted when a `Window` is dropped.
+- On Web (web-sys only), the event listeners are now removed when a `Window` is dropped or when the event loop is destroyed.
+- On Web, the event handler closure passed to `EventLoop::run` now gets dropped after the event loop is destroyed.
+- **Breaking:** On Web, the canvas element associated to a `Window` is no longer removed from the DOM when the `Window` is dropped.
 
 # 0.22.2 (2020-05-16)
 

src/platform_impl/web/event_loop/runner.rs

@@ -9,7 +9,7 @@ use std::{
     clone::Clone,
     collections::{HashSet, VecDeque},
     iter,
-    rc::Rc,
+    rc::{Rc, Weak},
 };
 
 pub struct Shared<T: 'static>(Rc<Execution<T>>);
@@ -21,12 +21,34 @@ impl<T> Clone for Shared<T> {
 }
 
 pub struct Execution<T: 'static> {
-    runner: RefCell<Option<Runner<T>>>,
+    runner: RefCell<RunnerEnum<T>>,
     events: RefCell<VecDeque<Event<'static, T>>>,
     id: RefCell<u32>,
-    all_canvases: RefCell<Vec<(WindowId, backend::RawCanvasType)>>,
+    all_canvases: RefCell<Vec<(WindowId, Weak<RefCell<backend::Canvas>>)>>,
     redraw_pending: RefCell<HashSet<WindowId>>,
+    destroy_pending: RefCell<VecDeque<WindowId>>,
     scale_change_detector: RefCell<Option<backend::ScaleChangeDetector>>,
+    unload_event_handle: RefCell<Option<backend::UnloadEventHandle>>,
+}
+
+enum RunnerEnum<T: 'static> {
+    /// The `EventLoop` is created but not being run.
+    Pending,
+    /// The `EventLoop` is being run.
+    Running(Runner<T>),
+    /// The `EventLoop` is exited after being started with `EventLoop::run`. Since
+    /// `EventLoop::run` takes ownership of the `EventLoop`, we can be certain
+    /// that this event loop will never be run again.
+    Destroyed,
+}
+
+impl<T: 'static> RunnerEnum<T> {
+    fn maybe_runner(&self) -> Option<&Runner<T>> {
+        match self {
+            RunnerEnum::Running(runner) => Some(runner),
+            _ => None,
+        }
+    }
 }
 
 struct Runner<T: 'static> {
@@ -83,17 +105,26 @@ impl<T: 'static> Runner<T> {
 impl<T: 'static> Shared<T> {
     pub fn new() -> Self {
         Shared(Rc::new(Execution {
-            runner: RefCell::new(None),
+            runner: RefCell::new(RunnerEnum::Pending),
             events: RefCell::new(VecDeque::new()),
             id: RefCell::new(0),
             all_canvases: RefCell::new(Vec::new()),
             redraw_pending: RefCell::new(HashSet::new()),
+            destroy_pending: RefCell::new(VecDeque::new()),
             scale_change_detector: RefCell::new(None),
+            unload_event_handle: RefCell::new(None),
         }))
     }
 
-    pub fn add_canvas(&self, id: WindowId, canvas: backend::RawCanvasType) {
-        self.0.all_canvases.borrow_mut().push((id, canvas));
+    pub fn add_canvas(&self, id: WindowId, canvas: &Rc<RefCell<backend::Canvas>>) {
+        self.0
+            .all_canvases
+            .borrow_mut()
+            .push((id, Rc::downgrade(canvas)));
+    }
+
+    pub fn notify_destroy_window(&self, id: WindowId) {
+        self.0.destroy_pending.borrow_mut().push_back(id);
     }
 
     // Set the event callback to use for the event loop runner
@@ -103,11 +134,16 @@ impl<T: 'static> Shared<T> {
         &self,
         event_handler: Box<dyn FnMut(Event<'_, T>, &mut root::ControlFlow)>,
     ) {
-        self.0.runner.replace(Some(Runner::new(event_handler)));
+        {
+            let mut runner = self.0.runner.borrow_mut();
+            assert!(matches!(*runner, RunnerEnum::Pending));
+            *runner = RunnerEnum::Running(Runner::new(event_handler));
+        }
         self.init();
 
         let close_instance = self.clone();
-        backend::on_unload(move || close_instance.handle_unload());
+        *self.0.unload_event_handle.borrow_mut() =
+            Some(backend::on_unload(move || close_instance.handle_unload()));
     }
 
     pub(crate) fn set_on_scale_change<F>(&self, handler: F)
@@ -169,18 +205,23 @@ impl<T: 'static> Shared<T> {
         }
         // If we can run the event processing right now, or need to queue this and wait for later
         let mut process_immediately = true;
-        if let Some(ref runner) = &*self.0.runner.borrow() {
-            // If we're currently polling, queue this and wait for the poll() method to be called
-            if let State::Poll { .. } = runner.state {
-                process_immediately = false;
+        match &*self.0.runner.borrow() {
+            RunnerEnum::Running(ref runner) => {
+                // If we're currently polling, queue this and wait for the poll() method to be called
+                if let State::Poll { .. } = runner.state {
+                    process_immediately = false;
+                }
+                // If the runner is busy, queue this and wait for it to process it later
+                if runner.is_busy {
+                    process_immediately = false;
+                }
             }
-            // If the runner is busy, queue this and wait for it to process it later
-            if runner.is_busy {
+            RunnerEnum::Pending => {
+                // The runner still hasn't been attached: queue this event and wait for it to be
                 process_immediately = false;
             }
-        } else {
-            // The runner still hasn't been attached: queue this event and wait for it to be
-            process_immediately = false;
+            // This is unreachable since `self.is_closed() == true`.
+            RunnerEnum::Destroyed => unreachable!(),
         }
         if !process_immediately {
             // Queue these events to look at later
@@ -189,7 +230,7 @@ impl<T: 'static> Shared<T> {
         }
         // At this point, we know this is a fresh set of events
         // Now we determine why new events are incoming, and handle the events
-        let start_cause = match (self.0.runner.borrow().as_ref())
+        let start_cause = match (self.0.runner.borrow().maybe_runner())
             .unwrap_or_else(|| {
                 unreachable!("The runner cannot process events when it is not attached")
             })
@@ -206,6 +247,26 @@ impl<T: 'static> Shared<T> {
         self.run_until_cleared(events);
     }
 
+    // Process the destroy-pending windows. This should only be called from
+    // `run_until_cleared` and `handle_scale_changed`, somewhere between emitting
+    // `NewEvents` and `MainEventsCleared`.
+    fn process_destroy_pending_windows(&self, control: &mut root::ControlFlow) {
+        while let Some(id) = self.0.destroy_pending.borrow_mut().pop_front() {
+            self.0
+                .all_canvases
+                .borrow_mut()
+                .retain(|&(item_id, _)| item_id != id);
+            self.handle_event(
+                Event::WindowEvent {
+                    window_id: id,
+                    event: crate::event::WindowEvent::Destroyed,
+                },
+                control,
+            );
+            self.0.redraw_pending.borrow_mut().remove(&id);
+        }
+    }
+
     // Given the set of new events, run the event loop until the main events and redraw events are
     // cleared
     //
@@ -215,6 +276,7 @@ impl<T: 'static> Shared<T> {
         for event in events {
             self.handle_event(event, &mut control);
         }
+        self.process_destroy_pending_windows(&mut control);
         self.handle_event(Event::MainEventsCleared, &mut control);
 
         // Collect all of the redraw events to avoid double-locking the RefCell
@@ -228,12 +290,17 @@ impl<T: 'static> Shared<T> {
         // If the event loop is closed, it has been closed this iteration and now the closing
         // event should be emitted
         if self.is_closed() {
-            self.handle_event(Event::LoopDestroyed, &mut control);
+            self.handle_loop_destroyed(&mut control);
         }
     }
 
     pub fn handle_scale_changed(&self, old_scale: f64, new_scale: f64) {
-        let start_cause = match (self.0.runner.borrow().as_ref())
+        // If there aren't any windows, then there is nothing to do here.
+        if self.0.all_canvases.borrow().is_empty() {
+            return;
+        }
+
+        let start_cause = match (self.0.runner.borrow().maybe_runner())
             .unwrap_or_else(|| unreachable!("`scale_changed` should not happen without a runner"))
             .maybe_start_cause()
         {
@@ -246,8 +313,18 @@ impl<T: 'static> Shared<T> {
         // Handle the start event and all other events in the queue.
         self.handle_event(Event::NewEvents(start_cause), &mut control);
 
+        // It is possible for windows to be dropped before this point. We don't
+        // want to send `ScaleFactorChanged` for destroyed windows, so we process
+        // the destroy-pending windows here.
+        self.process_destroy_pending_windows(&mut control);
+
         // Now handle the `ScaleFactorChanged` events.
         for &(id, ref canvas) in &*self.0.all_canvases.borrow() {
+            let canvas = match canvas.upgrade() {
+                Some(rc) => rc.borrow().raw().clone(),
+                // This shouldn't happen, but just in case...
+                None => continue,
+            };
             // First, we send the `ScaleFactorChanged` event:
             let current_size = crate::dpi::PhysicalSize {
                 width: canvas.width() as u32,
@@ -267,7 +344,7 @@ impl<T: 'static> Shared<T> {
             );
 
             // Then we resize the canvas to the new size and send a `Resized` event:
-            backend::set_canvas_size(canvas, crate::dpi::Size::Physical(new_size));
+            backend::set_canvas_size(&canvas, crate::dpi::Size::Physical(new_size));
             self.handle_single_event_sync(
                 Event::WindowEvent {
                     window_id: id,
@@ -277,6 +354,8 @@ impl<T: 'static> Shared<T> {
             );
         }
 
+        // Process the destroy-pending windows again.
+        self.process_destroy_pending_windows(&mut control);
         self.handle_event(Event::MainEventsCleared, &mut control);
 
         // Discard all the pending redraw as we shall just redraw all windows.
@@ -290,13 +369,15 @@ impl<T: 'static> Shared<T> {
         // If the event loop is closed, it has been closed this iteration and now the closing
         // event should be emitted
         if self.is_closed() {
-            self.handle_event(Event::LoopDestroyed, &mut control);
+            self.handle_loop_destroyed(&mut control);
         }
     }
 
     fn handle_unload(&self) {
         self.apply_control_flow(root::ControlFlow::Exit);
         let mut control = self.current_control_flow();
+        // We don't call `handle_loop_destroyed` here because we don't need to
+        // perform cleanup when the web browser is going to destroy the page.
         self.handle_event(Event::LoopDestroyed, &mut control);
     }
 
@@ -308,7 +389,7 @@ impl<T: 'static> Shared<T> {
             *control = root::ControlFlow::Exit;
         }
         match *self.0.runner.borrow_mut() {
-            Some(ref mut runner) => {
+            RunnerEnum::Running(ref mut runner) => {
                 runner.handle_single_event(event, control);
             }
             _ => panic!("Cannot handle event synchronously without a runner"),
@@ -323,19 +404,21 @@ impl<T: 'static> Shared<T> {
             *control = root::ControlFlow::Exit;
         }
         match *self.0.runner.borrow_mut() {
-            Some(ref mut runner) => {
+            RunnerEnum::Running(ref mut runner) => {
                 runner.handle_single_event(event, control);
             }
             // If an event is being handled without a runner somehow, add it to the event queue so
             // it will eventually be processed
-            _ => self.0.events.borrow_mut().push_back(event),
+            RunnerEnum::Pending => self.0.events.borrow_mut().push_back(event),
+            // If the Runner has been destroyed, there is nothing to do.
+            RunnerEnum::Destroyed => return,
         }
 
         let is_closed = *control == root::ControlFlow::Exit;
 
         // Don't take events out of the queue if the loop is closed or the runner doesn't exist
         // If the runner doesn't exist and this method recurses, it will recurse infinitely
-        if !is_closed && self.0.runner.borrow().is_some() {
+        if !is_closed && self.0.runner.borrow().maybe_runner().is_some() {
             // Take an event out of the queue and handle it
             // Make sure not to let the borrow_mut live during the next handle_event
             let event = { self.0.events.borrow_mut().pop_front() };
@@ -382,26 +465,58 @@ impl<T: 'static> Shared<T> {
         };
 
         match *self.0.runner.borrow_mut() {
-            Some(ref mut runner) => {
+            RunnerEnum::Running(ref mut runner) => {
                 runner.state = new_state;
             }
-            None => (),
+            _ => (),
+        }
+    }
+
+    fn handle_loop_destroyed(&self, control: &mut root::ControlFlow) {
+        self.handle_event(Event::LoopDestroyed, control);
+        let all_canvases = std::mem::take(&mut *self.0.all_canvases.borrow_mut());
+        *self.0.scale_change_detector.borrow_mut() = None;
+        *self.0.unload_event_handle.borrow_mut() = None;
+        // Dropping the `Runner` drops the event handler closure, which will in
+        // turn drop all `Window`s moved into the closure.
+        *self.0.runner.borrow_mut() = RunnerEnum::Destroyed;
+        for (_, canvas) in all_canvases {
+            // In case any remaining `Window`s are still not dropped, we will need
+            // to explicitly remove the event handlers associated with their canvases.
+            if let Some(canvas) = canvas.upgrade() {
+                let mut canvas = canvas.borrow_mut();
+                canvas.remove_listeners();
+            }
         }
+        // At this point, the `self.0` `Rc` should only be strongly referenced
+        // by the following:
+        // * `self`, i.e. the item which triggered this event loop wakeup, which
+        //   is usually a `wasm-bindgen` `Closure`, which will be dropped after
+        //   returning to the JS glue code.
+        // * The `EventLoopWindowTarget` leaked inside `EventLoop::run` due to the
+        //   JS exception thrown at the end.
+        // * For each undropped `Window`:
+        //     * The `register_redraw_request` closure.
+        //     * The `destroy_fn` closure.
     }
 
     // Check if the event loop is currently closed
     fn is_closed(&self) -> bool {
         match *self.0.runner.borrow() {
-            Some(ref runner) => runner.state.is_exit(),
-            None => false, // If the event loop is None, it has not been intialised yet, so it cannot be closed
+            RunnerEnum::Running(ref runner) => runner.state.is_exit(),
+            // The event loop is not closed since it is not initialized.
+            RunnerEnum::Pending => false,
+            // The event loop is closed since it has been destroyed.
+            RunnerEnum::Destroyed => true,
         }
     }
 
     // Get the current control flow state
     fn current_control_flow(&self) -> root::ControlFlow {
         match *self.0.runner.borrow() {
-            Some(ref runner) => runner.state.control_flow(),
-            None => root::ControlFlow::Poll,
+            RunnerEnum::Running(ref runner) => runner.state.control_flow(),
+            RunnerEnum::Pending => root::ControlFlow::Poll,
+            RunnerEnum::Destroyed => root::ControlFlow::Exit,
         }
     }
 }

src/platform_impl/web/event_loop/window_target.rs

@@ -4,8 +4,10 @@ use crate::event::{DeviceId, ElementState, Event, KeyboardInput, TouchPhase, Win
 use crate::event_loop::ControlFlow;
 use crate::monitor::MonitorHandle as RootMH;
 use crate::window::{Theme, WindowId};
+use std::cell::RefCell;
 use std::clone::Clone;
 use std::collections::{vec_deque::IntoIter as VecDequeIter, VecDeque};
+use std::rc::Rc;
 
 pub struct WindowTarget<T: 'static> {
     pub(crate) runner: runner::Shared<T>,
@@ -42,11 +44,12 @@ impl<T> WindowTarget<T> {
         window::Id(self.runner.generate_id())
     }
 
-    pub fn register(&self, canvas: &mut backend::Canvas, id: window::Id) {
-        let runner = self.runner.clone();
+    pub fn register(&self, canvas: &Rc<RefCell<backend::Canvas>>, id: window::Id) {
+        self.runner.add_canvas(WindowId(id), canvas);
+        let mut canvas = canvas.borrow_mut();
         canvas.set_attribute("data-raw-handle", &id.0.to_string());
-        runner.add_canvas(WindowId(id), canvas.raw().clone());
 
+        let runner = self.runner.clone();
         canvas.on_blur(move || {
             runner.send_event(Event::WindowEvent {
                 window_id: WindowId(id),