Refactor GTK Application to make use of the new structure. (#892)

CHANGELOG.md

@@ -95,6 +95,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
 - Enabled Clippy checks for all targets. ([#850] by [@xStrom])
 - Added rendering tests. ([#784] by [@fishrockz])
 - Revamped CI testing to optimize coverage and speed. ([#857] by [@xStrom])
+- GTK: Refactored `Application` to use the new structure. ([#892] by [@xStrom])
 - X11: Refactored `Application` to use the new structure. ([#894] by [@xStrom])
 - X11: Refactored `Window` to support some reentrancy and invalidation. ([#894] by [@xStrom])
 
@@ -145,6 +146,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
 [#878]: https://github.com/xi-editor/druid/pull/878
 [#880]: https://github.com/xi-editor/druid/pull/880
 [#889]: https://github.com/xi-editor/druid/pull/889
+[#892]: https://github.com/xi-editor/druid/pull/892
 [#894]: https://github.com/xi-editor/druid/pull/894
 [#897]: https://github.com/xi-editor/druid/pull/897
 [#898]: https://github.com/xi-editor/druid/pull/898

druid-shell/src/platform/gtk/application.rs

@@ -14,8 +14,6 @@
 
 //! GTK implementation of features at the application scope.
 
-use std::cell::RefCell;
-
 use gio::prelude::ApplicationExtManual;
 use gio::{ApplicationExt, ApplicationFlags, Cancellable};
 use gtk::{Application as GtkApplication, GtkApplicationExt};
@@ -24,69 +22,59 @@ use crate::application::AppHandler;
 
 use super::clipboard::Clipboard;
 use super::error::Error;
-use super::util;
-
-// XXX: The application needs to be global because WindowBuilder::build wants
-// to construct an ApplicationWindow, which needs the application, but
-// WindowBuilder::build does not get the RunLoop
-thread_local!(
-    static GTK_APPLICATION: RefCell<Option<GtkApplication>> = RefCell::new(None);
-);
 
 #[derive(Clone)]
-pub(crate) struct Application;
+pub(crate) struct Application {
+    gtk_app: GtkApplication,
+}
 
 impl Application {
     pub fn new() -> Result<Application, Error> {
         // TODO: we should give control over the application ID to the user
-        let application = GtkApplication::new(
+        let gtk_app = match GtkApplication::new(
             Some("com.github.xi-editor.druid"),
             // TODO we set this to avoid connecting to an existing running instance
             // of "com.github.xi-editor.druid" after which we would never receive
             // the "Activate application" below. See pull request druid#384
             // Which shows another way once we have in place a mechanism for
             // communication with remote instances.
             ApplicationFlags::NON_UNIQUE,
-        )
-        .expect("Unable to create GTK application");
+        ) {
+            Ok(app) => app,
+            Err(err) => return Err(Error::BoolError(err)),
+        };
 
-        application.connect_activate(|_app| {
+        gtk_app.connect_activate(|_app| {
             log::info!("gtk: Activated application");
         });
 
-        application
-            .register(None as Option<&Cancellable>)
-            .expect("Could not register GTK application");
+        if let Err(err) = gtk_app.register(None as Option<&Cancellable>) {
+            return Err(Error::Error(err));
+        }
 
-        GTK_APPLICATION.with(move |x| *x.borrow_mut() = Some(application));
-        Ok(Application)
+        Ok(Application { gtk_app })
     }
 
-    pub fn run(self, _handler: Option<Box<dyn AppHandler>>) {
-        util::assert_main_thread();
+    #[inline]
+    pub fn gtk_app(&self) -> &GtkApplication {
+        &self.gtk_app
+    }
 
+    pub fn run(self, _handler: Option<Box<dyn AppHandler>>) {
         // TODO: should we pass the command line arguments?
-        GTK_APPLICATION.with(|x| {
-            x.borrow()
-                .as_ref()
-                .unwrap() // Safe because we initialized this in RunLoop::new
-                .run(&[])
-        });
+        self.gtk_app.run(&[]);
     }
 
     pub fn quit(&self) {
-        util::assert_main_thread();
-        with_application(|app| {
-            match app.get_active_window() {
-                None => {
-                    // no application is running, main is not running
-                }
-                Some(_) => {
-                    // we still have an active window, close the run loop
-                    app.quit();
-                }
+        match self.gtk_app.get_active_window() {
+            None => {
+                // no application is running, main is not running
             }
-        });
+            Some(_) => {
+                // we still have an active window, close the run loop
+                self.gtk_app.quit();
+            }
+        }
     }
 
     pub fn clipboard(&self) -> Clipboard {
@@ -97,18 +85,3 @@ impl Application {
         glib::get_language_names()[0].as_str().into()
     }
 }
-
-#[inline]
-pub(crate) fn with_application<F, R>(f: F) -> R
-where
-    F: std::ops::FnOnce(GtkApplication) -> R,
-{
-    util::assert_main_thread();
-    GTK_APPLICATION.with(move |app| {
-        let app = app
-            .borrow()
-            .clone()
-            .expect("Tried to manipulate the application before RunLoop::new was called");
-        f(app)
-    })
-}

druid-shell/src/platform/gtk/error.rs

@@ -14,14 +14,26 @@
 
 //! GTK platform errors.
 
-//TODO: add a platform error for GTK
+use std::fmt;
 
+use glib::{BoolError, Error as GLibError};
+
+/// GTK platform errors.
 #[derive(Debug, Clone)]
-pub struct Error;
+pub enum Error {
+    /// Generic GTK error.
+    Error(GLibError),
+    /// GTK error that has no information provided by GTK,
+    /// but may have extra information provided by gtk-rs.
+    BoolError(BoolError),
+}
 
-impl std::fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "GTK Error")
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        match self {
+            Error::Error(err) => write!(f, "GTK Error: {}", err),
+            Error::BoolError(err) => write!(f, "GTK BoolError: {}", err),
+        }
     }
 }
 

druid-shell/src/platform/gtk/window.rs

@@ -33,18 +33,18 @@ use gtk::{AccelGroup, ApplicationWindow, DrawingArea};
 use crate::kurbo::{Point, Rect, Size, Vec2};
 use crate::piet::{Piet, RenderContext};
 
-use super::application::{with_application, Application};
-use super::dialog;
-use super::menu::Menu;
-use super::util::assert_main_thread;
-
 use crate::common_util::IdleCallback;
 use crate::dialog::{FileDialogOptions, FileDialogType, FileInfo};
 use crate::keyboard;
 use crate::mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
 use crate::window::{IdleToken, Text, TimerToken, WinHandler};
 use crate::Error;
 
+use super::application::Application;
+use super::dialog;
+use super::menu::Menu;
+use super::util;
+
 /// Taken from https://gtk-rs.org/docs-src/tutorial/closures
 /// It is used to reduce the boilerplate of setting up gtk callbacks
 /// Example:
@@ -82,6 +82,7 @@ pub struct WindowHandle {
 
 /// Builder abstraction for creating new windows
 pub(crate) struct WindowBuilder {
+    app: Application,
     handler: Option<Box<dyn WinHandler>>,
     title: String,
     menu: Option<Menu>,
@@ -112,8 +113,9 @@ pub(crate) struct WindowState {
 }
 
 impl WindowBuilder {
-    pub fn new(_app: Application) -> WindowBuilder {
+    pub fn new(app: Application) -> WindowBuilder {
         WindowBuilder {
+            app,
             handler: None,
             title: String::new(),
             menu: None,
@@ -153,13 +155,11 @@ impl WindowBuilder {
     }
 
     pub fn build(self) -> Result<WindowHandle, Error> {
-        assert_main_thread();
-
         let handler = self
             .handler
             .expect("Tried to build a window without setting the handler");
 
-        let window = with_application(|app| ApplicationWindow::new(&app));
+        let window = ApplicationWindow::new(self.app.gtk_app());
 
         window.set_title(&self.title);
         window.set_resizable(self.resizable);
@@ -191,14 +191,14 @@ impl WindowBuilder {
             current_keyval: RefCell::new(None),
         });
 
-        with_application(|app| {
-            app.connect_shutdown(clone!(win_state => move |_| {
+        self.app
+            .gtk_app()
+            .connect_shutdown(clone!(win_state => move |_| {
                 // this ties a clone of Arc<WindowState> to the ApplicationWindow to keep it alive
                 // when the ApplicationWindow is destroyed, the last Arc is dropped
                 // and any Weak<WindowState> will be None on upgrade()
                 let _ = &win_state;
-            }))
-        });
+            }));
 
         let handle = WindowHandle {
             state: Arc::downgrade(&win_state),
@@ -742,7 +742,7 @@ impl IdleHandle {
 }
 
 fn run_idle(state: &Arc<WindowState>) -> glib::source::Continue {
-    assert_main_thread();
+    util::assert_main_thread();
     let mut handler = state.handler.borrow_mut();
 
     let queue: Vec<_> = std::mem::replace(&mut state.idle_queue.lock().unwrap(), Vec::new());