fix: keep GNOME notifications open (fix #2566) (#3588)

* experimenting...

Signed-off-by: BestCodes <106822363+The-Best-Codes@users.noreply.github.com>

* omit unconfirmed fact from .changes

Signed-off-by: BestCodes <106822363+The-Best-Codes@users.noreply.github.com>

* fix(notification): send untracked Linux notifications on the shared connection

Holding notify-rust's handle with `on_close` blocked an async runtime
worker until the server closed the notification, and opened a D-Bus
connection per notification. Since GNOME keeps notifications in its tray,
a handful of them could stall every async task of the app.

Send every notification on the connection the plugin already keeps open
for its action listener instead: it keeps the sender's name alive for
GNOME at the cost of a single connection and thread, and the listener
runs the `on_close` callbacks.

* fix(notification): offer `on_close` on the BSDs too

The BSDs share the D-Bus implementation with Linux, including the GNOME
issue, so gate `NotificationBuilder::on_close` like the rest of that
implementation and document when the callback runs.

* fix(notification): run `on_close` without the tracker lock held

The callback ran on the listener thread while the guard of the callback
map was alive, and a panic in it ended that thread, which then reported
no action of any notification for the rest of the app's life. Take the
callback out of the map before running it and contain its panics.

* fix(notification): use the covector package names in the change files

`tauri-plugin-notification` and `@tauri-apps/plugin-notification` are
not packages of .changes/config.json, so covector refused the change
file. Use `notification` and `notification-js`, and record the new
`on_close` API as a feature of its own.

Also let `on_close` take an `FnOnce`, since it runs at most once.

---------

Signed-off-by: BestCodes <106822363+The-Best-Codes@users.noreply.github.com>
Co-authored-by: Lucas Nogueira <lucas@tauri.app>

.changes/fix-gnome-notifications.md

@@ -0,0 +1,6 @@
+---
+notification: patch
+notification-js: patch
+---
+
+Fix notifications closing right after they show on GNOME: on Linux and the BSDs, every notification is now sent on the D-Bus connection the plugin keeps open.

.changes/notification-on-close.md

@@ -0,0 +1,6 @@
+---
+notification: minor:feat
+notification-js: minor:feat
+---
+
+Add `NotificationBuilder::on_close` on Linux and the BSDs, a Rust callback that runs once the notification server reports the notification closed.

plugins/notification/src/desktop.rs

@@ -139,19 +139,27 @@ impl<R: Runtime> crate::NotificationBuilder<R> {
         if let Some(sound) = self.data.sound.clone() {
             notification = notification.sound(sound);
         }
-        if shared.has_listeners() {
-            if let Some(action_type) = self
+        let report = shared.has_listeners();
+        if report
+            && let Some(action_type) = self
                 .data
                 .action_type_id
                 .as_ref()
                 .and_then(|id| lock(&shared.action_types).get(id).cloned())
-            {
-                for action in action_type.actions() {
-                    notification = notification.action(action.id(), action.title());
-                }
+        {
+            for action in action_type.actions() {
+                notification = notification.action(action.id(), action.title());
             }
-            let id = self.data.id;
-            let data = serde_json::to_value(&self.data).unwrap_or(Value::Null);
+        }
+        let id = self.data.id;
+        let data = serde_json::to_value(&self.data).unwrap_or(Value::Null);
+        // Linux and the BSDs send every notification on the connection the plugin keeps open,
+        // since servers like GNOME's close a notification once the connection that sent it goes
+        // away.
+        #[cfg(all(unix, not(target_os = "macos")))]
+        notification.show_tracked(shared, id, data, report, self.on_close)?;
+        #[cfg(any(windows, target_os = "macos"))]
+        if report {
             notification.show_tracked(shared, id, data)?;
         } else {
             notification.show()?;
@@ -346,6 +354,8 @@ mod imp {
         /// The tracked notifications the server still shows, by server id: their identifier and
         /// the payload their actions carry.
         pub(super) shown: Mutex<HashMap<u32, (i32, Value)>>,
+        /// The `on_close` callbacks of the tracked notifications, by server id.
+        pub(super) on_close: Mutex<HashMap<u32, crate::OnClose>>,
     }
 
     /// The payload of an `actionPerformed` event, as the mobile plugins send it.
@@ -421,6 +431,7 @@ mod imp {
         ///   .run(tauri::generate_context!("test/tauri.conf.json"))
         ///   .expect("error while running tauri application");
         /// ```
+        #[cfg(any(windows, target_os = "macos", feature = "windows7-compat"))]
         pub fn show(self) -> crate::Result<()> {
             let notification = self.build()?;
             tauri::async_runtime::spawn(async move {
@@ -430,21 +441,29 @@ mod imp {
             Ok(())
         }
 
-        /// Shows the notification and reports what the user does with it: a click on the
-        /// notification (`tap`) or on an action goes to `shared`'s listeners as an
-        /// `actionPerformed` event carrying `data`. Closing it reports nothing.
+        /// Shows the notification on the connection `shared` keeps open. When `report` is set,
+        /// what the user does with it is reported: a click on the notification (`tap`) or on an
+        /// action goes to `shared`'s listeners as an `actionPerformed` event carrying `data`.
+        /// `on_close` runs once the server reports the notification closed.
         #[cfg(all(unix, not(target_os = "macos")))]
         pub(crate) fn show_tracked(
             self,
             shared: Arc<Shared>,
             id: i32,
             data: Value,
+            report: bool,
+            on_close: Option<crate::OnClose>,
         ) -> crate::Result<()> {
             let mut notification = self.build()?;
-            // The Desktop Notifications Specification reports a click on the notification itself
-            // as the action `default`, and only for a notification that offers that action.
-            notification.action("default", "");
-            tauri::async_runtime::spawn_blocking(move || track(notification, &shared, id, data));
+            if report {
+                // The Desktop Notifications Specification reports a click on the notification
+                // itself as the action `default`, and only for a notification that offers that
+                // action.
+                notification.action("default", "");
+            }
+            tauri::async_runtime::spawn_blocking(move || {
+                track(notification, &shared, id, report.then_some(data), on_close)
+            });
             Ok(())
         }
 
@@ -527,9 +546,15 @@ mod imp {
     );
 
     /// Linux and the BSDs: shows the notification and records it, so the listener thread knows
-    /// which payload its signals carry.
+    /// which payload its signals carry, when there is one, and what to run once it closes.
     #[cfg(all(unix, not(target_os = "macos")))]
-    fn track(notification: notify_rust::Notification, shared: &Arc<Shared>, id: i32, data: Value) {
+    fn track(
+        notification: notify_rust::Notification,
+        shared: &Arc<Shared>,
+        id: i32,
+        data: Option<Value>,
+        on_close: Option<crate::OnClose>,
+    ) {
         let tracker = &shared.tracker;
         let Some(connection) = tracker
             .connection
@@ -542,7 +567,12 @@ mod imp {
         // recorded waits for it instead of being missed.
         let mut shown = lock(&tracker.shown);
         if let Ok(server_id) = notify(connection, &notification) {
-            shown.insert(server_id, (id, data));
+            if let Some(data) = data {
+                shown.insert(server_id, (id, data));
+            }
+            if let Some(on_close) = on_close {
+                lock(&tracker.on_close).insert(server_id, on_close);
+            }
         }
     }
 
@@ -632,6 +662,13 @@ mod imp {
             Some("NotificationClosed") => {
                 if let Ok((server_id, _reason)) = body.deserialize::<(u32, u32)>() {
                     lock(&shared.tracker.shown).remove(&server_id);
+                    // Taken out first, so the callback runs without the lock held.
+                    let on_close = lock(&shared.tracker.on_close).remove(&server_id);
+                    if let Some(on_close) = on_close {
+                        // A panicking callback must not end the thread that reports the signals
+                        // of the other notifications.
+                        let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(on_close.0));
+                    }
                 }
                 return;
             }
@@ -802,6 +839,29 @@ mod tests {
         assert!(lock(&shared.tracker.shown).is_empty());
     }
 
+    #[cfg(all(unix, not(target_os = "macos")))]
+    #[test]
+    fn on_close_runs_once_without_the_lock_held() {
+        let shared = Arc::new(Shared::default());
+        let closed = Arc::new(Mutex::new(0));
+        let (inner, count) = (Arc::clone(&shared), Arc::clone(&closed));
+        lock(&shared.tracker.on_close).insert(
+            41,
+            crate::OnClose(Box::new(move || {
+                // deadlocks if the callback runs with the lock held
+                assert!(lock(&inner.tracker.on_close).is_empty());
+                *count.lock().unwrap() += 1;
+            })),
+        );
+        lock(&shared.tracker.on_close).insert(42, crate::OnClose(Box::new(|| panic!("boom"))));
+
+        imp::on_signal(&shared, &signal("NotificationClosed", &(42u32, 2u32)));
+        imp::on_signal(&shared, &signal("NotificationClosed", &(41u32, 2u32)));
+        imp::on_signal(&shared, &signal("NotificationClosed", &(41u32, 2u32)));
+
+        assert_eq!(*closed.lock().unwrap(), 1);
+    }
+
     #[test]
     fn a_handler_may_register_another_handler() {
         let shared = Arc::new(Shared::default());

plugins/notification/src/lib.rs

@@ -14,6 +14,8 @@
 )]
 
 use serde::Serialize;
+#[cfg(all(desktop, unix, not(target_os = "macos")))]
+use std::fmt;
 #[cfg(desktop)]
 use tauri::AppHandle;
 #[cfg(mobile)]
@@ -50,6 +52,18 @@ pub struct NotificationBuilder<R: Runtime> {
     #[cfg(mobile)]
     handle: PluginHandle<R>,
     pub(crate) data: NotificationData,
+    #[cfg(all(desktop, unix, not(target_os = "macos")))]
+    on_close: Option<OnClose>,
+}
+
+#[cfg(all(desktop, unix, not(target_os = "macos")))]
+pub(crate) struct OnClose(Box<dyn FnOnce() + Send + 'static>);
+
+#[cfg(all(desktop, unix, not(target_os = "macos")))]
+impl fmt::Debug for OnClose {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("OnClose")
+    }
 }
 
 impl<R: Runtime> NotificationBuilder<R> {
@@ -58,6 +72,8 @@ impl<R: Runtime> NotificationBuilder<R> {
         Self {
             app,
             data: Default::default(),
+            #[cfg(all(desktop, unix, not(target_os = "macos")))]
+            on_close: None,
         }
     }
 
@@ -212,6 +228,21 @@ impl<R: Runtime> NotificationBuilder<R> {
         self.data.silent = true;
         self
     }
+
+    /// Sets a callback to run when the notification closes, whether the user dismissed it, it
+    /// expired or the app removed it.
+    ///
+    /// The callback runs on a background thread and should return quickly. It does not run when
+    /// the session bus cannot be reached, or when the app exits first.
+    ///
+    /// ## Platform-specific
+    ///
+    /// - **Linux and the BSDs**: only available there.
+    #[cfg(all(desktop, unix, not(target_os = "macos")))]
+    pub fn on_close<F: FnOnce() + Send + 'static>(mut self, handler: F) -> Self {
+        self.on_close = Some(OnClose(Box::new(handler)));
+        self
+    }
 }
 
 /// Extensions to [`tauri::App`], [`tauri::AppHandle`], [`tauri::WebviewWindow`], [`tauri::Webview`] and [`tauri::Window`] to access the notification APIs.