Browse Source

Fixed the multiple window closing issue, where the two multiwindow examples weren't working. (#3499)

* fixed window when multiple windows not closing

* fixed window when multiple windows not closing

* fixed accidental includes

* polished

* fix last window closing hiding app and rc hardcount

* find the suspect strong count holders and fix them

* Use the proper type alias

* fmt

---------

Co-authored-by: Jonathan Kelley <jkelleyrtp@gmail.com>
sertschgi 5 months ago
parent
commit
25a64d97c0

+ 5 - 1
examples/multiwindow.rs

@@ -5,9 +5,13 @@
 //! own context, root elements, etc.
 
 use dioxus::prelude::*;
+use dioxus::{desktop::Config, desktop::WindowCloseBehaviour};
 
 fn main() {
-    dioxus::LaunchBuilder::desktop().launch(app);
+    dioxus::LaunchBuilder::desktop()
+        // We can choose the close behavior of the last window to hide. See WindowCloseBehaviour for more options.
+        .with_cfg(Config::new().with_close_behaviour(WindowCloseBehaviour::LastWindowHides))
+        .launch(app);
 }
 
 fn app() -> Element {

+ 14 - 7
packages/desktop/src/app.rs

@@ -20,7 +20,7 @@ use tao::{
     dpi::PhysicalSize,
     event::Event,
     event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget},
-    window::WindowId,
+    window::{Window, WindowId},
 };
 
 /// The single top-level object that manages all the running windows, assets, shortcuts, etc
@@ -198,11 +198,14 @@ impl App {
                 }
             }
 
