Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions packages/desktop/src/app.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -22,6 +14,7 @@ use tao::{
event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget},
window::WindowId,
};
use tracing::instrument::WithSubscriber;

/// The single top-level object that manages all the running windows, assets, shortcuts, etc
pub(crate) struct App {
Expand Down Expand Up @@ -193,6 +186,7 @@ impl App {
self.persist_window_state();

self.webviews.remove(&id);

if self.webviews.is_empty() {
self.control_flow = ControlFlow::Exit
}
Expand Down
20 changes: 13 additions & 7 deletions packages/desktop/src/desktop_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<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.
Expand Down Expand Up @@ -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<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(|| {
ScopeId::ROOT
.consume_context::<Rc<DesktopService>>()
.consume_context()
.unwrap()
});

Expand All @@ -117,7 +123,7 @@ impl DesktopService {

self.shared.pending_webviews.borrow_mut().push(window);

Rc::downgrade(&cx)
cx
}

/// trigger the drag-window event
Expand All @@ -142,9 +148,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
Expand Down
20 changes: 13 additions & 7 deletions packages/desktop/src/document.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
}
Expand All @@ -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);
}
}
}

Expand All @@ -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<Box<dyn Evaluator>> {
let ctx = desktop_ctx.clone();
let query = desktop_ctx.query.new_query(&js, ctx);
pub fn create(weak_desktop_ctx: WeakDesktopContext, js: String) -> GenerationalBox<Box<dyn Evaluator>> {
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();
Expand Down
7 changes: 4 additions & 3 deletions packages/desktop/src/element.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
12 changes: 7 additions & 5 deletions packages/desktop/src/query.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -45,7 +46,7 @@ impl QueryEngine {
pub fn new_query<V: DeserializeOwned>(
&self,
script: &str,
context: DesktopContext,
weak_context: WeakDesktopContext,
) -> Query<V> {
let (tx, rx) = futures_channel::mpsc::unbounded();
let (return_tx, return_rx) = futures_channel::oneshot::channel();
Expand All @@ -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
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -140,7 +142,7 @@ impl QueryEngine {
}

pub(crate) struct Query<V: DeserializeOwned> {
desktop: DesktopContext,
desktop: WeakDesktopContext,
receiver: futures_channel::mpsc::UnboundedReceiver<Value>,
return_receiver: Option<futures_channel::oneshot::Receiver<Result<Value, String>>>,
pub id: usize,
Expand All @@ -161,7 +163,7 @@ impl<V: DeserializeOwned> Query<V> {
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()))?;
Expand Down
23 changes: 15 additions & 8 deletions packages/desktop/src/webview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Runtime>,
pub wry_queue: WryQueue,
desktop_context: Rc<OnceCell<DesktopContext>>,
desktop_context: Rc<OnceCell<WeakDesktopContext>>,
}

impl WebviewEdits {
Expand All @@ -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);
}

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -400,12 +405,14 @@ impl WebviewInstance {
file_hover,
));

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<dyn Document> = Rc::new(DesktopDocument::new(desktop_context.clone()));
edits.set_desktop_context(weak_desktop.clone());
let provider: Rc<dyn Document> = Rc::new(DesktopDocument::new(weak_desktop.clone()));
let history_provider: Rc<dyn History> = 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);
});
Expand Down