Browse Source

fix(core): remove window from HashMap on close (#2024)

Lucas Fernandes Nogueira 4 years ago
parent
commit
08c161c5e8

+ 5 - 0
.changes/remove-window-on-exit.md

@@ -0,0 +1,5 @@
+---
+"tauri": patch
+---
+
+Remove window object from the `Manager` internal `HashMap` on close. This fixes the behavior of using `[App|AppHandle|Window]#get_window` after the window is closed (now correctly returns `None`).

+ 30 - 19
core/tauri-runtime-wry/src/lib.rs

@@ -13,7 +13,7 @@ use tauri_runtime::{
     dpi::{LogicalPosition, LogicalSize, PhysicalPosition, PhysicalSize, Position, Size},
     DetachedWindow, PendingWindow, WindowEvent,
   },
-  Dispatch, Error, Icon, Params, Result, RunIteration, Runtime, RuntimeHandle,
+  Dispatch, Error, Icon, Params, Result, RunEvent, RunIteration, Runtime, RuntimeHandle,
 };
 
 #[cfg(feature = "menu")]
@@ -951,6 +951,7 @@ struct TrayContext {
 }
 
 struct WebviewWrapper {
+  label: String,
   inner: WebView,
   #[cfg(feature = "menu")]
   menu_items: HashMap<u32, WryCustomMenuItem>,
@@ -1130,7 +1131,7 @@ impl Runtime for Wry {
   }
 
   #[cfg(any(target_os = "windows", target_os = "macos"))]
-  fn run_iteration(&mut self) -> RunIteration {
+  fn run_iteration<F: Fn(RunEvent) + 'static>(&mut self, callback: F) -> RunIteration {
     use wry::application::platform::run_return::EventLoopExtRunReturn;
     let webviews = self.webviews.clone();
     let window_event_listeners = self.window_event_listeners.clone();
@@ -1153,7 +1154,7 @@ impl Runtime for Wry {
           event_loop,
           control_flow,
           EventLoopIterationContext {
-            callback: None,
+            callback: &callback,
             webviews: webviews.lock().expect("poisoned webview collection"),
             window_event_listeners: window_event_listeners.clone(),
             #[cfg(feature = "menu")]
@@ -1168,7 +1169,7 @@ impl Runtime for Wry {
     iteration
   }
 
-  fn run<F: Fn() + 'static>(self, callback: F) {
+  fn run<F: Fn(RunEvent) + 'static>(self, callback: F) {
     self.is_event_loop_running.store(true, Ordering::Relaxed);
     let webviews = self.webviews.clone();
     let window_event_listeners = self.window_event_listeners.clone();
@@ -1183,7 +1184,7 @@ impl Runtime for Wry {
         event_loop,
         control_flow,
         EventLoopIterationContext {
-          callback: Some(&callback),
+          callback: &callback,
           webviews: webviews.lock().expect("poisoned webview collection"),
           window_event_listeners: window_event_listeners.clone(),
           #[cfg(feature = "menu")]
@@ -1197,7 +1198,7 @@ impl Runtime for Wry {
 }
 
 struct EventLoopIterationContext<'a> {
-  callback: Option<&'a (dyn Fn() + 'static)>,
+  callback: &'a (dyn Fn(RunEvent) + 'static),
   webviews: MutexGuard<'a, HashMap<WindowId, WebviewWrapper>>,
   window_event_listeners: WindowEventListeners,
   #[cfg(feature = "menu")]
@@ -1273,13 +1274,7 @@ fn handle_event_loop(
       }
       match event {
         WryWindowEvent::CloseRequested => {
-          webviews.remove(&window_id);
-          if webviews.is_empty() {
-            *control_flow = ControlFlow::Exit;
-            if let Some(callback) = callback {
-              callback();
-            }
-          }
+          on_window_close(callback, window_id, &mut webviews, control_flow);
         }
         WryWindowEvent::Resized(_) => {
           if let Err(e) = webviews[&window_id].inner.resize() {
@@ -1365,10 +1360,7 @@ fn handle_event_loop(
             WindowMessage::Show => window.set_visible(true),
             WindowMessage::Hide => window.set_visible(false),
             WindowMessage::Close => {
-              webviews.remove(&id);
-              if webviews.is_empty() {
-                *control_flow = ControlFlow::Exit;
-              }
+              on_window_close(callback, id, &mut webviews, control_flow);
             }
             WindowMessage::SetDecorations(decorations) => window.set_decorations(decorations),
             WindowMessage::SetAlwaysOnTop(always_on_top) => window.set_always_on_top(always_on_top),
@@ -1489,6 +1481,21 @@ fn handle_event_loop(
   }
 }
 
+fn on_window_close<'a>(
+  callback: &'a (dyn Fn(RunEvent) + 'static),
+  window_id: WindowId,
+  webviews: &mut MutexGuard<'a, HashMap<WindowId, WebviewWrapper>>,
+  control_flow: &mut ControlFlow,
+) {
+  if let Some(webview) = webviews.remove(&window_id) {
+    callback(RunEvent::WindowClose(webview.label));
+  }
+  if webviews.is_empty() {
+    *control_flow = ControlFlow::Exit;
+    callback(RunEvent::Exit);
+  }
+}
+
 fn center_window(window: &Window) -> Result<()> {
   if let Some(monitor) = window.current_monitor() {
     let screen_size = monitor.size();
@@ -1534,8 +1541,11 @@ fn create_webview<P: Params<Runtime = Wry>>(
       webview_builder.with_rpc_handler(create_rpc_handler(context.clone(), label.clone(), handler));
   }
   if let Some(handler) = file_drop_handler {
-    webview_builder =
-      webview_builder.with_file_drop_handler(create_file_drop_handler(context, label, handler));
+    webview_builder = webview_builder.with_file_drop_handler(create_file_drop_handler(
+      context,
+      label.clone(),
+      handler,
+    ));
   }
   for (scheme, protocol) in webview_attributes.uri_scheme_protocols {
     webview_builder = webview_builder.with_custom_protocol(scheme, move |_window, url| {
@@ -1558,6 +1568,7 @@ fn create_webview<P: Params<Runtime = Wry>>(
     .map_err(|e| Error::CreateWebview(Box::new(e)))?;
 
   Ok(WebviewWrapper {
+    label: format!("{}", label),
     inner: webview,
     #[cfg(feature = "menu")]
     menu_items,

+ 10 - 2
core/tauri-runtime/src/lib.rs

@@ -169,6 +169,14 @@ impl Icon {
   }
 }
 
+/// Event triggered on the event loop run.
+pub enum RunEvent {
+  /// Event loop is exiting.
+  Exit,
+  /// Window closed.
+  WindowClose(String),
+}
+
 /// A system tray event.
 pub enum SystemTrayEvent {
   MenuItemClick(u32),
@@ -240,10 +248,10 @@ pub trait Runtime: Sized + 'static {
 
   /// Runs the one step of the webview runtime event loop and returns control flow to the caller.
   #[cfg(any(target_os = "windows", target_os = "macos"))]
-  fn run_iteration(&mut self) -> RunIteration;
+  fn run_iteration<F: Fn(RunEvent) + 'static>(&mut self, callback: F) -> RunIteration;
 
   /// Run the webview runtime.
-  fn run<F: Fn() + 'static>(self, callback: F);
+  fn run<F: Fn(RunEvent) + 'static>(self, callback: F);
 }
 
 /// Webview dispatcher. A thread-safe handle to the webview API.

+ 25 - 10
core/tauri/src/app.rs

@@ -16,7 +16,7 @@ use crate::{
     tag::Tag,
     webview::{CustomProtocol, WebviewAttributes, WindowBuilder},
     window::{PendingWindow, WindowEvent},
-    Dispatch, MenuId, Params, Runtime,
+    Dispatch, MenuId, Params, RunEvent, Runtime,
   },
   sealed::{ManagerBase, RuntimeOrDispatch},
   Context, Invoke, Manager, StateManager, Window,
@@ -260,7 +260,12 @@ impl<P: Params> App<P> {
   /// }
   #[cfg(any(target_os = "windows", target_os = "macos"))]
   pub fn run_iteration(&mut self) -> crate::runtime::RunIteration {
-    self.runtime.as_mut().unwrap().run_iteration()
+    let manager = self.manager.clone();
+    self
+      .runtime
+      .as_mut()
+      .unwrap()
+      .run_iteration(move |event| on_event_loop_event(event, &manager))
   }
 }
 
@@ -807,20 +812,30 @@ where
     let mut app = self.build(context)?;
     #[cfg(all(windows, feature = "system-tray"))]
     let app_handle = app.handle();
-    app.runtime.take().unwrap().run(move || {
-      #[cfg(shell_execute)]
-      {
-        crate::api::process::kill_children();
-      }
-      #[cfg(all(windows, feature = "system-tray"))]
-      {
-        let _ = app_handle.remove_system_tray();
+    let manager = app.manager.clone();
+    app.runtime.take().unwrap().run(move |event| match event {
+      RunEvent::Exit => {
+        #[cfg(shell_execute)]
+        {
+          crate::api::process::kill_children();
+        }
+        #[cfg(all(windows, feature = "system-tray"))]
+        {
+          let _ = app_handle.remove_system_tray();
+        }
       }
+      _ => on_event_loop_event(event, &manager),
     });
     Ok(())
   }
 }
 
+fn on_event_loop_event<P: Params>(event: RunEvent, manager: &WindowManager<P>) {
+  if let RunEvent::WindowClose(label) = event {
+    manager.on_window_close(label);
+  }
+}
+
 /// Make `Wry` the default `Runtime` for `Builder`
 #[cfg(feature = "wry")]
 impl<A: Assets> Default for Builder<String, String, String, String, A, crate::Wry> {

+ 6 - 0
core/tauri/src/manager.rs

@@ -685,6 +685,12 @@ impl<P: Params> WindowManager<P> {
     window
   }
 
+  pub(crate) fn on_window_close(&self, label: String) {
+    self
+      .windows_lock()
+      .remove(&label.parse().unwrap_or_else(|_| panic!("bad label")));
+  }
+
   pub fn emit_filter<E: ?Sized, S, F>(&self, event: &E, payload: S, filter: F) -> crate::Result<()>
   where
     P::Event: Borrow<E>,