+            LastWindowHides if self.webviews.len() > 1 => {
+                self.webviews.remove(&id);
+            }
+
             LastWindowHides => {
-                let Some(webview) = self.webviews.get(&id) else {
-                    return;
-                };
-                hide_app_window(&webview.desktop_context.webview);
+                if let Some(webview) = self.webviews.get(&id) {
+                    hide_last_window(&webview.desktop_context.window);
+                }
             }
 
             CloseWindow => {
@@ -529,9 +532,13 @@ struct PreservedWindowState {
     monitor: String,
 }
 
-/// Different hide implementations per platform
+/// Hide the last window when using LastWindowHides.
+///
+/// On macOS, if we use `set_visibility(false)` on the window, it will hide the window but not show
+/// it again when the user switches back to the app. `NSApplication::hide:` has the correct behaviour,
+/// so we need to special case it.
 #[allow(unused)]
-pub fn hide_app_window(window: &wry::WebView) {
+fn hide_last_window(window: &Window) {
     #[cfg(target_os = "windows")]
     {
         use tao::platform::windows::WindowExtWindows;

+ 6 - 1
packages/desktop/src/desktop_context.rs

@@ -35,6 +35,11 @@ pub fn window() -> DesktopContext {
 /// A handle to the [`DesktopService`] that can be passed around.
 pub type DesktopContext = Rc<DesktopService>;
 
+/// A weak handle to the [`DesktopService`] to ensure safe passing.
+/// The problem without this is that the tao window is never dropped and therefore cannot be closed.
+/// This was due to the Rc that had still references because of multiple copies when creating a webview.
+pub type WeakDesktopContext = Weak<DesktopService>;
+
 /// An imperative interface to the current window.
 ///
 /// To get a handle to the current window, use the [`use_window`] hook.
@@ -101,7 +106,7 @@ impl DesktopService {
     /// You can use this to control other windows from the current window.
     ///
     /// Be careful to not create a cycle of windows, or you might leak memory.
-    pub fn new_window(&self, dom: VirtualDom, cfg: Config) -> Weak<DesktopService> {
+    pub fn new_window(&self, dom: VirtualDom, cfg: Config) -> WeakDesktopContext {
         let window = WebviewInstance::new(cfg, dom, self.shared.clone());
 
         let cx = window.dom.in_runtime(|| {

+ 13 - 5
packages/desktop/src/document.rs

@@ -1,11 +1,11 @@
+use crate::{query::Query, DesktopContext, WeakDesktopContext};
 use dioxus_core::prelude::queue_effect;
 use dioxus_document::{
     create_element_in_head, Document, Eval, EvalError, Evaluator, LinkProps, MetaProps,
     ScriptProps, StyleProps,
 };
-use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage};
 
-use crate::{query::Query, DesktopContext};
+use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage};
 
 /// Code for the Dioxus channel used to communicate between the dioxus and javascript code
 pub const NATIVE_EVAL_JS: &str = include_str!("./js/native_eval.js");
@@ -13,22 +13,30 @@ pub const NATIVE_EVAL_JS: &str = include_str!("./js/native_eval.js");
 /// Represents the desktop-target's provider of evaluators.
 #[derive(Clone)]
 pub struct DesktopDocument {
-    pub(crate) desktop_ctx: DesktopContext,
+    pub(crate) desktop_ctx: WeakDesktopContext,
 }
 
 impl DesktopDocument {
     pub fn new(desktop_ctx: DesktopContext) -> Self {
+        let desktop_ctx = std::rc::Rc::downgrade(&desktop_ctx);
         Self { desktop_ctx }
     }
 }
 
 impl Document for DesktopDocument {
     fn eval(&self, js: String) -> Eval {
-        Eval::new(DesktopEvaluator::create(self.desktop_ctx.clone(), js))
+        Eval::new(DesktopEvaluator::create(
+            self.desktop_ctx
+                .upgrade()
+                .expect("Window to exist when document is alive"),
+            js,
+        ))
     }
 
     fn set_title(&self, title: String) {
-        self.desktop_ctx.window.set_title(&title);
+        if let Some(ctx) = self.desktop_ctx.upgrade() {
+            ctx.set_title(&title);
+        }
     }
 
     /// Create a new meta tag in the head

+ 1 - 1
packages/desktop/src/lib.rs

@@ -48,7 +48,7 @@ pub mod trayicon;
 // Public exports
 pub use assets::AssetRequest;
 pub use config::{Config, WindowCloseBehaviour};
-pub use desktop_context::{window, DesktopContext, DesktopService};
+pub use desktop_context::{window, DesktopContext, DesktopService, WeakDesktopContext};
 pub use event_handlers::WryEventHandler;
 pub use hooks::*;
 pub use shortcut::{ShortcutHandle, ShortcutRegistryError};

+ 6 - 4
packages/desktop/src/webview.rs

@@ -1,4 +1,3 @@
-use crate::document::DesktopDocument;
 use crate::element::DesktopElement;
 use crate::file_upload::DesktopFileDragEvent;
 use crate::menubar::DioxusMenu;
@@ -12,6 +11,7 @@ use crate::{
     waker::tao_waker,
     Config, DesktopContext, DesktopService,
 };
+use crate::{document::DesktopDocument, WeakDesktopContext};
 use base64::prelude::BASE64_STANDARD;
 use dioxus_core::{Runtime, ScopeId, VirtualDom};
 use dioxus_document::Document;
@@ -28,7 +28,7 @@ use wry::{DragDropEvent, RequestAsyncResponder, WebContext, WebViewBuilder};
 pub(crate) struct WebviewEdits {
     runtime: Rc<Runtime>,
     pub wry_queue: WryQueue,
-    desktop_context: Rc<OnceCell<DesktopContext>>,
+    desktop_context: Rc<OnceCell<WeakDesktopContext>>,
 }
 
 impl WebviewEdits {
@@ -40,7 +40,7 @@ impl WebviewEdits {
         }
     }
 
-    fn set_desktop_context(&self, context: DesktopContext) {
+    fn set_desktop_context(&self, context: WeakDesktopContext) {
         _ = self.desktop_context.set(context);
     }
 
@@ -115,6 +115,8 @@ impl WebviewEdits {
             return Default::default();
         };
 
+        let desktop_context = desktop_context.upgrade().unwrap();
+
         let query = desktop_context.query.clone();
         let recent_file = desktop_context.file_hover.clone();
 
@@ -412,7 +414,7 @@ impl WebviewInstance {
         ));
 
         // Provide the desktop context to the virtual dom and edit handler
-        edits.set_desktop_context(desktop_context.clone());
+        edits.set_desktop_context(Rc::downgrade(&desktop_context));
         let provider: Rc<dyn Document> = Rc::new(DesktopDocument::new(desktop_context.clone()));
         let history_provider: Rc<dyn History> = Rc::new(MemoryHistory::default());
         dom.in_runtime(|| {