notifications: Refactor and clean up

Fixes a regression where clicking on notifications would not open the
app anymore.

src/application.rs

@@ -167,8 +167,8 @@ mod imp {
                 gio::ActionEntry::builder("quit")
                     .activate(|obj: &super::Application, _, _| {
                         if let Some(window) = obj.active_window() {
-                            // This is needed to trigger the delete event
+                            // This is needed to trigger the close request and save the window
-                            // and saving the window state
+                            // state.
                             window.close();
                         }
 
@@ -186,12 +186,19 @@ mod imp {
                 gio::ActionEntry::builder(SessionIntentType::ShowMatrixId.action_name())
                     .parameter_type(Some(&SessionIntentType::static_variant_type()))
                     .activate(|obj: &super::Application, _, variant| {
-                        let Some((session_id, intent)) = SessionIntent::parse_with_session_id(
+                        debug!(
-                            SessionIntentType::ShowMatrixId,
+                            "`app.{}` action activated",
-                            variant,
+                            SessionIntentType::ShowMatrixId.action_name()
-                        ) else {
+                        );
+
+                        let Some((session_id, intent)) = variant.and_then(|variant| {
+                            SessionIntent::from_variant_with_session_id(
+                                SessionIntentType::ShowMatrixId,
+                                variant,
+                            )
+                        }) else {
                             error!(
-                                "Triggered `{}` action without the proper payload",
+                                "Activated `app.{}` action without the proper payload",
                                 SessionIntentType::ShowMatrixId.action_name()
                             );
                             return;
@@ -207,12 +214,19 @@ mod imp {
                 )
                 .parameter_type(Some(&SessionIntentType::static_variant_type()))
                 .activate(|obj: &super::Application, _, variant| {
-                    let Some((session_id, intent)) = SessionIntent::parse_with_session_id(
+                    debug!(
-                        SessionIntentType::ShowIdentityVerification,
+                        "`app.{}` action activated",
-                        variant,
+                        SessionIntentType::ShowIdentityVerification.action_name()
-                    ) else {
+                    );
+
+                    let Some((session_id, intent)) = variant.and_then(|variant| {
+                        SessionIntent::from_variant_with_session_id(
+                            SessionIntentType::ShowIdentityVerification,
+                            variant,
+                        )
+                    }) else {
                         error!(
-                            "Triggered `{}` action without the proper payload",
+                            "Activated `app.{}` action without the proper payload",
                             SessionIntentType::ShowIdentityVerification.action_name()
                         );
                         return;
@@ -289,6 +303,7 @@ mod imp {
 
         /// Process the given URI.
         fn process_uri(&self, uri: &str) {
+            debug!(uri, "Processing URI…");
             match MatrixIdUri::parse(uri) {
                 Ok(matrix_id) => {
                     self.select_session_for_intent(SessionIntent::ShowMatrixId(matrix_id));
@@ -299,7 +314,7 @@ mod imp {
 
         /// Select a session to handle the given intent as soon as possible.
         fn select_session_for_intent(&self, intent: SessionIntent) {
-            debug!("Selecting session for intent {intent:?}");
+            debug!(?intent, "Selecting session for intent…");
 
             // We only handle a single intent at time, the latest one.
             self.intent_handler.disconnect_signals();
@@ -307,26 +322,27 @@ mod imp {
             if self.session_list.state() == LoadingState::Ready {
                 match self.session_list.n_items() {
                     0 => {
-                        warn!("Cannot open URI with no logged in session");
+                        warn!("Cannot process intent with no logged in session");
                     }
                     1 => {
                         let session = self
                             .session_list
                             .first()
-                            .expect("There should be one session");
+                            .expect("there should be one session");
                         self.process_session_intent(session.session_id(), intent);
                     }
                     _ => {
                         spawn!(clone!(
-                            #[weak(rename_to = obj)]
+                            #[weak(rename_to = imp)]
                             self,
                             async move {
-                                obj.ask_session_for_intent(intent).await;
+                                imp.ask_session_for_intent(intent).await;
                             }
                         ));
                     }
                 }
             } else {
+                debug!(?intent, "Session list is not ready, queuing intent…");
                 // Wait for the list to be ready.
                 let cell = Rc::new(RefCell::new(Some(intent)));
                 let handler = self.session_list.connect_state_notify(clone!(
@@ -353,6 +369,7 @@ mod imp {
         ///
         /// The session list needs to be ready.
         async fn ask_session_for_intent(&self, intent: SessionIntent) {
+            debug!(?intent, "Asking to select a session to process intent…");
             let main_window = self.present_main_window();
 
             let Some(session_id) = main_window.ask_session().await else {
@@ -366,22 +383,35 @@ mod imp {
         /// Process the given intent for the given session, as soon as the
         /// session is ready.
         fn process_session_intent(&self, session_id: String, intent: SessionIntent) {
-            debug!(session = session_id, "Processing session intent {intent:?}");
-
             let Some(session_info) = self.session_list.get(&session_id) else {
-                warn!("Could not find session to process intent {intent:?}");
+                warn!(
+                    session = session_id,
+                    ?intent,
+                    "Could not find session to process intent"
+                );
                 toast!(self.present_main_window(), gettext("Session not found"));
                 return;
             };
 
+            debug!(session = session_id, ?intent, "Processing session intent…");
+
             if session_info.is::<FailedSession>() {
                 // We can't do anything, it should show an error screen.
-                warn!("Could not process intent {intent:?} for failed session");
+                warn!(
+                    session = session_id,
+                    ?intent,
+                    "Could not process intent for failed session"
+                );
             } else if let Some(session) = session_info.downcast_ref::<Session>() {
                 if session.state() == SessionState::Ready {
                     self.present_main_window()
                         .process_session_intent(session.session_id(), intent);
                 } else {
+                    debug!(
+                        session = session_id,
+                        ?intent,
+                        "Session is not ready, queuing intent…"
+                    );
                     // Wait for the session to be ready.
                     let cell = Rc::new(RefCell::new(Some((session_id, intent))));
                     let handler = session.connect_ready(clone!(
@@ -401,6 +431,11 @@ mod imp {
                     self.intent_handler.set(session.upcast_ref(), vec![handler]);
                 }
             } else {
+                debug!(
+                    session = session_id,
+                    ?intent,
+                    "Session is still loading, queuing intent…"
+                );
                 // Wait for the session to be a `Session`.
                 let cell = Rc::new(RefCell::new(Some((session_id, intent))));
                 let handler = self.session_list.connect_items_changed(clone!(

src/intent.rs

@@ -5,7 +5,7 @@ use ruma::OwnedUserId;
 
 use crate::{session::model::VerificationKey, utils::matrix::MatrixIdUri};
 
-/// An intent that can be handled by a session.
+/// Intents that can be handled by a session.
 ///
 /// It cannot be cloned intentionnally, so it is handled only once.
 #[derive(Debug)]
@@ -17,11 +17,34 @@ pub(crate) enum SessionIntent {
 }
 
 impl SessionIntent {
-    /// Construct a `SessionIntent` from its type and a payload in a `GVariant`.
+    /// Get the application action name for this session intent type.
+    pub(crate) fn app_action_name(&self) -> &'static str {
+        match self {
+            SessionIntent::ShowMatrixId(_) => "app.show-matrix-id",
+            SessionIntent::ShowIdentityVerification(_) => "app.show-identity-verification",
+        }
+    }
+
+    /// Convert this intent to a `GVariant` with the given session ID.
+    pub(crate) fn to_variant_with_session_id(&self, session_id: &str) -> glib::Variant {
+        let payload = match self {
+            Self::ShowMatrixId(uri) => uri.to_variant(),
+            Self::ShowIdentityVerification(key) => key.to_variant(),
+        };
+        (session_id, payload).to_variant()
+    }
+
+    /// Convert a `GVariant` to a `SessionIntent` and session ID, given the
+    /// intent type.
     ///
-    /// Returns the intent on success. Returns `None` if the payload could not
+    /// Returns an  `(session_id, intent)` tuple on success. Returns `None` if
-    /// be parsed successfully.
+    /// the payload could not be parsed successfully.
-    pub(crate) fn parse(intent_type: SessionIntentType, payload: &glib::Variant) -> Option<Self> {
+    pub(crate) fn from_variant_with_session_id(
+        intent_type: SessionIntentType,
+        variant: &glib::Variant,
+    ) -> Option<(String, Self)> {
+        let (session_id, payload) = variant.get::<(String, glib::Variant)>()?;
+
         let intent = match intent_type {
             SessionIntentType::ShowMatrixId => Self::ShowMatrixId(payload.get::<MatrixIdUri>()?),
             SessionIntentType::ShowIdentityVerification => {
@@ -31,40 +54,19 @@ impl SessionIntent {
             }
         };
 
-        Some(intent)
-    }
-
-    /// Construct a `SessionIntent` from its type and a payload in a `GVariant`
-    /// containing a session ID.
-    ///
-    /// Returns a `(session_id, intent)` tuple on success. Returns `None` if the
-    /// payload could not be parsed successfully.
-    pub(crate) fn parse_with_session_id(
-        intent_type: SessionIntentType,
-        payload: Option<&glib::Variant>,
-    ) -> Option<(String, Self)> {
-        let (session_id, payload) = payload?.get::<(String, glib::Variant)>()?;
-        let intent = Self::parse(intent_type, &payload)?;
         Some((session_id, intent))
     }
+}
 
-    /// Convert this intent to a `GVariant` with the given session ID.
+impl From<MatrixIdUri> for SessionIntent {
-    pub(crate) fn to_variant_with_session_id(&self, session_id: &str) -> glib::Variant {
+    fn from(value: MatrixIdUri) -> Self {
-        let payload = self.to_variant();
+        Self::ShowMatrixId(value)
-        (session_id, payload).to_variant()
     }
 }
 
-impl ToVariant for SessionIntent {
+impl From<VerificationKey> for SessionIntent {
-    fn to_variant(&self) -> glib::Variant {
+    fn from(value: VerificationKey) -> Self {
-        match self {
+        Self::ShowIdentityVerification(value)
-            SessionIntent::ShowMatrixId(matrix_uri) => matrix_uri.to_variant(),
-            SessionIntent::ShowIdentityVerification(verification_key) => (
-                verification_key.user_id.as_str(),
-                verification_key.flow_id.as_str(),
-            )
-                .to_variant(),
-        }
     }
 }
 
@@ -85,14 +87,6 @@ impl SessionIntentType {
             SessionIntentType::ShowIdentityVerification => "show-identity-verification",
         }
     }
-
-    /// Get the application action name for this session intent type.
-    pub(crate) fn app_action_name(self) -> &'static str {
-        match self {
-            SessionIntentType::ShowMatrixId => "app.show-matrix-id",
-            SessionIntentType::ShowIdentityVerification => "app.show-identity-verification",
-        }
-    }
 }
 
 impl StaticVariantType for SessionIntentType {

src/session/model/mod.rs

@@ -12,7 +12,7 @@ mod user;
 mod user_sessions_list;
 mod verification;
 
-pub use self::{
+pub(crate) use self::{
     ignored_users::IgnoredUsers,
     notifications::{
         Notifications, NotificationsGlobalSetting, NotificationsRoomSetting, NotificationsSettings,

src/session/model/notifications/mod.rs

@@ -6,13 +6,13 @@ use tracing::{debug, warn};
 
 mod notifications_settings;
 
-pub use self::notifications_settings::{
+pub(crate) use self::notifications_settings::{
     NotificationsGlobalSetting, NotificationsRoomSetting, NotificationsSettings,
 };
 use super::{IdentityVerification, Session, VerificationKey};
 use crate::{
     gettext_f,
-    intent::{SessionIntent, SessionIntentType},
+    intent::SessionIntent,
     prelude::*,
     spawn_tokio,
     utils::matrix::{
@@ -35,18 +35,18 @@ mod imp {
     pub struct Notifications {
         /// The current session.
         #[property(get, set = Self::set_session, explicit_notify, nullable)]
-        pub session: glib::WeakRef<Session>,
+        session: glib::WeakRef<Session>,
         /// The push notifications that were presented.
         ///
         /// A map of room ID to list of notification IDs.
-        pub push: RefCell<HashMap<OwnedRoomId, HashSet<String>>>,
+        pub(super) push: RefCell<HashMap<OwnedRoomId, HashSet<String>>>,
         /// The identity verification notifications that were presented.
         ///
         /// A map of verification key to notification ID.
-        pub identity_verifications: RefCell<HashMap<VerificationKey, String>>,
+        pub(super) identity_verifications: RefCell<HashMap<VerificationKey, String>>,
         /// The notifications settings for this session.
         #[property(get)]
-        pub settings: NotificationsSettings,
+        settings: NotificationsSettings,
     }
 
     #[glib::object_subclass]
@@ -84,7 +84,7 @@ impl Notifications {
     }
 
     /// Whether notifications are enabled for the current session.
-    pub fn enabled(&self) -> bool {
+    pub(crate) fn enabled(&self) -> bool {
         let settings = self.settings();
         settings.account_enabled() && settings.session_enabled()
     }
@@ -94,7 +94,8 @@ impl Notifications {
         id: &str,
         title: &str,
         body: &str,
-        default_action: (&str, glib::Variant),
+        session_id: &str,
+        intent: &SessionIntent,
         icon: Option<&gdk::Texture>,
     ) {
         let notification = gio::Notification::new(title);
@@ -102,7 +103,8 @@ impl Notifications {
         notification.set_priority(gio::NotificationPriority::High);
         notification.set_body(Some(body));
 
-        let (action, target_value) = default_action;
+        let action = intent.app_action_name();
+        let target_value = intent.to_variant_with_session_id(session_id);
         notification.set_default_action_and_target_value(action, Some(&target_value));
 
         if let Some(notification_icon) = icon {
@@ -116,7 +118,11 @@ impl Notifications {
     ///
     /// The notification will not be shown if the application is active and the
     /// room of the event is displayed.
-    pub async fn show_push(&self, matrix_notification: Notification, matrix_room: MatrixRoom) {
+    pub(crate) async fn show_push(
+        &self,
+        matrix_notification: Notification,
+        matrix_room: MatrixRoom,
+    ) {
         // Do not show notifications if they are disabled.
         if !self.enabled() {
             return;
@@ -164,7 +170,7 @@ impl Notifications {
         let handle =
             spawn_tokio!(async move { matrix_room.get_member_no_sync(&owned_sender_id).await });
 
-        let sender = match handle.await.unwrap() {
+        let sender = match handle.await.expect("task was not aborted") {
             Ok(member) => member,
             Err(error) => {
                 warn!("Could not get member for notification: {error}");
@@ -212,18 +218,14 @@ impl Notifications {
             let random_id = glib::uuid_string_random();
             format!("{session_id}//{matrix_uri}//{random_id}")
         };
-        let payload =
-            SessionIntent::ShowMatrixId(matrix_uri).to_variant_with_session_id(session_id);
         let icon = room.avatar_data().as_notification_icon().await;
 
         Self::send_notification(
             &id,
             &room.display_name(),
             &body,
-            (
+            session_id,
-                SessionIntentType::ShowMatrixId.app_action_name(),
+            &SessionIntent::ShowMatrixId(matrix_uri),
-                payload.to_variant(),
-            ),
             icon.as_ref(),
         );
 
@@ -236,7 +238,10 @@ impl Notifications {
     }
 
     /// Show a notification for the given in-room identity verification.
-    pub async fn show_in_room_identity_verification(&self, verification: &IdentityVerification) {
+    pub(crate) async fn show_in_room_identity_verification(
+        &self,
+        verification: &IdentityVerification,
+    ) {
         // Do not show notifications if they are disabled.
         if !self.enabled() {
             return;
@@ -265,9 +270,6 @@ impl Notifications {
             &[("user", &user.display_name())],
         );
 
-        let payload = SessionIntent::ShowIdentityVerification(verification.key())
-            .to_variant_with_session_id(session_id);
-
         let icon = user.avatar_data().as_notification_icon().await;
 
         let id = format!("{session_id}//{room_id}//{user_id}//{flow_id}");
@@ -275,10 +277,8 @@ impl Notifications {
             &id,
             &title,
             &body,
-            (
+            session_id,
-                SessionIntentType::ShowIdentityVerification.app_action_name(),
+            &SessionIntent::ShowIdentityVerification(verification.key()),
-                payload.to_variant(),
-            ),
             icon.as_ref(),
         );
 
@@ -289,7 +289,10 @@ impl Notifications {
     }
 
     /// Show a notification for the given to-device identity verification.
-    pub async fn show_to_device_identity_verification(&self, verification: &IdentityVerification) {
+    pub(crate) async fn show_to_device_identity_verification(
+        &self,
+        verification: &IdentityVerification,
+    ) {
         // Do not show notifications if they are disabled.
         if !self.enabled() {
             return;
@@ -310,7 +313,7 @@ impl Notifications {
         let request = get_device::v3::Request::new(other_device_id.clone());
         let handle = spawn_tokio!(async move { client.send(request).await });
 
-        let display_name = match handle.await.unwrap() {
+        let display_name = match handle.await.expect("task was not aborted") {
             Ok(res) => res.device.display_name,
             Err(error) => {
                 warn!("Could not get device for notification: {error}");
@@ -329,19 +332,14 @@ impl Notifications {
             &[("name", display_name)],
         );
 
-        let payload = SessionIntent::ShowIdentityVerification(verification.key())
-            .to_variant_with_session_id(session_id);
-
         let id = format!("{session_id}//{other_device_id}//{flow_id}");
 
         Self::send_notification(
             &id,
             &title,
             &body,
-            (
+            session_id,
-                SessionIntentType::ShowIdentityVerification.app_action_name(),
+            &SessionIntent::ShowIdentityVerification(verification.key()),
-                payload.to_variant(),
-            ),
             None,
         );
 
@@ -356,7 +354,7 @@ impl Notifications {
     ///
     /// Only the notifications that were shown since the application's startup
     /// are known, older ones might still be present.
-    pub fn withdraw_all_for_room(&self, room_id: &RoomId) {
+    pub(crate) fn withdraw_all_for_room(&self, room_id: &RoomId) {
         if let Some(notifications) = self.imp().push.borrow_mut().remove(room_id) {
             let app = Application::default();
 
@@ -368,7 +366,7 @@ impl Notifications {
 
     /// Ask the system to remove the known notification for the identity
     /// verification with the given key.
-    pub fn withdraw_identity_verification(&self, key: &VerificationKey) {
+    pub(crate) fn withdraw_identity_verification(&self, key: &VerificationKey) {
         if let Some(id) = self.imp().identity_verifications.borrow_mut().remove(key) {
             let app = Application::default();
             app.withdraw_notification(&id);
@@ -379,7 +377,7 @@ impl Notifications {
     ///
     /// Only the notifications that were shown since the application's startup
     /// are known, older ones might still be present.
-    pub fn clear(&self) {
+    pub(crate) fn clear(&self) {
         let app = Application::default();
 
         for id in self.imp().push.take().values().flatten() {

src/session/model/notifications/notifications_settings.rs

@@ -12,8 +12,9 @@ use ruma::{
     push::{PredefinedOverrideRuleId, RuleKind},
     OwnedRoomId, RoomId,
 };
-use tokio::sync::broadcast::error::RecvError;
+use tokio::task::AbortHandle;
-use tracing::{error, warn};
+use tokio_stream::wrappers::BroadcastStream;
+use tracing::error;
 
 use crate::{
     session::model::{Room, Session, SessionState},
@@ -24,36 +25,34 @@ use crate::{
 #[derive(
     Debug, Default, Hash, Eq, PartialEq, Clone, Copy, glib::Enum, strum::Display, strum::EnumString,
 )]
-#[repr(u32)]
 #[enum_type(name = "NotificationsGlobalSetting")]
 #[strum(serialize_all = "kebab-case")]
 pub enum NotificationsGlobalSetting {
     /// Every message in every room.
     #[default]
-    All = 0,
+    All,
     /// Every message in 1-to-1 rooms, and mentions and keywords in every room.
-    DirectAndMentions = 1,
+    DirectAndMentions,
     /// Only mentions and keywords in every room.
-    MentionsOnly = 2,
+    MentionsOnly,
 }
 
 /// The possible values for a room notifications setting.
 #[derive(
     Debug, Default, Hash, Eq, PartialEq, Clone, Copy, glib::Enum, strum::Display, strum::EnumString,
 )]
-#[repr(u32)]
 #[enum_type(name = "NotificationsRoomSetting")]
 #[strum(serialize_all = "kebab-case")]
 pub enum NotificationsRoomSetting {
     /// Use the global setting.
     #[default]
-    Global = 0,
+    Global,
     /// All messages.
-    All = 1,
+    All,
     /// Only mentions and keywords.
-    MentionsOnly = 2,
+    MentionsOnly,
     /// No notifications.
-    Mute = 3,
+    Mute,
 }
 
 impl NotificationsRoomSetting {
@@ -88,25 +87,26 @@ mod imp {
     pub struct NotificationsSettings {
         /// The parent `Session`.
         #[property(get, set = Self::set_session, explicit_notify, nullable)]
-        pub session: glib::WeakRef<Session>,
+        session: glib::WeakRef<Session>,
         /// The SDK notification settings API.
-        pub api: RefCell<Option<MatrixNotificationSettings>>,
+        api: RefCell<Option<MatrixNotificationSettings>>,
         /// Whether notifications are enabled for this Matrix account.
         #[property(get)]
-        pub account_enabled: Cell<bool>,
+        account_enabled: Cell<bool>,
         /// Whether notifications are enabled for this session.
         #[property(get, set = Self::set_session_enabled, explicit_notify)]
-        pub session_enabled: Cell<bool>,
+        session_enabled: Cell<bool>,
         /// The global setting about which messages trigger notifications.
         #[property(get, builder(NotificationsGlobalSetting::default()))]
-        pub global_setting: Cell<NotificationsGlobalSetting>,
+        global_setting: Cell<NotificationsGlobalSetting>,
         /// The list of keywords that trigger notifications.
         #[property(get)]
-        pub keywords_list: gtk::StringList,
+        keywords_list: gtk::StringList,
         /// The map of room ID to per-room notification setting.
         ///
         /// Any room not in this map uses the global setting.
-        pub per_room_settings: RefCell<HashMap<OwnedRoomId, NotificationsRoomSetting>>,
+        per_room_settings: RefCell<HashMap<OwnedRoomId, NotificationsRoomSetting>>,
+        abort_handle: RefCell<Option<AbortHandle>>,
     }
 
     #[glib::object_subclass]
@@ -116,7 +116,13 @@ mod imp {
     }
 
     #[glib::derived_properties]
-    impl ObjectImpl for NotificationsSettings {}
+    impl ObjectImpl for NotificationsSettings {
+        fn dispose(&self) {
+            if let Some(handle) = self.abort_handle.take() {
+                handle.abort();
+            }
+        }
+    }
 
     impl NotificationsSettings {
         /// Set the parent `Session`.
@@ -140,10 +146,10 @@ mod imp {
             obj.notify_session();
 
             spawn!(clone!(
-                #[weak]
+                #[weak(rename_to = imp)]
-                obj,
+                self,
                 async move {
-                    obj.init_api().await;
+                    imp.init_api().await;
                 }
             ));
         }
@@ -163,138 +169,256 @@ mod imp {
             self.session_enabled.set(enabled);
             self.obj().notify_session_enabled();
         }
-    }
-}
-
-glib::wrapper! {
-    /// The notifications settings of a `Session`.
-    pub struct NotificationsSettings(ObjectSubclass<imp::NotificationsSettings>);
-}
 
-impl NotificationsSettings {
+        /// The SDK notification settings API.
-    /// Create a new `NotificationsSettings`.
+        pub(super) fn api(&self) -> Option<MatrixNotificationSettings> {
-    pub fn new() -> Self {
+            self.api.borrow().clone()
-        glib::Object::new()
+        }
-    }
 
-    /// The SDK notification settings API.
+        /// Initialize the SDK notification settings API.
-    fn api(&self) -> Option<MatrixNotificationSettings> {
+        async fn init_api(&self) {
-        self.imp().api.borrow().clone()
+            let Some(session) = self.session.upgrade() else {
-    }
+                self.api.take();
+                return;
+            };
 
-    /// Initialize the SDK notification settings API.
+            // If the session is not ready, there is no client so let's wait to initialize
-    async fn init_api(&self) {
+            // the API.
-        let Some(session) = self.session() else {
+            if session.state() != SessionState::Ready {
-            self.imp().api.take();
+                self.api.take();
-            return;
+
-        };
+                session.connect_ready(clone!(
+                    #[weak(rename_to = imp)]
+                    self,
+                    move |_| {
+                        spawn!(async move {
+                            imp.init_api().await;
+                        });
+                    }
+                ));
 
-        // If the session is not ready, there is no client so let's wait to initialize
+                return;
-        // the API.
+            }
-        if session.state() != SessionState::Ready {
-            self.imp().api.take();
 
-            session.connect_ready(clone!(
+            let client = session.client();
-                #[weak(rename_to = obj)]
+            let api = spawn_tokio!(async move { client.notification_settings().await })
-                self,
+                .await
-                move |_| {
+                .expect("task was not aborted");
-                    spawn!(async move {
+            let stream = BroadcastStream::new(api.subscribe_to_changes());
-                        obj.init_api().await;
-                    });
-                }
-            ));
 
-            return;
+            self.api.replace(Some(api.clone()));
-        }
 
-        let client = session.client();
+            let obj_weak = glib::SendWeakRef::from(self.obj().downgrade());
-        let api = spawn_tokio!(async move { client.notification_settings().await })
+            let fut = stream.for_each(move |res| {
-            .await
+                let obj_weak = obj_weak.clone();
-            .unwrap();
+                async move {
-        let mut api_receiver = api.subscribe_to_changes();
+                    if res.is_err() {
-
+                        return;
-        self.imp().api.replace(Some(api.clone()));
-
-        let (mut sender, mut receiver) = futures_channel::mpsc::channel(10);
-        spawn_tokio!(async move {
-            loop {
-                match api_receiver.recv().await {
-                    Ok(()) => {
-                        if let Err(error) = sender.try_send(()) {
-                            error!("Error sending notifications settings change: {error}");
-                            panic!();
-                        }
-                    }
-                    Err(RecvError::Closed) => {
-                        break;
-                    }
-                    Err(RecvError::Lagged(_)) => {
-                        warn!("Some notifications settings changes were dropped");
                     }
+
+                    let ctx = glib::MainContext::default();
+                    ctx.spawn(async move {
+                        spawn!(async move {
+                            if let Some(obj) = obj_weak.upgrade() {
+                                obj.imp().update().await;
+                            }
+                        });
+                    });
                 }
-            }
+            });
-        });
 
-        spawn!(clone!(
+            self.abort_handle
-            #[weak(rename_to = obj)]
+                .replace(Some(spawn_tokio!(fut).abort_handle()));
-            self,
-            async move {
-                obj.update().await;
-            }
-        ));
 
-        while let Some(()) = receiver.next().await {
             spawn!(clone!(
-                #[weak(rename_to = obj)]
+                #[weak(rename_to = imp)]
                 self,
                 async move {
-                    obj.update().await;
+                    imp.update().await;
                 }
             ));
         }
-    }
 
-    /// Update the notification settings from the SDK API.
+        /// Update the notification settings from the SDK API.
-    async fn update(&self) {
+        async fn update(&self) {
-        let Some(api) = self.api() else {
+            let Some(api) = self.api() else {
-            return;
+                return;
-        };
+            };
 
-        let api_clone = api.clone();
+            let api_clone = api.clone();
-        let handle = spawn_tokio!(async move {
+            let handle = spawn_tokio!(async move {
-            api_clone
+                api_clone
-                .is_push_rule_enabled(RuleKind::Override, PredefinedOverrideRuleId::Master)
+                    .is_push_rule_enabled(RuleKind::Override, PredefinedOverrideRuleId::Master)
+                    .await
+            });
+
+            let account_enabled = match handle.await.expect("task was not aborted") {
+                // The rule disables notifications, so we need to invert the boolean.
+                Ok(enabled) => !enabled,
+                Err(error) => {
+                    error!("Could not get account notifications setting: {error}");
+                    true
+                }
+            };
+            self.set_account_enabled(account_enabled);
+
+            if default_rooms_notifications_is_all(api.clone(), false).await {
+                self.set_global_setting(NotificationsGlobalSetting::All);
+            } else if default_rooms_notifications_is_all(api, true).await {
+                self.set_global_setting(NotificationsGlobalSetting::DirectAndMentions);
+            } else {
+                self.set_global_setting(NotificationsGlobalSetting::MentionsOnly);
+            }
+
+            self.update_keywords_list().await;
+            self.update_per_room_settings().await;
+        }
+
+        /// Set whether notifications are enabled for this session.
+        pub(super) fn set_account_enabled(&self, enabled: bool) {
+            if self.account_enabled.get() == enabled {
+                return;
+            }
+
+            self.account_enabled.set(enabled);
+            self.obj().notify_account_enabled();
+        }
+
+        /// Set the global setting about which messages trigger notifications.
+        pub(super) fn set_global_setting(&self, setting: NotificationsGlobalSetting) {
+            if self.global_setting.get() == setting {
+                return;
+            }
+
+            self.global_setting.set(setting);
+            self.obj().notify_global_setting();
+        }
+
+        /// Update the local list of keywords with the remote one.
+        pub(super) async fn update_keywords_list(&self) {
+            let Some(api) = self.api() else {
+                return;
+            };
+
+            let keywords = spawn_tokio!(async move { api.enabled_keywords().await })
                 .await
-        });
+                .expect("task was not aborted");
+
+            let list = &self.keywords_list;
+            let mut diverges_at = None;
+
+            let keywords = keywords.iter().map(String::as_str).collect::<Vec<_>>();
+            let new_len = keywords.len() as u32;
+            let old_len = list.n_items();
+
+            // Check if there is any keyword that changed, was moved or was added.
+            for (pos, keyword) in keywords.iter().enumerate() {
+                if Some(*keyword)
+                    != list
+                        .item(pos as u32)
+                        .and_downcast::<gtk::StringObject>()
+                        .map(|o| o.string())
+                        .as_deref()
+                {
+                    diverges_at = Some(pos as u32);
+                    break;
+                }
+            }
 
-        let account_enabled = match handle.await.unwrap() {
+            // Check if keywords were removed.
-            // The rule disables notifications, so we need to invert the boolean.
+            if diverges_at.is_none() && old_len > new_len {
-            Ok(enabled) => !enabled,
+                diverges_at = Some(new_len);
-            Err(error) => {
-                error!("Could not get account notifications setting: {error}");
-                true
             }
-        };
-        self.set_account_enabled_inner(account_enabled);
 
-        if default_rooms_notifications_is_all(api.clone(), false).await {
+            let Some(pos) = diverges_at else {
-            self.set_global_setting_inner(NotificationsGlobalSetting::All);
+                // Nothing to do.
-        } else if default_rooms_notifications_is_all(api, true).await {
+                return;
-            self.set_global_setting_inner(NotificationsGlobalSetting::DirectAndMentions);
+            };
-        } else {
+
-            self.set_global_setting_inner(NotificationsGlobalSetting::MentionsOnly);
+            let additions = &keywords[pos as usize..];
+            list.splice(pos, old_len.saturating_sub(pos), additions);
         }
 
-        self.update_keywords_list().await;
+        /// Update the local list of per-room settings with the remote one.
-        self.update_per_room_settings().await;
+        pub(super) async fn update_per_room_settings(&self) {
+            let Some(api) = self.api() else {
+                return;
+            };
+
+            let api_clone = api.clone();
+            let room_ids = spawn_tokio!(async move {
+                api_clone
+                    .get_rooms_with_user_defined_rules(Some(true))
+                    .await
+            })
+            .await
+            .expect("task was not aborted");
+
+            // Update the local map.
+            let mut per_room_settings = HashMap::with_capacity(room_ids.len());
+            for room_id in room_ids {
+                let Ok(room_id) = RoomId::parse(room_id) else {
+                    continue;
+                };
+
+                let room_id_clone = room_id.clone();
+                let api_clone = api.clone();
+                let handle = spawn_tokio!(async move {
+                    api_clone
+                        .get_user_defined_room_notification_mode(&room_id_clone)
+                        .await
+                });
+
+                if let Some(setting) = handle.await.expect("task was not aborted") {
+                    per_room_settings.insert(room_id, setting.into());
+                }
+            }
+
+            self.per_room_settings.replace(per_room_settings.clone());
+
+            // Update the setting in the rooms.
+            // Since we don't know when a room was added or removed, we have to update every
+            // room.
+            let Some(session) = self.session.upgrade() else {
+                return;
+            };
+            let room_list = session.room_list();
+
+            for room in room_list.iter::<Room>() {
+                let Ok(room) = room else {
+                    // Returns an error when the list changed, just stop.
+                    break;
+                };
+
+                if let Some(setting) = per_room_settings.get(room.room_id()) {
+                    room.set_notifications_setting(*setting);
+                } else {
+                    room.set_notifications_setting(NotificationsRoomSetting::Global);
+                }
+            }
+        }
+    }
+}
+
+glib::wrapper! {
+    /// The notifications settings of a `Session`.
+    pub struct NotificationsSettings(ObjectSubclass<imp::NotificationsSettings>);
+}
+
+impl NotificationsSettings {
+    /// Create a new `NotificationsSettings`.
+    pub fn new() -> Self {
+        glib::Object::new()
     }
 
     /// Set whether notifications are enabled for this session.
-    pub async fn set_account_enabled(
+    pub(crate) async fn set_account_enabled(
         &self,
         enabled: bool,
     ) -> Result<(), NotificationSettingsError> {
-        let Some(api) = self.api() else {
+        let imp = self.imp();
+
+        let Some(api) = imp.api() else {
             error!("Cannot update notifications settings when API is not initialized");
             return Err(NotificationSettingsError::UnableToUpdatePushRule);
         };
@@ -309,9 +433,9 @@ impl NotificationsSettings {
             .await
         });
 
-        match handle.await.unwrap() {
+        match handle.await.expect("task was not aborted") {
             Ok(()) => {
-                self.set_account_enabled_inner(enabled);
+                imp.set_account_enabled(enabled);
                 Ok(())
             }
             Err(error) => {
@@ -321,21 +445,14 @@ impl NotificationsSettings {
         }
     }
 
-    fn set_account_enabled_inner(&self, enabled: bool) {
-        if self.account_enabled() == enabled {
-            return;
-        }
-
-        self.imp().account_enabled.set(enabled);
-        self.notify_account_enabled();
-    }
-
     /// Set the global setting about which messages trigger notifications.
-    pub async fn set_global_setting(
+    pub(crate) async fn set_global_setting(
         &self,
         setting: NotificationsGlobalSetting,
     ) -> Result<(), NotificationSettingsError> {
-        let Some(api) = self.api() else {
+        let imp = self.imp();
+
+        let Some(api) = imp.api() else {
             error!("Cannot update notifications settings when API is not initialized");
             return Err(NotificationSettingsError::UnableToUpdatePushRule);
         };
@@ -356,68 +473,19 @@ impl NotificationsSettings {
             return Err(error);
         }
 
-        self.set_global_setting_inner(setting);
+        imp.set_global_setting(setting);
 
         Ok(())
     }
 
-    fn set_global_setting_inner(&self, setting: NotificationsGlobalSetting) {
-        if self.global_setting() == setting {
-            return;
-        }
-
-        self.imp().global_setting.set(setting);
-        self.notify_global_setting();
-    }
-
-    /// Update the local list of keywords with the remote one.
-    async fn update_keywords_list(&self) {
-        let Some(api) = self.api() else {
-            return;
-        };
-
-        let keywords = spawn_tokio!(async move { api.enabled_keywords().await })
-            .await
-            .unwrap();
-
-        let list = &self.imp().keywords_list;
-        let mut diverges_at = None;
-
-        let keywords = keywords.iter().map(String::as_str).collect::<Vec<_>>();
-        let new_len = keywords.len() as u32;
-        let old_len = list.n_items();
-
-        // Check if there is any keyword that changed, was moved or was added.
-        for (pos, keyword) in keywords.iter().enumerate() {
-            if Some(*keyword)
-                != list
-                    .item(pos as u32)
-                    .and_downcast::<gtk::StringObject>()
-                    .map(|o| o.string())
-                    .as_deref()
-            {
-                diverges_at = Some(pos as u32);
-                break;
-            }
-        }
-
-        // Check if keywords were removed.
-        if diverges_at.is_none() && old_len > new_len {
-            diverges_at = Some(new_len);
-        }
-
-        let Some(pos) = diverges_at else {
-            // Nothing to do.
-            return;
-        };
-
-        let additions = &keywords[pos as usize..];
-        list.splice(pos, old_len.saturating_sub(pos), additions);
-    }
-
     /// Remove a keyword from the list.
-    pub async fn remove_keyword(&self, keyword: String) -> Result<(), NotificationSettingsError> {
+    pub(crate) async fn remove_keyword(
-        let Some(api) = self.api() else {
+        &self,
+        keyword: String,
+    ) -> Result<(), NotificationSettingsError> {
+        let imp = self.imp();
+
+        let Some(api) = imp.api() else {
             error!("Cannot update notifications settings when API is not initialized");
             return Err(NotificationSettingsError::UnableToUpdatePushRule);
         };
@@ -425,19 +493,24 @@ impl NotificationsSettings {
         let keyword_clone = keyword.clone();
         let handle = spawn_tokio!(async move { api.remove_keyword(&keyword_clone).await });
 
-        if let Err(error) = handle.await.unwrap() {
+        if let Err(error) = handle.await.expect("task was not aborted") {
             error!("Could not remove notification keyword `{keyword}`: {error}");
             return Err(error);
         }
 
-        self.update_keywords_list().await;
+        imp.update_keywords_list().await;
 
         Ok(())
     }
 
     /// Add a keyword to the list.
-    pub async fn add_keyword(&self, keyword: String) -> Result<(), NotificationSettingsError> {
+    pub(crate) async fn add_keyword(
-        let Some(api) = self.api() else {
+        &self,
+        keyword: String,
+    ) -> Result<(), NotificationSettingsError> {
+        let imp = self.imp();
+
+        let Some(api) = imp.api() else {
             error!("Cannot update notifications settings when API is not initialized");
             return Err(NotificationSettingsError::UnableToUpdatePushRule);
         };
@@ -445,84 +518,25 @@ impl NotificationsSettings {
         let keyword_clone = keyword.clone();
         let handle = spawn_tokio!(async move { api.add_keyword(keyword_clone).await });
 
-        if let Err(error) = handle.await.unwrap() {
+        if let Err(error) = handle.await.expect("task was not aborted") {
             error!("Could not add notification keyword `{keyword}`: {error}");
             return Err(error);
         }
 
-        self.update_keywords_list().await;
+        imp.update_keywords_list().await;
 
         Ok(())
     }
 
-    /// Update the local list of per-room settings with the remote one.
-    async fn update_per_room_settings(&self) {
-        let Some(api) = self.api() else {
-            return;
-        };
-
-        let api_clone = api.clone();
-        let room_ids = spawn_tokio!(async move {
-            api_clone
-                .get_rooms_with_user_defined_rules(Some(true))
-                .await
-        })
-        .await
-        .unwrap();
-
-        // Update the local map.
-        let mut per_room_settings = HashMap::with_capacity(room_ids.len());
-        for room_id in room_ids {
-            let Ok(room_id) = RoomId::parse(room_id) else {
-                continue;
-            };
-
-            let room_id_clone = room_id.clone();
-            let api_clone = api.clone();
-            let handle = spawn_tokio!(async move {
-                api_clone
-                    .get_user_defined_room_notification_mode(&room_id_clone)
-                    .await
-            });
-
-            if let Some(setting) = handle.await.unwrap() {
-                per_room_settings.insert(room_id, setting.into());
-            }
-        }
-
-        self.imp()
-            .per_room_settings
-            .replace(per_room_settings.clone());
-
-        // Update the setting in the rooms.
-        // Since we don't know when a room was added or removed, we have to update every
-        // room.
-        let Some(session) = self.session() else {
-            return;
-        };
-        let room_list = session.room_list();
-
-        for room in room_list.iter::<Room>() {
-            let Ok(room) = room else {
-                // Returns an error when the list changed, just stop.
-                break;
-            };
-
-            if let Some(setting) = per_room_settings.get(room.room_id()) {
-                room.set_notifications_setting(*setting);
-            } else {
-                room.set_notifications_setting(NotificationsRoomSetting::Global);
-            }
-        }
-    }
-
     /// Set the notification setting for the room with the given ID.
-    pub async fn set_per_room_setting(
+    pub(crate) async fn set_per_room_setting(
         &self,
         room_id: OwnedRoomId,
         setting: NotificationsRoomSetting,
     ) -> Result<(), NotificationSettingsError> {
-        let Some(api) = self.api() else {
+        let imp = self.imp();
+
+        let Some(api) = imp.api() else {
             error!("Cannot update notifications settings when API is not initialized");
             return Err(NotificationSettingsError::UnableToUpdatePushRule);
         };
@@ -534,12 +548,12 @@ impl NotificationsSettings {
             spawn_tokio!(async move { api.delete_user_defined_room_rules(&room_id_clone).await })
         };
 
-        if let Err(error) = handle.await.unwrap() {
+        if let Err(error) = handle.await.expect("task was not aborted") {
             error!("Could not update notifications setting for room `{room_id}`: {error}");
             return Err(error);
         }
 
-        self.update_per_room_settings().await;
+        imp.update_per_room_settings().await;
 
         Ok(())
     }
@@ -560,7 +574,7 @@ async fn default_rooms_notifications_is_all(
             .await
     })
     .await
-    .unwrap();
+    .expect("task was not aborted");
 
     mode == RoomNotificationMode::AllMessages
 }
@@ -581,5 +595,5 @@ async fn set_default_rooms_notifications_all(
             .await
     })
     .await
-    .unwrap()
+    .expect("task was not aborted")
 }

src/session/model/verification/identity_verification.rs

@@ -422,7 +422,7 @@ impl IdentityVerification {
     }
 
     /// The unique identifying key of this verification.
-    pub fn key(&self) -> VerificationKey {
+    pub(crate) fn key(&self) -> VerificationKey {
         VerificationKey::from_request(self.request())
     }
 

src/session/model/verification/mod.rs

@@ -1,10 +1,11 @@
+use gtk::{glib, prelude::*};
 use matrix_sdk::encryption::verification::VerificationRequest;
-use ruma::{events::key::verification::VerificationMethod, OwnedUserId};
+use ruma::{events::key::verification::VerificationMethod, OwnedUserId, UserId};
 
 mod identity_verification;
 mod verification_list;
 
-pub use self::{
+pub(crate) use self::{
     identity_verification::{
         IdentityVerification, VerificationState, VerificationSupportedMethods,
     },
@@ -14,21 +15,21 @@ use crate::{components::Camera, prelude::*};
 
 /// A unique key to identify an identity verification.
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
-pub struct VerificationKey {
+pub(crate) struct VerificationKey {
     /// The ID of the user being verified.
-    pub user_id: OwnedUserId,
+    pub(crate) user_id: OwnedUserId,
     /// The ID of the verification.
-    pub flow_id: String,
+    pub(crate) flow_id: String,
 }
 
 impl VerificationKey {
     /// Create a new `VerificationKey` with the given user ID and flow ID.
-    pub fn new(user_id: OwnedUserId, flow_id: String) -> Self {
+    pub(crate) fn new(user_id: OwnedUserId, flow_id: String) -> Self {
         Self { user_id, flow_id }
     }
 
     /// Create a new `VerificationKey` from the given [`VerificationRequest`].
-    pub fn from_request(request: &VerificationRequest) -> Self {
+    pub(crate) fn from_request(request: &VerificationRequest) -> Self {
         Self::new(
             request.other_user_id().to_owned(),
             request.flow_id().to_owned(),
@@ -36,6 +37,26 @@ impl VerificationKey {
     }
 }
 
+impl StaticVariantType for VerificationKey {
+    fn static_variant_type() -> std::borrow::Cow<'static, glib::VariantTy> {
+        <(String, String)>::static_variant_type()
+    }
+}
+
+impl ToVariant for VerificationKey {
+    fn to_variant(&self) -> glib::Variant {
+        (self.user_id.as_str(), self.flow_id.as_str()).to_variant()
+    }
+}
+
+impl FromVariant for VerificationKey {
+    fn from_variant(variant: &glib::Variant) -> Option<Self> {
+        let (user_id_str, flow_id) = variant.get::<(String, String)>()?;
+        let user_id = UserId::parse(user_id_str).ok()?;
+        Some(Self { user_id, flow_id })
+    }
+}
+
 /// Load the supported verification methods on this system.
 async fn load_supported_verification_methods() -> Vec<VerificationMethod> {
     let mut methods = vec![

src/session/model/verification/verification_list.rs

@@ -29,7 +29,7 @@ mod imp {
     #[properties(wrapper_type = super::VerificationList)]
     pub struct VerificationList {
         /// The ongoing verification requests.
-        pub list: RefCell<IndexMap<VerificationKey, IdentityVerification>>,
+        pub(super) list: RefCell<IndexMap<VerificationKey, IdentityVerification>>,
         /// The current session.
         #[property(get, construct_only)]
         pub session: glib::WeakRef<Session>,
@@ -282,7 +282,7 @@ impl VerificationList {
     }
 
     /// Remove the verification with the given key.
-    pub fn remove(&self, key: &VerificationKey) {
+    pub(crate) fn remove(&self, key: &VerificationKey) {
         let Some((pos, ..)) = self.imp().list.borrow_mut().shift_remove_full(key) else {
             return;
         };
@@ -295,7 +295,7 @@ impl VerificationList {
     }
 
     /// Get the verification with the given key.
-    pub fn get(&self, key: &VerificationKey) -> Option<IdentityVerification> {
+    pub(crate) fn get(&self, key: &VerificationKey) -> Option<IdentityVerification> {
         self.imp().list.borrow().get(key).cloned()
     }
 

src/window.rs

@@ -235,7 +235,6 @@ mod imp {
     }
 
     impl WindowImpl for Window {
-        // save window state on delete event
         fn close_request(&self) -> glib::Propagation {
             if let Err(error) = self.save_window_size() {
                 warn!("Could not save window state: {error}");