diff --git a/local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs b/local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs index 81fba229c1..33d8186bdb 100644 --- a/local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs +++ b/local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs @@ -4,17 +4,12 @@ use std::pin::pin; use std::sync::{Arc, Mutex}; use zbus::{ - connection::Builder as ConnectionBuilder, - export::futures_core::Stream, - fdo, - interface, - message::Header, - object_server::SignalEmitter, - proxy, - zvariant::ObjectPath, + connection::Builder as ConnectionBuilder, export::futures_core::Stream, fdo, interface, + message::Header, object_server::SignalEmitter, proxy, zvariant::ObjectPath, }; -const BUS_NAME: &str = "org.freedesktop.StatusNotifierWatcher"; +/// Well-known D-Bus service name for the KDE StatusNotifierWatcher protocol. +const BUS_NAME: &str = "org.kde.StatusNotifierWatcher"; const OBJECT_PATH: &str = "/StatusNotifierWatcher"; /// Maximum number of entries (item paths or host names) retained per registry. @@ -51,13 +46,45 @@ fn validate_input(s: &str) -> Result<(), String> { Ok(()) } +// --------------------------------------------------------------------------- +// Canonical item identity +// --------------------------------------------------------------------------- + +/// Build the canonical dedup key for an item registration. +/// +/// When `value` is an object path (starts with `/`), prepend the sender's +/// unique bus name so that two clients using the conventional +/// `/StatusNotifierItem` path do not collide. When `value` is a bus name +/// (starts with a letter or `:`), use it as-is. +fn canonicalize_item_key(sender: &str, value: &str) -> String { + if value.starts_with('/') { + format!("{sender}{value}") + } else { + value.to_owned() + } +} + +/// Recover the original display value from a canonical registry key. +/// +/// Canonical keys are either `` — where the sender is a unique +/// bus name (never contains `/`) — or a bare bus name (also never contains +/// `/`). If the key contains `/` at a non-zero position, everything before +/// that `/` is the owner prefix and is stripped. +fn strip_owner_prefix(key: &str) -> &str { + match key.find('/') { + Some(0) => key, + Some(pos) => &key[pos..], + None => key, + } +} + // --------------------------------------------------------------------------- // Owner-keyed registry with insertion-order tracking // --------------------------------------------------------------------------- /// Maps owner unique bus names (e.g. ``:1.42``) to the set of values they -/// registered. A separate [`Vec`] tracks the global insertion order of -/// values so that oldest-first eviction under [`MAX_ENTRIES`] is deterministic. +/// registered. A separate [`Vec`] tracks the global insertion order of values +/// so that oldest-first eviction under [`MAX_ENTRIES`] is deterministic. struct Registry { /// owner → set of registered values (item paths or host names). by_owner: HashMap>, @@ -75,11 +102,10 @@ impl Registry { /// Register ``value`` on behalf of ``owner``. /// - /// Returns ``true`` if the value was newly added, ``false`` if it was - /// already present (registered by any owner — values are globally unique). - /// When the registry is at capacity the oldest entry is evicted first. + /// Returns ``true`` if the exact value was newly added, ``false`` if it + /// was already present. When the registry is at capacity the oldest entry + /// is evicted first. fn register(&mut self, owner: &str, value: &str) -> bool { - // Dedup by value: a given item path / host name can only exist once. if self.insertion_order.iter().any(|v| v == value) { return false; } @@ -130,20 +156,22 @@ impl Registry { self.insertion_order.len() } - /// Remove every entry owned by ``owner``. Called when a bus name - /// vanishes (NameOwnerChanged with empty ``new_owner``). - /// Returns the number of entries removed. - fn purge_owner(&mut self, owner: &str) -> usize { - match self.by_owner.remove(owner) { - Some(set) => { - let count = set.len(); - for value in &set { - self.insertion_order.retain(|v| v != value); - } - count + /// Remove every entry owned by ``owner`` in insertion order. + fn purge_owner(&mut self, owner: &str) -> Vec { + let Some(set) = self.by_owner.remove(owner) else { + return Vec::new(); + }; + + let mut removed = Vec::new(); + self.insertion_order.retain(|value| { + if set.contains(value) { + removed.push(value.clone()); + false + } else { + true } - None => 0, - } + }); + removed } /// Drop the oldest value (front of ``insertion_order``) and remove it @@ -165,7 +193,7 @@ impl Registry { // StatusNotifierWatcher D-Bus interface // --------------------------------------------------------------------------- -/// org.freedesktop.StatusNotifierWatcher D-Bus interface. +/// org.kde.StatusNotifierWatcher D-Bus interface. /// /// Tracks registered system tray items and hosts for KDE Plasma. Each /// registration is bound to the caller's unique bus name; only the owning @@ -177,6 +205,55 @@ struct StatusNotifierWatcher { hosts: Arc>, } +#[derive(Debug, Default, PartialEq, Eq)] +struct PurgeResult { + items: Vec, + hosts: Vec, +} + +impl PurgeResult { + fn is_empty(&self) -> bool { + self.items.is_empty() && self.hosts.is_empty() + } +} + +#[derive(Debug, PartialEq, Eq)] +enum PurgeSignalEvent { + Item(String), + Host, +} + +fn should_purge_name_owner_change(name: &str, old_owner: &str, new_owner: &str) -> bool { + new_owner.is_empty() && !old_owner.is_empty() && name == old_owner && name.starts_with(':') +} + +fn purge_signal_events(purged: &PurgeResult) -> Vec { + let mut events = Vec::with_capacity(purged.items.len() + purged.hosts.len()); + for item in &purged.items { + events.push(PurgeSignalEvent::Item(item.clone())); + } + for _ in &purged.hosts { + events.push(PurgeSignalEvent::Host); + } + events +} + +async fn emit_purge_unregistered_signals(signal_emitter: &SignalEmitter<'_>, purged: &PurgeResult) { + for event in purge_signal_events(purged) { + match event { + PurgeSignalEvent::Item(item) => { + let _ = + StatusNotifierWatcher::status_notifier_item_unregistered(signal_emitter, &item) + .await; + } + PurgeSignalEvent::Host => { + let _ = + StatusNotifierWatcher::status_notifier_host_unregistered(signal_emitter).await; + } + } + } +} + impl StatusNotifierWatcher { fn new() -> Self { Self { @@ -188,17 +265,19 @@ impl StatusNotifierWatcher { /// Register an item on behalf of ``owner``. Returns ``true`` if newly /// added. fn register_item(&self, owner: &str, item: &str) -> bool { + let key = canonicalize_item_key(owner, item); self.items .lock() - .map(|mut g| g.register(owner, item)) + .map(|mut g| g.register(owner, &key)) .unwrap_or(false) } /// Unregister an item, but only if ``owner`` matches the recorded owner. fn unregister_item(&self, owner: &str, item: &str) -> bool { + let key = canonicalize_item_key(owner, item); self.items .lock() - .map(|mut g| g.unregister(owner, item)) + .map(|mut g| g.unregister(owner, &key)) .unwrap_or(false) } @@ -219,7 +298,15 @@ impl StatusNotifierWatcher { } fn items_snapshot(&self) -> Vec { - self.items.lock().map(|g| g.snapshot()).unwrap_or_default() + self.items + .lock() + .map(|g| { + g.snapshot() + .into_iter() + .map(|k| strip_owner_prefix(&k).to_owned()) + .collect() + }) + .unwrap_or_default() } fn is_host_registered(&self) -> bool { @@ -231,24 +318,40 @@ impl StatusNotifierWatcher { self.items.lock().map(|g| g.total_len()).unwrap_or(0) } - /// Remove all items **and** hosts owned by ``owner``. - /// Returns the total number of entries purged. - fn purge_owner(&self, owner: &str) -> usize { + fn purge_owner(&self, owner: &str) -> PurgeResult { let items = self .items .lock() - .map(|mut g| g.purge_owner(owner)) - .unwrap_or(0); + .map(|mut g| { + g.purge_owner(owner) + .into_iter() + .map(|k| strip_owner_prefix(&k).to_owned()) + .collect() + }) + .unwrap_or_default(); let hosts = self .hosts .lock() - .map(|mut g| g.purge_owner(owner)) - .unwrap_or(0); - items + hosts + .map(|mut g| g.purge_owner(owner).into_iter().collect()) + .unwrap_or_default(); + PurgeResult { items, hosts } + } + + fn purge_for_name_owner_change( + &self, + name: &str, + old_owner: &str, + new_owner: &str, + ) -> PurgeResult { + if should_purge_name_owner_change(name, old_owner, new_owner) { + self.purge_owner(old_owner) + } else { + PurgeResult::default() + } } } -#[interface(name = "org.freedesktop.StatusNotifierWatcher")] +#[interface(name = "org.kde.StatusNotifierWatcher")] impl StatusNotifierWatcher { // --- Methods --- @@ -399,6 +502,12 @@ impl StatusNotifierWatcher { async fn status_notifier_host_registered( signal_emitter: &SignalEmitter<'_>, ) -> zbus::Result<()>; + + /// Emitted when a status notifier host is unregistered. + #[zbus(signal, name = "StatusNotifierHostUnregistered")] + async fn status_notifier_host_unregistered( + signal_emitter: &SignalEmitter<'_>, + ) -> zbus::Result<()>; } // --------------------------------------------------------------------------- @@ -432,9 +541,21 @@ async fn run_name_owner_changed_listener( let signals = match proxy.receive_name_owner_changed().await { Ok(s) => s, Err(e) => { - eprintln!( - "statusnotifierwatcher: failed to subscribe to NameOwnerChanged: {e}" - ); + eprintln!("statusnotifierwatcher: failed to subscribe to NameOwnerChanged: {e}"); + return; + } + }; + let object_path: ObjectPath<'_> = match OBJECT_PATH.try_into() { + Ok(path) => path, + Err(e) => { + eprintln!("statusnotifierwatcher: invalid object path {OBJECT_PATH}: {e}"); + return; + } + }; + let signal_emitter = match SignalEmitter::new(&connection, object_path) { + Ok(emitter) => emitter, + Err(e) => { + eprintln!("statusnotifierwatcher: failed to create signal emitter: {e}"); return; } }; @@ -448,15 +569,18 @@ async fn run_name_owner_changed_listener( match item { Some(signal) => { if let Ok(args) = signal.args() { - // new_owner is empty when a name is released / client disconnected. - if args.new_owner.is_empty() && !args.old_owner.is_empty() { - let removed = watcher.purge_owner(&args.old_owner); - if removed > 0 { - eprintln!( - "statusnotifierwatcher: purged {removed} entries for vanished owner {}", - args.old_owner - ); - } + let purged = watcher.purge_for_name_owner_change( + &args.name, + &args.old_owner, + &args.new_owner, + ); + if !purged.is_empty() { + emit_purge_unregistered_signals(&signal_emitter, &purged).await; + let removed = purged.items.len() + purged.hosts.len(); + eprintln!( + "statusnotifierwatcher: purged {removed} entries for vanished owner {}", + args.old_owner + ); } } } @@ -550,10 +674,7 @@ mod tests { let w = StatusNotifierWatcher::new(); assert!(w.register_item(":1.1", "/org/example/Item1")); assert!(!w.register_item(":1.1", "/org/example/Item1")); - assert_eq!( - w.items_snapshot(), - vec!["/org/example/Item1".to_string()] - ); + assert_eq!(w.items_snapshot(), vec!["/org/example/Item1".to_string()]); } #[test] @@ -673,15 +794,34 @@ mod tests { } #[test] - fn same_value_registered_by_different_owner_is_idempotent() { + fn same_bus_name_registered_by_different_owner_is_idempotent() { let w = StatusNotifierWatcher::new(); - assert!(w.register_item(":1.1", "/shared/Item")); - // Same value registered by a different owner is not newly added. - assert!(!w.register_item(":1.2", "/shared/Item")); - // Only one entry exists. + assert!(w.register_item(":1.1", "org.example.SharedItem")); + assert!(!w.register_item(":1.2", "org.example.SharedItem")); assert_eq!(w.items_snapshot().len(), 1); - // Original owner can still unregister. - assert!(w.unregister_item(":1.1", "/shared/Item")); + assert!(w.unregister_item(":1.1", "org.example.SharedItem")); + } + + #[test] + fn same_path_registered_by_different_owners_are_distinct_items() { + let w = StatusNotifierWatcher::new(); + assert!(w.register_item(":1.1", "/StatusNotifierItem")); + assert!(w.register_item(":1.2", "/StatusNotifierItem")); + + let items = w.items_snapshot(); + assert_eq!(items.len(), 2); + assert_eq!( + items + .iter() + .filter(|item| item.as_str() == "/StatusNotifierItem") + .count(), + 2 + ); + + assert!(!w.unregister_item(":1.3", "/StatusNotifierItem")); + assert!(w.unregister_item(":1.1", "/StatusNotifierItem")); + assert_eq!(w.items_snapshot(), vec!["/StatusNotifierItem".to_string()]); + assert!(w.unregister_item(":1.2", "/StatusNotifierItem")); } #[test] @@ -710,9 +850,13 @@ mod tests { w.register_host(":1.10", "org.kde.plasma"); let purged = w.purge_owner(":1.10"); - assert_eq!(purged, 3, "should purge 2 items + 1 host"); - - // :1.10's items and hosts are gone; :1.20's item survives. + assert_eq!( + purged, + PurgeResult { + items: vec!["/item/a".to_string(), "/item/b".to_string()], + hosts: vec!["org.kde.plasma".to_string()], + } + ); assert_eq!(w.items_snapshot(), vec!["/item/c".to_string()]); assert!(!w.is_host_registered()); } @@ -721,10 +865,38 @@ mod tests { fn purge_unknown_owner_is_noop() { let w = StatusNotifierWatcher::new(); w.register_item(":1.1", "/item/a"); - assert_eq!(w.purge_owner(":9.9"), 0); + assert_eq!(w.purge_owner(":9.9"), PurgeResult::default()); assert_eq!(w.items_snapshot().len(), 1); } + #[test] + fn well_known_name_release_does_not_trigger_purge() { + let w = StatusNotifierWatcher::new(); + w.register_item(":1.10", "/item/a"); + + let purged = w.purge_for_name_owner_change("org.example.App", ":1.10", ""); + assert_eq!(purged, PurgeResult::default()); + assert_eq!(w.items_snapshot(), vec!["/item/a".to_string()]); + } + + #[test] + fn unique_name_release_purge_collects_unregister_signal_events() { + let w = StatusNotifierWatcher::new(); + w.register_item(":1.10", "/item/a"); + w.register_item(":1.10", "/item/b"); + w.register_host(":1.10", "org.kde.plasma"); + + let purged = w.purge_for_name_owner_change(":1.10", ":1.10", ""); + assert_eq!( + purge_signal_events(&purged), + vec![ + PurgeSignalEvent::Item("/item/a".to_string()), + PurgeSignalEvent::Item("/item/b".to_string()), + PurgeSignalEvent::Host, + ] + ); + } + // ======================================================================= // New tests — input validation // ======================================================================= @@ -805,8 +977,7 @@ mod tests { // Evict one more — the owner set shrinks but should still exist. w.register_item(":1.99", "/x/extra"); assert_eq!(w.items_count(), MAX_ENTRIES); - // All entries still belong to :1.99. let purged = w.purge_owner(":1.99"); - assert_eq!(purged, MAX_ENTRIES); + assert_eq!(purged.items.len(), MAX_ENTRIES); } }