refactor App and add WeakApp

This cleans up App and addes the possibility to have a weak reference to
the App. It's heavily inspired by the example application written by
Slomo: https://github.com/sdroege/rustfest-rome18-gtk-gst-workshop

fractal-gtk/src/actions/global.rs

@@ -8,8 +8,7 @@ use gtk::prelude::*;
 
 /* This creates globale actions which are connected to the application */
 /* TODO: Remove op */
-pub fn new(op: Arc<Mutex<AppOp>>) {
-    let app = op.lock().unwrap().gtk_app.clone();
+pub fn new(app: &gtk::Application, op: &Arc<Mutex<AppOp>>) {
     let settings = SimpleAction::new("settings", None);
     let account = SimpleAction::new("account_settings", None);
     let dir = SimpleAction::new("directory", None);
@@ -23,10 +22,11 @@ pub fn new(op: Arc<Mutex<AppOp>>) {
     let search = SimpleAction::new("search", None);
     let leave = SimpleAction::new("leave_room", None);
 
-    let quit = SimpleAction::new("quit", None);
     let shortcuts = SimpleAction::new("shortcuts", None);
     let about = SimpleAction::new("about", None);
+    let quit = gio::SimpleAction::new("quit", None);
 
+    let close_room = SimpleAction::new("close-room", None);
     let open_room = SimpleAction::new("open-room", glib::VariantTy::new("s").ok());
 
     app.add_action(&settings);
@@ -46,8 +46,15 @@ pub fn new(op: Arc<Mutex<AppOp>>) {
     app.add_action(&shortcuts);
     app.add_action(&about);
     app.add_action(&open_room);
+    app.add_action(&close_room);
+
+    // When activated, shuts down the application
+    let app_weak = app.downgrade();
+    quit.connect_activate(move |_action, _parameter| {
+        let app = upgrade_weak!(app_weak);
+        app.quit();
+    });
 
-    quit.connect_activate(clone!(op => move |_, _| op.lock().unwrap().quit() ));
     about.connect_activate(clone!(op => move |_, _| op.lock().unwrap().about_dialog() ));
 
     settings.connect_activate(move |_, _| {
@@ -80,8 +87,17 @@ pub fn new(op: Arc<Mutex<AppOp>>) {
         }
     }));
 
+    close_room.connect_activate(clone!(op => move |_, _| {
+        op.lock().unwrap().escape();
+    }));
+
     /* Add Keybindings to actions */
     app.set_accels_for_action("app.quit", &["<Ctrl>Q"]);
+    app.set_accels_for_action("app.close-room", &["Escape"]);
+    app.set_accels_for_action("app.notification", &["<Ctrl>N"]);
+
+    // TODO: Mark active room as read when window gets focus
+    //op.lock().unwrap().mark_active_room_messages();
 }
 
 fn get_room_id(data: &Option<glib::Variant>) -> Option<&str> {

fractal-gtk/src/app/connect/mod.rs

@@ -1,7 +1,3 @@
-use gdk;
-use gtk;
-use gtk::prelude::*;
-
 mod account;
 mod attach;
 mod autocomplete;
@@ -18,53 +14,16 @@ mod roomlist_search;
 mod send;
 mod stickers;
 
-use actions;
 use app::App;
 
 impl App {
     pub fn connect_gtk(&self) {
-        // Set up shutdown callback
-        let window: gtk::Window = self
-            .ui
-            .builder
-            .get_object("main_window")
-            .expect("Couldn't find main_window in ui file.");
-
-        window.set_title("Fractal");
-        window.show_all();
-
-        let op = self.op.clone();
-        window.connect_delete_event(move |_, _| {
-            op.lock().unwrap().quit();
-            Inhibit(false)
-        });
-
-        let op = self.op.clone();
-        let main_window = self
-            .ui
-            .builder
-            .get_object::<gtk::ApplicationWindow>("main_window")
-            .expect("Cant find main_window in ui file.");
-        main_window.connect_key_press_event(move |w, k| match k.get_keyval() {
-            gdk::enums::key::Escape => Inhibit(op.lock().unwrap().escape(w)),
-            _ => Inhibit(false),
-        });
-
-        let op = self.op.clone();
-        window.connect_property_has_toplevel_focus_notify(move |w| {
-            if !w.is_active() {
-                op.lock().unwrap().mark_active_room_messages();
-            }
-        });
-
-        actions::Global::new(self.op.clone());
         self.connect_headerbars();
         self.connect_login_view();
 
         self.connect_send();
         self.connect_attach();
         self.connect_markdown();
-        //self.connect_stickers();
         self.connect_autocomplete();
 
         self.connect_directory();

fractal-gtk/src/app/mod.rs

@@ -1,11 +1,11 @@
-use gdk;
 use gettextrs::{bindtextdomain, setlocale, textdomain, LocaleCategory};
-use gio;
-use gio::ApplicationExt;
-use gio::ApplicationExtManual;
-use glib;
+use gio::prelude::*;
 use gtk;
 use gtk::prelude::*;
+use std::cell::RefCell;
+use std::error::Error;
+use std::ops;
+use std::rc::{Rc, Weak};
 use std::sync::mpsc::channel;
 use std::sync::mpsc::{Receiver, Sender};
 use std::sync::{Arc, Mutex};
@@ -14,6 +14,7 @@ use appop::AppOp;
 use backend::BKResponse;
 use backend::Backend;
 
+use actions;
 use globals;
 use uibuilder;
 
@@ -23,16 +24,16 @@ static mut OP: Option<Arc<Mutex<AppOp>>> = None;
 #[macro_export]
 macro_rules! APPOP {
     ($fn: ident, ($($x:ident),*) ) => {{
-        let ctx = glib::MainContext::default();
-        ctx.invoke(move || {
-            $( let $x = $x.clone(); )*
-            if let Some(op) = App::get_op() {
-                op.lock().unwrap().$fn($($x),*);
-            }
-        });
+	let ctx = glib::MainContext::default();
+	ctx.invoke(move || {
+	    $( let $x = $x.clone(); )*
+		if let Some(op) = App::get_op() {
+		    op.lock().unwrap().$fn($($x),*);
+		}
+	});
     }};
     ($fn: ident) => {{
-        APPOP!($fn, ( ) );
+	APPOP!($fn, ( ) );
     }}
 }
 
@@ -40,109 +41,172 @@ mod backend_loop;
 
 pub use self::backend_loop::backend_loop;
 
-/// State for the main thread.
-///
-/// It takes care of starting up the application and for loading and accessing the
-/// UI.
-pub struct App {
+// Our refcounted application struct for containing all the state we have to carry around.
+// TODO: subclass gtk::Application once possible
+pub struct App(Rc<AppInner>);
+
+pub struct AppInner {
+    main_window: gtk::ApplicationWindow,
+    /* Add widget directly here in place of uibuilder::UI*/
     ui: uibuilder::UI,
 
+    // TODO: Remove op needed in connect, but since it is global we could remove it form here
     op: Arc<Mutex<AppOp>>,
 }
 
-impl App {
-    /// Create an App instance
-    pub fn new() {
-        let appid = globals::APP_ID
-            .unwrap_or("org.gnome.FractalDevel")
-            .to_string();
-
-        let gtk_app = gtk::Application::new(Some(&appid[..]), gio::ApplicationFlags::empty())
-            .expect("Failed to initialize GtkApplication");
-
-        let path = "/org/gnome/Fractal".to_string();
-        gtk_app.set_property_resource_base_path(Some(&path));
-
-        gtk_app.connect_startup(move |gtk_app| {
-            let (tx, rx): (Sender<BKResponse>, Receiver<BKResponse>) = channel();
-
-            let bk = Backend::new(tx);
-            let apptx = bk.run();
-
-            // Set up the textdomain for gettext
-            setlocale(LocaleCategory::LcAll, "");
-            bindtextdomain("fractal", globals::LOCALEDIR.unwrap_or("./fractal-gtk/po"));
-            textdomain("fractal");
-
-            let ui = uibuilder::UI::new();
-            let window: gtk::Window = ui
-                .builder
-                .get_object("main_window")
-                .expect("Couldn't find main_window in ui file.");
-            window.set_application(gtk_app);
-
-            if appid.ends_with("Devel") {
-                window.get_style_context().map(|c| c.add_class("devel"));
-            }
+// Deref into the contained struct to make usage a bit more ergonomic
+impl ops::Deref for App {
+    type Target = AppInner;
 
-            let stack = ui
-                .builder
-                .get_object::<gtk::Stack>("main_content_stack")
-                .expect("Can't find main_content_stack in ui file.");
-            let stack_header = ui
-                .builder
-                .get_object::<gtk::Stack>("headerbar_stack")
-                .expect("Can't find headerbar_stack in ui file.");
-
-            /* Add account settings view to the main stack */
-            let child = ui
-                .builder
-                .get_object::<gtk::Box>("account_settings_box")
-                .expect("Can't find account_settings_box in ui file.");
-            let child_header = ui
-                .builder
-                .get_object::<gtk::Box>("account_settings_headerbar")
-                .expect("Can't find account_settings_headerbar in ui file.");
-            stack.add_named(&child, "account-settings");
-            stack_header.add_named(&child_header, "account-settings");
-
-            let op = Arc::new(Mutex::new(AppOp::new(gtk_app.clone(), ui.clone(), apptx)));
-
-            unsafe {
-                OP = Some(op.clone());
-            }
-
-            backend_loop(rx);
+    fn deref(&self) -> &AppInner {
+        &*self.0
+    }
+}
 
-            let app = App {
-                ui: ui,
-                op: op.clone(),
-            };
+// Weak reference to our application struct
+pub struct AppWeak(Weak<AppInner>);
 
-            gtk_app.connect_activate(move |_| op.lock().unwrap().activate());
+impl AppWeak {
+    // Upgrade to a strong reference if it still exists
+    pub fn upgrade(&self) -> Option<App> {
+        self.0.upgrade().map(App)
+    }
+}
 
-            app.connect_gtk();
-            app.run();
-        });
+impl App {
+    pub fn new(gtk_app: &gtk::Application) -> Result<App, Box<dyn Error>> {
+        let (tx, rx): (Sender<BKResponse>, Receiver<BKResponse>) = channel();
 
-        gtk_app.run(&[]);
-    }
+        let bk = Backend::new(tx);
+        let apptx = bk.run();
 
-    pub fn run(&self) {
-        self.op.lock().unwrap().init();
+        // Set up the textdomain for gettext
+        setlocale(LocaleCategory::LcAll, "");
+        bindtextdomain("fractal", globals::LOCALEDIR.unwrap_or("./fractal-gtk/po"));
+        textdomain("fractal");
 
         glib::set_application_name("fractal");
         glib::set_prgname(Some("fractal"));
 
+        // Add style provider
         let provider = gtk::CssProvider::new();
         provider.load_from_resource("/org/gnome/Fractal/app.css");
         gtk::StyleContext::add_provider_for_screen(
-            &gdk::Screen::get_default().unwrap(),
+            &gdk::Screen::get_default().expect("Error initializing gtk css provider."),
             &provider,
-            600,
+            gtk::STYLE_PROVIDER_PRIORITY_APPLICATION,
         );
+
+        let ui = uibuilder::UI::new();
+        let window: gtk::ApplicationWindow = ui
+            .builder
+            .get_object("main_window")
+            .expect("Couldn't find main_window in ui file.");
+        window.set_application(gtk_app);
+
+        window.set_title("Fractal");
+        window.show_all();
+
+        // TODO: set style for development e.g. appid.ends_with("Devel")
+        // window.get_style_context().map(|c| c.add_class("devel"));
+
+        let stack = ui
+            .builder
+            .get_object::<gtk::Stack>("main_content_stack")
+            .expect("Can't find main_content_stack in ui file.");
+        let stack_header = ui
+            .builder
+            .get_object::<gtk::Stack>("headerbar_stack")
+            .expect("Can't find headerbar_stack in ui file.");
+
+        /* Add account settings view to the main stack */
+        let child = ui
+            .builder
+            .get_object::<gtk::Box>("account_settings_box")
+            .expect("Can't find account_settings_box in ui file.");
+        let child_header = ui
+            .builder
+            .get_object::<gtk::Box>("account_settings_headerbar")
+            .expect("Can't find account_settings_headerbar in ui file.");
+        stack.add_named(&child, "account-settings");
+        stack_header.add_named(&child_header, "account-settings");
+
+        let op = Arc::new(Mutex::new(AppOp::new(ui.clone(), apptx)));
+
+        unsafe {
+            OP = Some(op.clone());
+        }
+
+        backend_loop(rx);
+
+        actions::Global::new(gtk_app, &op);
+
+        let app = App(Rc::new(AppInner {
+            main_window: window,
+            ui,
+            op,
+        }));
+
+        app.connect_gtk();
+
+        Ok(app)
+    }
+
+    // Downgrade to a weak reference
+    pub fn downgrade(&self) -> AppWeak {
+        AppWeak(Rc::downgrade(&self.0))
+    }
+
+    pub fn on_startup(gtk_app: &gtk::Application) {
+        // Create application and error out if that fails for whatever reason
+        let app = match App::new(gtk_app) {
+            Ok(app) => app,
+            Err(err) => {
+                let msg = format!("Error creating application: {}", err);
+                println!("{}", msg);
+                // TODO: Show error dialog
+                // https://github.com/sdroege/rustfest-rome18-gtk-gst-workshop/blob/master/src/utils.rs#L52
+                return;
+            }
+        };
+
+        let app_weak = app.downgrade();
+        gtk_app.connect_activate(move |_| {
+            let app = upgrade_weak!(app_weak);
+            app.on_activate();
+        });
+
+        let app_weak = app.downgrade();
+        app.main_window
+            .connect_property_has_toplevel_focus_notify(move |_| {
+                let app = upgrade_weak!(app_weak);
+                app.op.lock().unwrap().mark_active_room_messages();
+            });
+
+        app.op.lock().unwrap().init();
+
+        // When the application is shut down we drop our app struct
+        let app_container = RefCell::new(Some(app));
+        gtk_app.connect_shutdown(move |_| {
+            let app = app_container
+                .borrow_mut()
+                .take()
+                .expect("Shutdown called multiple times");
+            app.on_shutdown();
+        });
+    }
+
+    fn on_activate(&self) {
+        self.main_window.show();
+        self.main_window.present();
+    }
+
+    fn on_shutdown(self) {
+        self.op.lock().unwrap().quit();
     }
 
+    // Legazy function to get AppOp
+    // This shouldn't be used in new code
     pub fn get_op() -> Option<Arc<Mutex<AppOp>>> {
         unsafe {
             match OP {

fractal-gtk/src/appop/message.rs

@@ -348,7 +348,16 @@ impl AppOp {
                     || self.rooms.get(&msg.room).map_or(false, |r| r.direct));
 
             if should_notify {
-                self.notify(msg);
+                if let Some(ref id) = msg.id {
+                    let window: gtk::Window = self
+                        .ui
+                        .builder
+                        .get_object("main_window")
+                        .expect("Can't find main_window in ui file.");
+                    if let Some(app) = window.get_application() {
+                        self.notify(app, &msg.room, id);
+                    }
+                }
             }
 
             if &msg.room == self.active_room.as_ref()? && !msg.redacted {

fractal-gtk/src/appop/mod.rs

@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 use std::sync::mpsc::Sender;
 
-use gio::ApplicationExt;
 use gtk;
 use gtk::prelude::*;
 
@@ -46,7 +45,6 @@ pub use self::state::AppState;
 
 pub struct AppOp {
     pub ui: uibuilder::UI,
-    pub gtk_app: gtk::Application,
     pub backend: Sender<backend::BKCommand>,
 
     pub syncing: bool,
@@ -87,10 +85,9 @@ pub struct AppOp {
 impl PasswordStorage for AppOp {}
 
 impl AppOp {
-    pub fn new(app: gtk::Application, ui: uibuilder::UI, tx: Sender<BKCommand>) -> AppOp {
+    pub fn new(ui: uibuilder::UI, tx: Sender<BKCommand>) -> AppOp {
         AppOp {
             ui: ui,
-            gtk_app: app,
             backend: tx,
             active_room: None,
             rooms: HashMap::new(),
@@ -162,6 +159,5 @@ impl AppOp {
     pub fn quit(&self) {
         self.cache_rooms();
         self.disconnect();
-        self.gtk_app.quit();
     }
 }

fractal-gtk/src/appop/notify.rs

@@ -12,7 +12,6 @@ use i18n::i18n;
 use appop::AppOp;
 use backend::BKCommand;
 
-use types::Message;
 use widgets::ErrorDialog;
 
 impl AppOp {
@@ -40,10 +39,9 @@ impl AppOp {
         inapp.set_reveal_child(false);
     }
 
-    pub fn notify(&self, msg: &Message) -> Option<()> {
-        let id = msg.id.clone()?;
-        let room_id = msg.room.clone();
-        let r = self.rooms.get(&msg.room)?;
+    pub fn notify(&self, app: gtk::Application, room_id: &str, id: &str) -> Option<()> {
+        let msg = self.get_message_by_id(room_id, id)?;
+        let r = self.rooms.get(room_id)?;
         let mut body = msg.body.clone();
         body.truncate(80);
 
@@ -62,7 +60,9 @@ impl AppOp {
             .backend
             .send(BKCommand::GetUserInfoAsync(msg.sender.clone(), Some(tx)));
 
-        let app_weak = self.gtk_app.downgrade();
+        let room_id = room_id.to_string();
+        let id = id.to_string();
+        let app_weak = app.downgrade();
         gtk::timeout_add(50, move || match rx.try_recv() {
             Err(TryRecvError::Empty) => gtk::Continue(true),
             Err(TryRecvError::Disconnected) => gtk::Continue(false),
@@ -74,6 +74,7 @@ impl AppOp {
                 gtk::Continue(false)
             }
         });
+
         None
     }
 

fractal-gtk/src/appop/state.rs

@@ -80,7 +80,7 @@ impl AppOp {
         }
     }
 
-    pub fn escape(&mut self, _w: &gtk::ApplicationWindow) -> bool {
+    pub fn escape(&mut self) -> bool {
         if self.inhibit_escape {
             return true;
         }

fractal-gtk/src/main.rs

@@ -69,10 +69,30 @@ mod widgets;
 
 mod appop;
 
+use std::env::args;
+use std::error::Error;
+
 use app::App;
+use gio::ApplicationExt;
+use gio::ApplicationExtManual;
 
-fn main() {
+fn main() -> Result<(), Box<dyn Error>> {
     static_resources::init().expect("GResource initialization failed.");
-    gst::init().expect("Error initializing gstreamer");
-    App::new();
+
+    // Initialize GStreamer. This checks, among other things, what plugins are available
+    gst::init()?;
+
+    // Create a Application with default flags
+    let appid = globals::APP_ID.unwrap_or("org.gnome.FractalDevel");
+    let application = gtk::Application::new(appid, gio::ApplicationFlags::empty())?;
+
+    application.set_property_resource_base_path(Some("/org/gnome/Fractal"));
+
+    application.connect_startup(|application| {
+        App::on_startup(application);
+    });
+
+    application.run(&args().collect::<Vec<_>>());
+
+    Ok(())
 }