From 4dad65667f9b9a4c775524600684768502dce959 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 09:32:41 -0800 Subject: [PATCH] Save As: on Linux a portal that exits during the save dialog no longer leaves the save waiting When the desktop portal's process exited while its save dialog was open, no Response came. The stream that save_file read does not end while the session bus stays up, so the save waited forever on the calling thread and rfd's dialog never showed. save_file now waits with the screen color sampler's Responses, which moves from linux_screen_color/request.rs to portal_request.rs and is built on Linux and the BSDs. Before the call it listens for the Response and for the bus's NameOwnerChanged for the portal's name, so the wait ends with an error when the connection that replied leaves the bus, and the caller shows rfd's dialog, as for other portal errors. When the bus reports the portal gone, the wait first reads the Responses already queued, so an answer the portal sent before it left is still the answer. Before, the sampler's wait could take the bus's report while that answer was queued and end with an error. The mock portal gains three modes that close its connection after the reply: 300 ms later without a Response, at once without a Response, and at once after the Response. The private bus helper of the sampler's tests moves to portal_request.rs, and the save dialog's tests use it, so a failing test also stops its bus. --- apps/vectorcraft/Cargo.toml | 7 +- apps/vectorcraft/src/linux_screen_color.rs | 1 - .../src/linux_screen_color/portal.rs | 34 +-- .../src/linux_screen_color/request.rs | 127 ---------- apps/vectorcraft/src/main.rs | 2 + apps/vectorcraft/src/portal_request.rs | 231 ++++++++++++++++++ apps/vectorcraft/src/portal_save.rs | 101 ++++---- 7 files changed, 296 insertions(+), 207 deletions(-) delete mode 100644 apps/vectorcraft/src/linux_screen_color/request.rs create mode 100644 apps/vectorcraft/src/portal_request.rs diff --git a/apps/vectorcraft/Cargo.toml b/apps/vectorcraft/Cargo.toml index 55b9e344e7..1fab9ac36a 100644 --- a/apps/vectorcraft/Cargo.toml +++ b/apps/vectorcraft/Cargo.toml @@ -107,15 +107,16 @@ wayland-protocols = { version = "0.32", features = ["client", "unstable"] } # wayland-protocols' `unstable` set) made on the window's own connection. MIT, already in the build # through winit and rfd. wayland-backend = { version = "0.3", features = ["client_system"] } -# Waits for the screen color sampler's portal answer and for the portal leaving the bus at the same -# time (src/linux_screen_color/request.rs). Apache-2.0 OR MIT, already in the build through zbus. -futures-lite = "2" # The save dialog's file type (src/portal_save.rs, #2679): the desktop portal over D-Bus, on the # systems where rfd shows the portal's dialog. MIT, already in the build through accesskit. Pinned # exactly: review the diff before bumping. [target.'cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))'.dependencies] zbus = "=5.19.0" +# Waits for a portal answer (the save dialog's and the screen color sampler's) and for the portal +# leaving the bus at the same time (src/portal_request.rs). Apache-2.0 OR MIT, already in the build +# through zbus. +futures-lite = "2" # Tells whether arboard reads through the compositor's data-control protocol or through X11, the # same check arboard makes (src/clipboard.rs). MIT OR Apache-2.0, already in the build through # arboard. diff --git a/apps/vectorcraft/src/linux_screen_color.rs b/apps/vectorcraft/src/linux_screen_color.rs index 0f46af19d8..cfb8d7d69f 100644 --- a/apps/vectorcraft/src/linux_screen_color.rs +++ b/apps/vectorcraft/src/linux_screen_color.rs @@ -4,7 +4,6 @@ //! parent is the window, named on that thread as for the portal's save dialog (`portal_save`). mod portal; -mod request; /// The portal's color sampler over the window `parent` holds; `ui` is woken when it answers. pub fn sampler(ui: egui::Context, parent: super::host_services::Parent) -> vectorcraft_ui_egui::screen_color::StartSampler { diff --git a/apps/vectorcraft/src/linux_screen_color/portal.rs b/apps/vectorcraft/src/linux_screen_color/portal.rs index b7518e1ce3..21bcb34da5 100644 --- a/apps/vectorcraft/src/linux_screen_color/portal.rs +++ b/apps/vectorcraft/src/linux_screen_color/portal.rs @@ -61,9 +61,9 @@ pub(super) fn connect(builder: zbus::Result Result, Failure> { const DESKTOP: &str = "org.freedesktop.portal.Desktop"; - let token = super::request::token("vectorcraft_color").map_err(Failure::Failed)?; + let token = crate::portal_request::token("vectorcraft_color").map_err(Failure::Failed)?; // Made before the call: a portal can send its answer before the call returns. - let replies = super::request::Responses::new(conn).map_err(Failure::Failed)?; + let replies = crate::portal_request::Responses::new(conn).map_err(Failure::Failed)?; let proxy = Proxy::new(conn, DESKTOP, "/org/freedesktop/portal/desktop", "org.freedesktop.portal.Screenshot") .map_err(|e| Failure::Failed(e.to_string()))?; let options = HashMap::from([("handle_token", Value::from(token.as_str()))]); @@ -106,6 +106,7 @@ fn answer(code: u32, results: &HashMap) -> Result HashMap { HashMap::from([("color".into(), OwnedValue::try_from(Value::from(rgb)).unwrap())]) @@ -186,35 +187,6 @@ mod tests { } } - /// A session bus of its own (`dbus-daemon`), `None` when there is none to run. - fn private_bus() -> Option<(Daemon, String)> { - use std::io::BufRead as _; - let spawning = crate::spawning(); - let daemon = std::process::Command::new("dbus-daemon") - .args(["--session", "--nofork", "--print-address=1"]) - .stdout(std::process::Stdio::piped()) - .spawn(); - drop(spawning); - let Ok(daemon) = daemon else { - eprintln!("no dbus-daemon: skipped"); - return None; - }; - let mut daemon = Daemon(daemon); - let mut address = String::new(); - std::io::BufReader::new(daemon.0.stdout.take().unwrap()).read_line(&mut address).unwrap(); - Some((daemon, address.trim().to_string())) - } - - /// Stops the private bus when the test ends, unwinding included. - struct Daemon(std::process::Child); - - impl Drop for Daemon { - fn drop(&mut self) { - let _ = self.0.kill(); - let _ = self.0.wait(); - } - } - /// The client's connection to the private bus at `address`, made as the sampler makes its own. fn client(address: &str) -> Connection { connect(zbus::blocking::connection::Builder::address(address)).unwrap() diff --git a/apps/vectorcraft/src/linux_screen_color/request.rs b/apps/vectorcraft/src/linux_screen_color/request.rs deleted file mode 100644 index f442b840a3..0000000000 --- a/apps/vectorcraft/src/linux_screen_color/request.rs +++ /dev/null @@ -1,127 +0,0 @@ -//! The answer to the color sampler's XDG desktop portal request -//! (). -//! The wait ends with the request's `Response`, or with an error when the portal leaves the bus -//! first. - -use std::io::Read as _; - -use futures_lite::{StreamExt as _, future}; -use zbus::blocking::{Connection, MessageIterator}; -use zbus::zvariant::OwnedObjectPath; - -const DESKTOP: &str = "org.freedesktop.portal.Desktop"; - -/// A request token: `prefix`, then 128 bits from the operating system's random device, so the -/// token is unique and can't be guessed, as the Request documentation asks. -pub(super) fn token(prefix: &str) -> Result { - let mut bytes = [0; 16]; - std::fs::File::open("/dev/urandom") - .and_then(|mut file| file.read_exact(&mut bytes)) - .map_err(|e| format!("could not create a portal request token: {e}"))?; - let random: String = bytes.iter().map(|byte| format!("{byte:02x}")).collect(); - Ok(format!("{prefix}_{random}")) -} - -/// Subscriptions made before a portal method call, on every request path: a portal older than -/// version 0.9 ignores `handle_token` and answers on the request path it returns, and its answer -/// can come before the call's reply. Each stream keeps every signal from the time it is made. -pub(super) struct Responses { - replies: MessageIterator, - owners: MessageIterator, -} - -impl Responses { - pub(super) fn new(conn: &Connection) -> Result { - let reply_rule = zbus::MatchRule::builder() - .msg_type(zbus::message::Type::Signal) - .sender(DESKTOP) - .and_then(|b| b.interface("org.freedesktop.portal.Request")) - .and_then(|b| b.member("Response")) - .map_err(|e| e.to_string())? - .build(); - let owner_rule = zbus::MatchRule::builder() - .msg_type(zbus::message::Type::Signal) - .sender("org.freedesktop.DBus") - .and_then(|b| b.path("/org/freedesktop/DBus")) - .and_then(|b| b.interface("org.freedesktop.DBus")) - .and_then(|b| b.member("NameOwnerChanged")) - .and_then(|b| b.arg(0, DESKTOP)) - .map_err(|e| e.to_string())? - .build(); - Ok(Self { - replies: MessageIterator::for_match_rule(reply_rule, conn, None).map_err(|e| e.to_string())?, - owners: MessageIterator::for_match_rule(owner_rule, conn, None).map_err(|e| e.to_string())?, - }) - } - - /// The `Response` on the request path `call` returned, from the connection that returned it. - /// An error when that connection leaves the bus first, or the bus connection closes. - pub(super) fn wait(self, call: &zbus::Message) -> Result { - enum Event { - Reply(Option>), - Owner(Option>), - } - let handle: OwnedObjectPath = call.body().deserialize().map_err(|e| e.to_string())?; - let portal = call.header().sender().map(|s| s.as_str().to_string()).ok_or("the portal answered without a sender")?; - let mut replies = self.replies.into_inner(); - let mut owners = self.owners.into_inner(); - future::block_on(async { - loop { - let event = future::or(async { Event::Reply(replies.next().await) }, async { Event::Owner(owners.next().await) }).await; - match event { - Event::Reply(reply) => { - let reply = reply.ok_or("the portal closed without an answer")?.map_err(|e| e.to_string())?; - let header = reply.header(); - if header.path().map(|p| p.as_str()) == Some(handle.as_str()) && header.sender().map(|s| s.as_str()) == Some(portal.as_str()) - { - return Ok(reply); - } - } - Event::Owner(message) => { - let message = message.ok_or("the portal connection closed")?.map_err(|e| e.to_string())?; - if owner_disappeared(&message, &portal)? { - return Err("the desktop portal disappeared while waiting for its answer".into()); - } - } - } - } - }) - } -} - -/// Whether `message` is the bus's `NameOwnerChanged` saying that the connection `portal` no longer -/// owns the portal's name. The bus itself must send it: another connection can't end the wait. -fn owner_disappeared(message: &zbus::Message, portal: &str) -> Result { - if message.header().sender().map(|s| s.as_str()) != Some("org.freedesktop.DBus") { - return Ok(false); - } - let (name, old, new): (String, String, String) = message.body().deserialize().map_err(|e| e.to_string())?; - Ok(name == DESKTOP && old == portal && new != portal) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn only_the_bus_can_report_portal_owner_loss() { - let changed = |sender, name, old, new| { - zbus::Message::signal("/org/freedesktop/DBus", "org.freedesktop.DBus", "NameOwnerChanged") - .unwrap() - .sender(sender) - .unwrap() - .build(&(name, old, new)) - .unwrap() - }; - let forged = changed(":1.42", DESKTOP, ":1.7", ""); - assert!(!owner_disappeared(&forged, ":1.7").unwrap()); - let real = changed("org.freedesktop.DBus", DESKTOP, ":1.7", ""); - assert!(owner_disappeared(&real, ":1.7").unwrap()); - let other = changed("org.freedesktop.DBus", "org.example.Other", ":1.7", ""); - assert!(!owner_disappeared(&other, ":1.7").unwrap()); - let old = changed("org.freedesktop.DBus", DESKTOP, ":1.8", ""); - assert!(!owner_disappeared(&old, ":1.7").unwrap()); - let acquired = changed("org.freedesktop.DBus", DESKTOP, "", ":1.7"); - assert!(!owner_disappeared(&acquired, ":1.7").unwrap()); - } -} diff --git a/apps/vectorcraft/src/main.rs b/apps/vectorcraft/src/main.rs index c2b40e37e6..d91762dce0 100644 --- a/apps/vectorcraft/src/main.rs +++ b/apps/vectorcraft/src/main.rs @@ -53,6 +53,8 @@ mod mac_tablet; mod mac_window; #[cfg(target_os = "macos")] mod open_documents; +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] +mod portal_request; #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd", test))] mod portal_save; mod prefs_dir; diff --git a/apps/vectorcraft/src/portal_request.rs b/apps/vectorcraft/src/portal_request.rs new file mode 100644 index 0000000000..da4cb9ad23 --- /dev/null +++ b/apps/vectorcraft/src/portal_request.rs @@ -0,0 +1,231 @@ +//! Linux and the BSDs: the answer to an XDG desktop portal request +//! (), +//! for the save dialog (`portal_save`) and, on Linux, the screen color sampler +//! (`linux_screen_color`). The wait ends with the request's `Response`, or with an error when the +//! portal leaves the bus first. + +use futures_lite::{StreamExt as _, future}; +use zbus::blocking::{Connection, MessageIterator}; +use zbus::zvariant::OwnedObjectPath; + +const DESKTOP: &str = "org.freedesktop.portal.Desktop"; + +/// A request token: `prefix`, then 128 bits from the operating system's random device, so the +/// token is unique and can't be guessed, as the Request documentation asks. +#[cfg(target_os = "linux")] +pub(crate) fn token(prefix: &str) -> Result { + use std::io::Read as _; + + let mut bytes = [0; 16]; + std::fs::File::open("/dev/urandom") + .and_then(|mut file| file.read_exact(&mut bytes)) + .map_err(|e| format!("could not create a portal request token: {e}"))?; + let random: String = bytes.iter().map(|byte| format!("{byte:02x}")).collect(); + Ok(format!("{prefix}_{random}")) +} + +/// Subscriptions made before a portal method call, on every request path: a portal older than +/// version 0.9 ignores `handle_token` and answers on the request path it returns, and its answer +/// can come before the call's reply. Each stream keeps every signal from the time it is made. +pub(crate) struct Responses { + replies: MessageIterator, + owners: MessageIterator, +} + +impl Responses { + pub(crate) fn new(conn: &Connection) -> Result { + let reply_rule = zbus::MatchRule::builder() + .msg_type(zbus::message::Type::Signal) + .sender(DESKTOP) + .and_then(|b| b.interface("org.freedesktop.portal.Request")) + .and_then(|b| b.member("Response")) + .map_err(|e| e.to_string())? + .build(); + let owner_rule = zbus::MatchRule::builder() + .msg_type(zbus::message::Type::Signal) + .sender("org.freedesktop.DBus") + .and_then(|b| b.path("/org/freedesktop/DBus")) + .and_then(|b| b.interface("org.freedesktop.DBus")) + .and_then(|b| b.member("NameOwnerChanged")) + .and_then(|b| b.arg(0, DESKTOP)) + .map_err(|e| e.to_string())? + .build(); + Ok(Self { + replies: MessageIterator::for_match_rule(reply_rule, conn, None).map_err(|e| e.to_string())?, + owners: MessageIterator::for_match_rule(owner_rule, conn, None).map_err(|e| e.to_string())?, + }) + } + + /// The `Response` on the request path `call` returned, from the connection that returned it. + /// An error when that connection leaves the bus first, or the bus connection closes. A + /// `Response` the portal sent before it left is still the answer. + pub(crate) fn wait(self, call: &zbus::Message) -> Result { + enum Event { + Reply(Option>), + Owner(Option>), + } + let handle: OwnedObjectPath = call.body().deserialize().map_err(|e| e.to_string())?; + let portal = call.header().sender().map(|s| s.as_str().to_string()).ok_or("the portal answered without a sender")?; + let answers = |reply: &zbus::Message| { + let header = reply.header(); + header.path().map(|p| p.as_str()) == Some(handle.as_str()) && header.sender().map(|s| s.as_str()) == Some(portal.as_str()) + }; + let mut replies = self.replies.into_inner(); + let mut owners = self.owners.into_inner(); + future::block_on(async { + loop { + let event = future::or(async { Event::Owner(owners.next().await) }, async { Event::Reply(replies.next().await) }).await; + match event { + Event::Reply(reply) => { + let reply = reply.ok_or("the portal closed without an answer")?.map_err(|e| e.to_string())?; + if answers(&reply) { + return Ok(reply); + } + } + Event::Owner(message) => { + let message = message.ok_or("the portal connection closed")?.map_err(|e| e.to_string())?; + if owner_disappeared(&message, &portal)? { + // zbus puts each message in every matching stream before it reads the + // next one, so a `Response` that came before the bus's report is + // already queued: it is still the answer. + while let Some(Some(reply)) = future::poll_once(replies.next()).await { + if let Ok(reply) = reply + && answers(&reply) + { + return Ok(reply); + } + } + return Err("the desktop portal disappeared while waiting for its answer".into()); + } + } + } + } + }) + } +} + +/// Whether `message` is the bus's `NameOwnerChanged` saying that the connection `portal` no longer +/// owns the portal's name. The bus itself must send it: another connection can't end the wait. +fn owner_disappeared(message: &zbus::Message, portal: &str) -> Result { + if message.header().sender().map(|s| s.as_str()) != Some("org.freedesktop.DBus") { + return Ok(false); + } + let (name, old, new): (String, String, String) = message.body().deserialize().map_err(|e| e.to_string())?; + Ok(name == DESKTOP && old == portal && new != portal) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_the_bus_can_report_portal_owner_loss() { + let changed = |sender, name, old, new| { + zbus::Message::signal("/org/freedesktop/DBus", "org.freedesktop.DBus", "NameOwnerChanged") + .unwrap() + .sender(sender) + .unwrap() + .build(&(name, old, new)) + .unwrap() + }; + let forged = changed(":1.42", DESKTOP, ":1.7", ""); + assert!(!owner_disappeared(&forged, ":1.7").unwrap()); + let real = changed("org.freedesktop.DBus", DESKTOP, ":1.7", ""); + assert!(owner_disappeared(&real, ":1.7").unwrap()); + let other = changed("org.freedesktop.DBus", "org.example.Other", ":1.7", ""); + assert!(!owner_disappeared(&other, ":1.7").unwrap()); + let old = changed("org.freedesktop.DBus", DESKTOP, ":1.8", ""); + assert!(!owner_disappeared(&old, ":1.7").unwrap()); + let acquired = changed("org.freedesktop.DBus", DESKTOP, "", ":1.7"); + assert!(!owner_disappeared(&acquired, ":1.7").unwrap()); + } + + /// Through a real D-Bus: when the bus has reported the portal gone, a `Response` the portal sent + /// before it left is still the answer; without one the wait ends with an error. + #[test] + fn a_queued_answer_wins_over_the_portal_leaving() { + use std::collections::HashMap; + + use zbus::blocking::connection::Builder; + use zbus::zvariant::OwnedValue; + + for answered in [true, false] { + let Some((_daemon, address)) = test_bus::private_bus() else { return }; + let address = address.as_str(); + let portal = Builder::address(address).unwrap().name(DESKTOP).unwrap().build().unwrap(); + let unique = portal.unique_name().unwrap().to_string(); + let client = Builder::address(address).unwrap().build().unwrap(); + let responses = Responses::new(&client).unwrap(); + // zbus puts each message in every matching stream before it reads the next one + // (`src/connection/socket_reader.rs`), so once this stream has the bus's report, + // `responses` holds the `Response` sent before it. + let gone_rule = zbus::MatchRule::builder() + .msg_type(zbus::message::Type::Signal) + .sender("org.freedesktop.DBus") + .unwrap() + .member("NameOwnerChanged") + .unwrap() + .arg(0, DESKTOP) + .unwrap() + .build(); + let gone = MessageIterator::for_match_rule(gone_rule, &client, None).unwrap(); + let path = "/org/freedesktop/portal/desktop/request/test/t1"; + if answered { + let results = HashMap::::new(); + portal.emit_signal(None::<&str>, path, "org.freedesktop.portal.Request", "Response", &(0_u32, results)).unwrap(); + } + portal.close().unwrap(); + // On a thread of its own, so a report that never comes fails the test instead of + // hanging it. + let (seen, report) = std::sync::mpsc::channel(); + let left = unique.clone(); + std::thread::spawn(move || { + let mut gone = gone; + let _ = seen.send(gone.any(|m| m.is_ok_and(|m| owner_disappeared(&m, &left).unwrap_or(false)))); + }); + assert_eq!(report.recv_timeout(std::time::Duration::from_secs(30)), Ok(true), "the bus did not report the portal gone"); + // The call's reply as `wait` reads it: the request path, from the portal's connection. + let call = zbus::Message::signal("/", "org.example.Test", "Reply") + .unwrap() + .sender(unique.as_str()) + .unwrap() + .build(&OwnedObjectPath::try_from(path).unwrap()) + .unwrap(); + let waited = responses.wait(&call); + assert_eq!(waited.is_ok(), answered, "{waited:?}"); + } + } +} + +/// The private session bus that the D-Bus tests of the portal requests run on. +#[cfg(test)] +pub(crate) mod test_bus { + /// A session bus of its own (`dbus-daemon`), `None` when there is none to run. + pub(crate) fn private_bus() -> Option<(Daemon, String)> { + use std::io::BufRead as _; + let spawning = crate::spawning(); + let daemon = std::process::Command::new("dbus-daemon") + .args(["--session", "--nofork", "--print-address=1"]) + .stdout(std::process::Stdio::piped()) + .spawn(); + drop(spawning); + let Ok(daemon) = daemon else { + eprintln!("no dbus-daemon: skipped"); + return None; + }; + let mut daemon = Daemon(daemon); + let mut address = String::new(); + std::io::BufReader::new(daemon.0.stdout.take().unwrap()).read_line(&mut address).unwrap(); + Some((daemon, address.trim().to_string())) + } + + /// Stops the private bus when the test ends, unwinding included. + pub(crate) struct Daemon(std::process::Child); + + impl Drop for Daemon { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } +} diff --git a/apps/vectorcraft/src/portal_save.rs b/apps/vectorcraft/src/portal_save.rs index a9bf5d1009..6d1a8734b2 100644 --- a/apps/vectorcraft/src/portal_save.rs +++ b/apps/vectorcraft/src/portal_save.rs @@ -8,6 +8,7 @@ //! ([`vectorcraft_ui_egui::picks::PickAnswer`]); the app gives the name that type's extension. //! rfd shows the dialog when the portal can't (no session bus, no file chooser), with zenity as its //! own fallback. PhotoCraft's `portal_save.rs` (storytold/photocraft#6151) makes the same call. +//! rfd also shows the dialog when the portal leaves the bus before it answers. //! //! The dialog's parent is the window: `x11:` on X11, and on Wayland (Linux) a handle //! from the compositor's xdg-foreign protocol, as rfd names it (its `window_identifier`). @@ -183,37 +184,18 @@ pub fn pick(pick: &FilePick, window: Option<&winit::window::Window>) -> Option Result, String> { use std::sync::atomic::{AtomicU32, Ordering}; - use zbus::blocking::{MessageIterator, Proxy}; - use zbus::zvariant::OwnedObjectPath; + use zbus::blocking::Proxy; const DESKTOP: &str = "org.freedesktop.portal.Desktop"; - const REQUEST: &str = "org.freedesktop.portal.Request"; static NEXT: AtomicU32 = AtomicU32::new(0); let token = format!("vectorcraft_save_{}_{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed)); - // Listen before asking, on every request path: a portal older than version 0.9 ignores - // `handle_token` and answers on the request path it returns, and its answer can come before - // the call's reply. The stream keeps every `Response` from the time it is made. - let rule = zbus::MatchRule::builder() - .msg_type(zbus::message::Type::Signal) - .sender(DESKTOP) - .and_then(|b| b.interface(REQUEST)) - .and_then(|b| b.member("Response")) - .map_err(|e| e.to_string())? - .build(); - let responses = MessageIterator::for_match_rule(rule, conn, None).map_err(|e| e.to_string())?; + // Made before the call: a portal can send its answer before the call returns, and a portal + // that leaves the bus while its dialog is open ends the wait. + let responses = crate::portal_request::Responses::new(conn)?; let chooser = Proxy::new(conn, DESKTOP, "/org/freedesktop/portal/desktop", "org.freedesktop.portal.FileChooser").map_err(|e| e.to_string())?; // The title is empty, as rfd sends it for a save. let call = chooser.call_method("SaveFile", &(request.parent.as_str(), "", options(request, &token))).map_err(|e| e.to_string())?; - let handle: OwnedObjectPath = call.body().deserialize().map_err(|e| e.to_string())?; - // The answer is the `Response` on the path returned, from the connection that returned it. - let portal = call.header().sender().map(|s| s.as_str().to_string()); - let reply = responses - .filter_map(Result::ok) - .find(|m| { - let header = m.header(); - header.path().map(|p| p.as_str()) == Some(handle.as_str()) && header.sender().map(|s| s.as_str()) == portal.as_deref() - }) - .ok_or("the save dialog closed without an answer")?; + let reply = responses.wait(&call)?; let (code, results): (u32, HashMap) = reply.body().deserialize().map_err(|e| e.to_string())?; answer(code, &results) } @@ -495,26 +477,22 @@ mod tests { Old, /// As `Old`, with the `Response` sent before the reply that names its path. OldEarly, + /// As `Current`, but it leaves the bus 300 ms after its reply without a `Response`, as a + /// portal that exits while its dialog is open. + Exits, + /// Leaves the bus right after its reply, without a `Response`. + ExitsAtOnce, + /// Sends its `Response` right after its reply, then leaves the bus. + AnswersAndExits, } /// A session bus of its own (`dbus-daemon`) with a mock `org.freedesktop.portal.FileChooser` /// that answers `SaveFile` from `script` as `portal` says, as PhotoCraft's portal tests run it. /// `None` when there is no `dbus-daemon` to run. #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] - fn mock_portal(script: Script, portal: Portal) -> Option<(zbus::blocking::Connection, Seen, std::process::Child)> { - use std::io::BufRead as _; - + fn mock_portal(script: Script, portal: Portal) -> Option<(zbus::blocking::Connection, Seen, crate::portal_request::test_bus::Daemon)> { use zbus::zvariant::OwnedObjectPath; - let spawning = crate::spawning(); - let daemon = std::process::Command::new("dbus-daemon") - .args(["--session", "--nofork", "--print-address=1"]) - .stdout(std::process::Stdio::piped()) - .spawn(); - drop(spawning); - let mut daemon = daemon.ok()?; - let mut address = String::new(); - std::io::BufReader::new(daemon.stdout.take()?).read_line(&mut address).ok()?; - let address = address.trim().to_string(); + let (daemon, address) = crate::portal_request::test_bus::private_bus()?; let server = zbus::blocking::connection::Builder::address(address.as_str()).ok()?.name("org.freedesktop.portal.Desktop").ok()?.build().ok()?; let seen = Seen::default(); @@ -539,8 +517,8 @@ mod tests { }); let Some(sender) = header.sender().map(|s| s.as_str().trim_start_matches(':').replace('.', "_")) else { break }; let token = match portal { - Portal::Current => text("handle_token").unwrap_or_default(), Portal::Old | Portal::OldEarly => format!("old{}", calls.unwrap_or_default()), + Portal::Current | Portal::Exits | Portal::ExitsAtOnce | Portal::AnswersAndExits => text("handle_token").unwrap_or_default(), }; let path = format!("/org/freedesktop/portal/desktop/request/{sender}/{token}"); let Ok(handle) = OwnedObjectPath::try_from(path.as_str()) else { break }; @@ -558,14 +536,22 @@ mod tests { if server.reply(&header, &handle).is_err() { break; } - if portal == Portal::Old { + if matches!(portal, Portal::Old | Portal::Exits) { // As a person answers the dialog: after the caller has the path returned. std::thread::sleep(std::time::Duration::from_millis(300)); } + if matches!(portal, Portal::Exits | Portal::ExitsAtOnce) { + break; + } if portal != Portal::OldEarly && respond().is_err() { break; } + if portal == Portal::AnswersAndExits { + break; + } } + // The portal leaves the bus: the bus sends `NameOwnerChanged` for its name. + let _ = server.close(); }); let client = zbus::blocking::connection::Builder::address(address.as_str()).ok()?.build().ok()?; Some((client, seen, daemon)) @@ -589,7 +575,7 @@ mod tests { #[test] fn the_portal_is_asked_on_the_first_file_type_and_answers_the_one_picked() { let script = vec![(0, Some(("file:///home/me/Art/poster", "PDF-compatible .ai"))), (1, None)]; - let Some((conn, seen, mut daemon)) = mock_portal(script, Portal::Current) else { + let Some((conn, seen, _daemon)) = mock_portal(script, Portal::Current) else { eprintln!("no dbus-daemon: skipped"); return; }; @@ -599,7 +585,6 @@ mod tests { assert_eq!(save_within(&conn, &request), Ok(None)); let seen = seen.lock().map(|s| s.clone()).unwrap_or_default(); assert_eq!(seen.first(), Some(&(Some("Untitled-1.vectorcraft".to_string()), Some(request.filters[0].0.clone())))); - let _ = daemon.kill(); } /// A portal older than 0.9 answers on the request path it returns, and the answer is awaited @@ -607,13 +592,12 @@ mod tests { #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] #[test] fn the_answer_is_awaited_on_the_path_the_portal_returns() { - let Some((conn, _seen, mut daemon)) = mock_portal(vec![(0, Some(("file:///tmp/card", "SVG")))], Portal::Old) else { + let Some((conn, _seen, _daemon)) = mock_portal(vec![(0, Some(("file:///tmp/card", "SVG")))], Portal::Old) else { eprintln!("no dbus-daemon: skipped"); return; }; let request = request(); assert_eq!(pick_answer(save_within(&conn, &request).unwrap(), &request), ["/tmp/card", "svg"]); - let _ = daemon.kill(); } /// A portal older than 0.9 can answer before its reply names the request path: the answer is @@ -622,14 +606,41 @@ mod tests { #[test] fn an_answer_that_comes_before_the_path_is_kept() { let script = vec![(0, Some(("file:///tmp/card", "SVG"))), (1, None)]; - let Some((conn, _seen, mut daemon)) = mock_portal(script, Portal::OldEarly) else { + let Some((conn, _seen, _daemon)) = mock_portal(script, Portal::OldEarly) else { eprintln!("no dbus-daemon: skipped"); return; }; let request = request(); assert_eq!(pick_answer(save_within(&conn, &request).unwrap(), &request), ["/tmp/card", "svg"]); assert_eq!(save_within(&conn, &request), Ok(None)); - let _ = daemon.kill(); + } + + /// A portal that leaves the bus after its reply without answering, while its dialog is open or + /// at once, ends the wait with an error, and rfd shows the dialog. + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] + #[test] + fn a_portal_that_exits_during_the_dialog_ends_the_wait() { + for portal in [Portal::Exits, Portal::ExitsAtOnce] { + let Some((conn, _seen, _daemon)) = mock_portal(vec![(0, Some(("file:///tmp/card", "SVG")))], portal) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + assert_eq!(save_within(&conn, &request()), Err("the desktop portal disappeared while waiting for its answer".to_string())); + } + } + + /// A portal that answers and then leaves the bus: the save reads the answer. Whether the answer + /// or the bus's report comes first depends on timing here; `portal_request`'s + /// `a_queued_answer_wins_over_the_portal_leaving` queues both before the wait. + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] + #[test] + fn an_answer_sent_just_before_the_portal_exits_is_kept() { + let Some((conn, _seen, _daemon)) = mock_portal(vec![(0, Some(("file:///tmp/card", "SVG")))], Portal::AnswersAndExits) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + let request = request(); + assert_eq!(pick_answer(save_within(&conn, &request).unwrap(), &request), ["/tmp/card", "svg"]); } /// The labels go to the dialog in the interface's language, which is how the answer is matched.