From 3bde0d7fb2dc5d16b8e7599bdcd73b8455116c2a Mon Sep 17 00:00:00 2001 From: sertschgi Date: Fri, 3 Jan 2025 14:11:04 +0100 Subject: [PATCH 1/4] fixed window when multiple windows not closing --- packages/desktop/src/app.rs | 30 +++++++++++++++---------- packages/desktop/src/desktop_context.rs | 23 +++++++++++++------ packages/desktop/src/document.rs | 20 +++++++++++------ packages/desktop/src/element.rs | 7 +++--- packages/desktop/src/lib.rs | 2 +- packages/desktop/src/query.rs | 12 +++++----- packages/desktop/src/webview.rs | 28 ++++++++++++++++------- 7 files changed, 79 insertions(+), 43 deletions(-) diff --git a/packages/desktop/src/app.rs b/packages/desktop/src/app.rs index d41f552506..cd1841c390 100644 --- a/packages/desktop/src/app.rs +++ b/packages/desktop/src/app.rs @@ -1,12 +1,4 @@ -use crate::{ - config::{Config, WindowCloseBehaviour}, - event_handlers::WindowEventHandlers, - file_upload::{DesktopFileUploadForm, FileDialogRequest, NativeFileEngine}, - ipc::{IpcMessage, UserWindowEvent}, - query::QueryResult, - shortcut::ShortcutRegistry, - webview::WebviewInstance, -}; +use crate::{config::{Config, WindowCloseBehaviour}, event_handlers::WindowEventHandlers, file_upload::{DesktopFileUploadForm, FileDialogRequest, NativeFileEngine}, ipc::{IpcMessage, UserWindowEvent}, query::QueryResult, shortcut::ShortcutRegistry, webview::WebviewInstance, window}; use dioxus_core::{ElementId, VirtualDom}; use dioxus_html::PlatformEventData; use std::{ @@ -22,6 +14,9 @@ use tao::{ event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget}, window::WindowId, }; +use tao::platform::unix::{EventLoopWindowTargetExtUnix, WindowExtUnix}; +use tracing::instrument::WithSubscriber; +use wry::WebViewExtUnix; /// The single top-level object that manages all the running windows, assets, shortcuts, etc pub(crate) struct App { @@ -178,6 +173,7 @@ impl App { pub fn handle_new_window(&mut self) { for handler in self.shared.pending_webviews.borrow_mut().drain(..) { + println!("Create handler strong count: {}", Rc::strong_count(&handler.desktop_context)); let id = handler.desktop_context.window.id(); self.webviews.insert(id, handler); _ = self.shared.proxy.send_event(UserWindowEvent::Poll(id)); @@ -189,10 +185,20 @@ impl App { match self.window_behavior { LastWindowExitsApp => { - #[cfg(debug_assertions)] - self.persist_window_state(); + // #[cfg(debug_assertions)] + // self.persist_window_state(); + if let Some(webview) = self.webviews.remove(&id) + { + // println!("Exit strong count: {}", Rc::strong_count(&webview.desktop_context)); + // let mut raw = Rc::into_raw(webview.desktop_context); + // unsafe { + // Rc::decrement_strong_count(raw); + // Rc::decrement_strong_count(raw); + // Rc::decrement_strong_count(raw); + // Rc::decrement_strong_count(raw); + // } + } - self.webviews.remove(&id); if self.webviews.is_empty() { self.control_flow = ControlFlow::Exit } diff --git a/packages/desktop/src/desktop_context.rs b/packages/desktop/src/desktop_context.rs index 9480f9e74b..a3d111713b 100644 --- a/packages/desktop/src/desktop_context.rs +++ b/packages/desktop/src/desktop_context.rs @@ -29,12 +29,18 @@ use tao::platform::ios::WindowExtIOS; /// /// This function will panic if it is called outside of the context of a Dioxus App. pub fn window() -> DesktopContext { - dioxus_core::prelude::consume_context() + let ctx: WeakDesktopContext = dioxus_core::prelude::consume_context(); + ctx.upgrade().unwrap() // todo: implement error } /// A handle to the [`DesktopService`] that can be passed around. pub type DesktopContext = Rc; +/// 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; + /// An imperative interface to the current window. /// /// To get a handle to the current window, use the [`use_window`] hook. @@ -101,12 +107,12 @@ 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 { + 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(|| { ScopeId::ROOT - .consume_context::>() + .consume_context() .unwrap() }); @@ -115,9 +121,12 @@ impl DesktopService { .send_event(UserWindowEvent::NewWindow) .unwrap(); + + // println!("Create handler strong count in new_window: {}", Rc::strong_count(&window.desktop_context)); + self.shared.pending_webviews.borrow_mut().push(window); - Rc::downgrade(&cx) + cx } /// trigger the drag-window event @@ -142,9 +151,9 @@ impl DesktopService { /// Close this window pub fn close(&self) { let _ = self - .shared - .proxy - .send_event(UserWindowEvent::CloseWindow(self.id())); + .shared + .proxy + .send_event(UserWindowEvent::CloseWindow(self.id())); } /// Close a particular window, given its ID diff --git a/packages/desktop/src/document.rs b/packages/desktop/src/document.rs index fbc9fbfb2f..efb545ca2f 100644 --- a/packages/desktop/src/document.rs +++ b/packages/desktop/src/document.rs @@ -1,18 +1,22 @@ +use std::default; +use std::rc::Weak; +use webbrowser::Browser::Default; use dioxus_document::{Document, Eval, EvalError, Evaluator}; +use dioxus_html::track::default; use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage}; -use crate::{query::Query, DesktopContext}; +use crate::{query::Query, DesktopContext, WeakDesktopContext}; /// 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"); /// Represents the desktop-target's provider of evaluators. pub struct DesktopDocument { - pub(crate) desktop_ctx: DesktopContext, + pub(crate) desktop_ctx: WeakDesktopContext, } impl DesktopDocument { - pub fn new(desktop_ctx: DesktopContext) -> Self { + pub fn new(desktop_ctx: WeakDesktopContext) -> Self { Self { desktop_ctx } } } @@ -23,7 +27,9 @@ impl Document for DesktopDocument { } fn set_title(&self, title: String) { - self.desktop_ctx.window.set_title(&title); + if let Some(desktop_ctx) = self.desktop_ctx.upgrade() { + desktop_ctx.set_title(&title); + } } } @@ -34,9 +40,9 @@ pub(crate) struct DesktopEvaluator { impl DesktopEvaluator { /// Creates a new evaluator for desktop-based targets. - pub fn create(desktop_ctx: DesktopContext, js: String) -> GenerationalBox> { - let ctx = desktop_ctx.clone(); - let query = desktop_ctx.query.new_query(&js, ctx); + pub fn create(weak_desktop_ctx: WeakDesktopContext, js: String) -> GenerationalBox> { + let desktop_ctx = weak_desktop_ctx.upgrade().unwrap(); // todo: implement error or Default + let query = desktop_ctx.query.new_query(&js, weak_desktop_ctx); // We create a generational box that is owned by the query slot so that when we drop the query slot, the generational box is also dropped. let owner = UnsyncStorage::owner(); diff --git a/packages/desktop/src/element.rs b/packages/desktop/src/element.rs index 505b45c435..7b67d815b7 100644 --- a/packages/desktop/src/element.rs +++ b/packages/desktop/src/element.rs @@ -1,21 +1,22 @@ +use std::rc::Weak; use dioxus_core::ElementId; use dioxus_html::{ geometry::{PixelsRect, PixelsSize, PixelsVector2D}, MountedResult, RenderedElementBacking, }; -use crate::{desktop_context::DesktopContext, query::QueryEngine}; +use crate::{desktop_context::DesktopContext, query::QueryEngine, WeakDesktopContext}; #[derive(Clone)] /// A mounted element passed to onmounted events pub struct DesktopElement { id: ElementId, - webview: DesktopContext, + webview: WeakDesktopContext, query: QueryEngine, } impl DesktopElement { - pub(crate) fn new(id: ElementId, webview: DesktopContext, query: QueryEngine) -> Self { + pub(crate) fn new(id: ElementId, webview: WeakDesktopContext, query: QueryEngine) -> Self { Self { id, webview, query } } } diff --git a/packages/desktop/src/lib.rs b/packages/desktop/src/lib.rs index d71c3934eb..b4411d82c2 100644 --- a/packages/desktop/src/lib.rs +++ b/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, WeakDesktopContext, DesktopService}; pub use event_handlers::WryEventHandler; pub use hooks::*; pub use shortcut::{ShortcutHandle, ShortcutRegistryError}; diff --git a/packages/desktop/src/query.rs b/packages/desktop/src/query.rs index 15d6253804..c1c679ae6f 100644 --- a/packages/desktop/src/query.rs +++ b/packages/desktop/src/query.rs @@ -1,10 +1,11 @@ -use crate::DesktopContext; +use crate::{DesktopContext, WeakDesktopContext}; use futures_util::{FutureExt, StreamExt}; use generational_box::Owner; use serde::{de::DeserializeOwned, Deserialize}; use serde_json::Value; use slab::Slab; use std::{cell::RefCell, rc::Rc}; +use std::rc::Weak; use thiserror::Error; /// Tracks what query ids are currently active @@ -45,7 +46,7 @@ impl QueryEngine { pub fn new_query( &self, script: &str, - context: DesktopContext, + weak_context: WeakDesktopContext, ) -> Query { let (tx, rx) = futures_channel::mpsc::unbounded(); let (return_tx, return_rx) = futures_channel::oneshot::channel(); @@ -54,6 +55,7 @@ impl QueryEngine { return_sender: Some(return_tx), owner: None, }); + let context = weak_context.upgrade().unwrap(); // todo: implement error or Default // start the query // We embed the return of the eval in a function so we can send it back to the main thread @@ -107,7 +109,7 @@ impl QueryEngine { id: request_id, receiver: rx, return_receiver: Some(return_rx), - desktop: context, + desktop: weak_context, phantom: std::marker::PhantomData, } } @@ -140,7 +142,7 @@ impl QueryEngine { } pub(crate) struct Query { - desktop: DesktopContext, + desktop: WeakDesktopContext, receiver: futures_channel::mpsc::UnboundedReceiver, return_receiver: Option>>, pub id: usize, @@ -161,7 +163,7 @@ impl Query { let data = message.to_string(); let script = format!(r#"window.getQuery({queue_id}).rustSend({data});"#); - self.desktop + self.desktop.upgrade().unwrap() .webview .evaluate_script(&script) .map_err(|e| QueryError::Send(e.to_string()))?; diff --git a/packages/desktop/src/webview.rs b/packages/desktop/src/webview.rs index f155bb9021..7532041f8e 100644 --- a/packages/desktop/src/webview.rs +++ b/packages/desktop/src/webview.rs @@ -10,7 +10,7 @@ use crate::{ ipc::UserWindowEvent, protocol, waker::tao_waker, - Config, DesktopContext, DesktopService, + Config, DesktopContext, DesktopService, WeakDesktopContext }; use dioxus_core::{Runtime, ScopeId, VirtualDom}; use dioxus_document::Document; @@ -21,13 +21,14 @@ use futures_util::{pin_mut, FutureExt}; use std::cell::OnceCell; use std::sync::Arc; use std::{rc::Rc, task::Waker}; +use std::rc::Weak; use wry::{DragDropEvent, RequestAsyncResponder, WebContext, WebViewBuilder}; #[derive(Clone)] pub(crate) struct WebviewEdits { runtime: Rc, pub wry_queue: WryQueue, - desktop_context: Rc>, + desktop_context: Rc>, } impl WebviewEdits { @@ -39,7 +40,7 @@ impl WebviewEdits { } } - fn set_desktop_context(&self, context: DesktopContext) { + fn set_desktop_context(&self, context: WeakDesktopContext) { _ = self.desktop_context.set(context); } @@ -97,20 +98,24 @@ impl WebviewEdits { bubbles, data, } = event; - let Some(desktop_context) = self.desktop_context.get() else { + let Some(weak_desktop_context) = self.desktop_context.get() else { tracing::error!( "Tried to handle event before setting the desktop context on the event handler" ); return Default::default(); }; + let Some(desktop_context) = weak_desktop_context.upgrade() else { + return Default::default(); + }; + let query = desktop_context.query.clone(); let recent_file = desktop_context.file_hover.clone(); // check for a mounted event placeholder and replace it with a desktop specific element let as_any = match data { dioxus_html::EventData::Mounted => { - let element = DesktopElement::new(element, desktop_context.clone(), query.clone()); + let element = DesktopElement::new(element, weak_desktop_context.clone(), query.clone()); Rc::new(PlatformEventData::new(Box::new(element))) } dioxus_html::EventData::Drag(ref drag) => { @@ -400,16 +405,23 @@ impl WebviewInstance { file_hover, )); + + println!("Create handler strong count in webview::new before edits and dom runtime: {}", Rc::strong_count(&desktop_context)); + + let weak_desktop: WeakDesktopContext = Rc::downgrade(&desktop_context); + // Provide the desktop context to the virtual dom and edit handler - edits.set_desktop_context(desktop_context.clone()); - let provider: Rc = Rc::new(DesktopDocument::new(desktop_context.clone())); + edits.set_desktop_context(weak_desktop.clone()); + let provider: Rc = Rc::new(DesktopDocument::new(weak_desktop.clone())); let history_provider: Rc = Rc::new(MemoryHistory::default()); dom.in_runtime(|| { - ScopeId::ROOT.provide_context(desktop_context.clone()); + ScopeId::ROOT.provide_context(weak_desktop.clone()); ScopeId::ROOT.provide_context(provider); ScopeId::ROOT.provide_context(history_provider); }); + println!("Create handler strong count in webview::new after: {}", Rc::strong_count(&desktop_context)); + WebviewInstance { dom, edits, From 5a8646b59697b2d12a2ed7b4079a5b06924b9f4a Mon Sep 17 00:00:00 2001 From: sertschgi Date: Fri, 3 Jan 2025 16:35:30 +0100 Subject: [PATCH 2/4] fixed window when multiple windows not closing --- packages/desktop/src/app.rs | 12 ++++++++---- packages/desktop/src/webview.rs | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/desktop/src/app.rs b/packages/desktop/src/app.rs index cd1841c390..6f2f283246 100644 --- a/packages/desktop/src/app.rs +++ b/packages/desktop/src/app.rs @@ -173,7 +173,7 @@ impl App { pub fn handle_new_window(&mut self) { for handler in self.shared.pending_webviews.borrow_mut().drain(..) { - println!("Create handler strong count: {}", Rc::strong_count(&handler.desktop_context)); + // println!("Create handler strong count: {}", Rc::strong_count(&handler.desktop_context)); let id = handler.desktop_context.window.id(); self.webviews.insert(id, handler); _ = self.shared.proxy.send_event(UserWindowEvent::Poll(id)); @@ -197,11 +197,15 @@ impl App { // Rc::decrement_strong_count(raw); // Rc::decrement_strong_count(raw); // } + // self.webviews.iter().for_each(|wv| { + // wv.1.desktop_context.webview. + // // _ = self.shared.proxy.send_event(UserWindowEvent::Poll(wv.0.clone())); + // }); } - if self.webviews.is_empty() { - self.control_flow = ControlFlow::Exit - } + // if self.webviews.is_empty() { + // self.control_flow = ControlFlow::Exit + // } } LastWindowHides => { diff --git a/packages/desktop/src/webview.rs b/packages/desktop/src/webview.rs index 7532041f8e..3f25581fba 100644 --- a/packages/desktop/src/webview.rs +++ b/packages/desktop/src/webview.rs @@ -406,7 +406,7 @@ impl WebviewInstance { )); - println!("Create handler strong count in webview::new before edits and dom runtime: {}", Rc::strong_count(&desktop_context)); + // println!("Create handler strong count in webview::new before edits and dom runtime: {}", Rc::strong_count(&desktop_context)); let weak_desktop: WeakDesktopContext = Rc::downgrade(&desktop_context); @@ -420,7 +420,7 @@ impl WebviewInstance { ScopeId::ROOT.provide_context(history_provider); }); - println!("Create handler strong count in webview::new after: {}", Rc::strong_count(&desktop_context)); + // println!("Create handler strong count in webview::new after: {}", Rc::strong_count(&desktop_context)); WebviewInstance { dom, From 8970be15161acf776efe4e85f3a3313da0e87ae2 Mon Sep 17 00:00:00 2001 From: sertschgi Date: Fri, 3 Jan 2025 16:44:09 +0100 Subject: [PATCH 3/4] fixed accidental includes --- packages/desktop/src/app.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/desktop/src/app.rs b/packages/desktop/src/app.rs index 6f2f283246..38b0e727c6 100644 --- a/packages/desktop/src/app.rs +++ b/packages/desktop/src/app.rs @@ -14,9 +14,7 @@ use tao::{ event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget}, window::WindowId, }; -use tao::platform::unix::{EventLoopWindowTargetExtUnix, WindowExtUnix}; use tracing::instrument::WithSubscriber; -use wry::WebViewExtUnix; /// The single top-level object that manages all the running windows, assets, shortcuts, etc pub(crate) struct App { From d823ec214aeedd00f90c9e2557825a6804bc10f3 Mon Sep 17 00:00:00 2001 From: sertschgi Date: Sat, 4 Jan 2025 18:49:00 +0100 Subject: [PATCH 4/4] polished --- packages/desktop/src/app.rs | 28 +++++++------------------ packages/desktop/src/desktop_context.rs | 3 --- packages/desktop/src/webview.rs | 5 ----- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/packages/desktop/src/app.rs b/packages/desktop/src/app.rs index 38b0e727c6..9acd051ab0 100644 --- a/packages/desktop/src/app.rs +++ b/packages/desktop/src/app.rs @@ -171,7 +171,6 @@ impl App { pub fn handle_new_window(&mut self) { for handler in self.shared.pending_webviews.borrow_mut().drain(..) { - // println!("Create handler strong count: {}", Rc::strong_count(&handler.desktop_context)); let id = handler.desktop_context.window.id(); self.webviews.insert(id, handler); _ = self.shared.proxy.send_event(UserWindowEvent::Poll(id)); @@ -183,27 +182,14 @@ impl App { match self.window_behavior { LastWindowExitsApp => { - // #[cfg(debug_assertions)] - // self.persist_window_state(); - if let Some(webview) = self.webviews.remove(&id) - { - // println!("Exit strong count: {}", Rc::strong_count(&webview.desktop_context)); - // let mut raw = Rc::into_raw(webview.desktop_context); - // unsafe { - // Rc::decrement_strong_count(raw); - // Rc::decrement_strong_count(raw); - // Rc::decrement_strong_count(raw); - // Rc::decrement_strong_count(raw); - // } - // self.webviews.iter().for_each(|wv| { - // wv.1.desktop_context.webview. - // // _ = self.shared.proxy.send_event(UserWindowEvent::Poll(wv.0.clone())); - // }); - } + #[cfg(debug_assertions)] + self.persist_window_state(); + + self.webviews.remove(&id); - // if self.webviews.is_empty() { - // self.control_flow = ControlFlow::Exit - // } + if self.webviews.is_empty() { + self.control_flow = ControlFlow::Exit + } } LastWindowHides => { diff --git a/packages/desktop/src/desktop_context.rs b/packages/desktop/src/desktop_context.rs index a3d111713b..c6b042357f 100644 --- a/packages/desktop/src/desktop_context.rs +++ b/packages/desktop/src/desktop_context.rs @@ -121,9 +121,6 @@ impl DesktopService { .send_event(UserWindowEvent::NewWindow) .unwrap(); - - // println!("Create handler strong count in new_window: {}", Rc::strong_count(&window.desktop_context)); - self.shared.pending_webviews.borrow_mut().push(window); cx diff --git a/packages/desktop/src/webview.rs b/packages/desktop/src/webview.rs index 3f25581fba..41d808715c 100644 --- a/packages/desktop/src/webview.rs +++ b/packages/desktop/src/webview.rs @@ -405,9 +405,6 @@ impl WebviewInstance { file_hover, )); - - // println!("Create handler strong count in webview::new before edits and dom runtime: {}", Rc::strong_count(&desktop_context)); - let weak_desktop: WeakDesktopContext = Rc::downgrade(&desktop_context); // Provide the desktop context to the virtual dom and edit handler @@ -420,8 +417,6 @@ impl WebviewInstance { ScopeId::ROOT.provide_context(history_provider); }); - // println!("Create handler strong count in webview::new after: {}", Rc::strong_count(&desktop_context)); - WebviewInstance { dom, edits,