x11: Fix window close (#11008)

Fixes https://github.com/zed-industries/zed/issues/10483 on X11

Also calls the `should_close` callback before closing the window (needed
for the "Do you want to save?" dialog).

Release Notes:

- N/A

crates/gpui/src/platform/linux/x11/client.rs

@@ -1,6 +1,6 @@
 use std::cell::RefCell;
 use std::ops::Deref;
-use std::rc::Rc;
+use std::rc::{Rc, Weak};
 use std::time::{Duration, Instant};
 
 use calloop::{EventLoop, LoopHandle};
@@ -23,10 +23,10 @@ use crate::platform::linux::LinuxClient;
 use crate::platform::{LinuxCommon, PlatformWindow};
 use crate::{
     px, AnyWindowHandle, Bounds, CursorStyle, DisplayId, Modifiers, ModifiersChangedEvent, Pixels,
-    PlatformDisplay, PlatformInput, Point, ScrollDelta, Size, TouchPhase, WindowParams,
+    PlatformDisplay, PlatformInput, Point, ScrollDelta, Size, TouchPhase, WindowParams, X11Window,
 };
 
-use super::{super::SCROLL_LINES, X11Display, X11Window, XcbAtoms};
+use super::{super::SCROLL_LINES, X11Display, X11WindowStatePtr, XcbAtoms};
 use super::{button_of_key, modifiers_from_state};
 use crate::platform::linux::is_within_click_distance;
 use crate::platform::linux::platform::DOUBLE_CLICK_INTERVAL;
@@ -36,12 +36,12 @@ use calloop::{
 };
 
 pub(crate) struct WindowRef {
-    window: X11Window,
+    window: X11WindowStatePtr,
     refresh_event_token: RegistrationToken,
 }
 
 impl Deref for WindowRef {
-    type Target = X11Window;
+    type Target = X11WindowStatePtr;
 
     fn deref(&self) -> &Self::Target {
         &self.window
@@ -68,6 +68,24 @@ pub struct X11ClientState {
     pub(crate) primary: X11ClipboardContext<Primary>,
 }
 
+#[derive(Clone)]
+pub struct X11ClientStatePtr(pub Weak<RefCell<X11ClientState>>);
+
+impl X11ClientStatePtr {
+    pub fn drop_window(&self, x_window: u32) {
+        let client = X11Client(self.0.upgrade().expect("client already dropped"));
+        let mut state = client.0.borrow_mut();
+
+        if let Some(window_ref) = state.windows.remove(&x_window) {
+            state.loop_handle.remove(window_ref.refresh_event_token);
+        }
+
+        if state.windows.is_empty() {
+            state.common.signal.stop();
+        }
+    }
+}
+
 #[derive(Clone)]
 pub(crate) struct X11Client(Rc<RefCell<X11ClientState>>);
 
@@ -171,7 +189,7 @@ impl X11Client {
         })))
     }
 
-    fn get_window(&self, win: xproto::Window) -> Option<X11Window> {
+    fn get_window(&self, win: xproto::Window) -> Option<X11WindowStatePtr> {
         let state = self.0.borrow();
         state
             .windows
@@ -182,18 +200,16 @@ impl X11Client {
     fn handle_event(&self, event: Event) -> Option<()> {
         match event {
             Event::ClientMessage(event) => {
+                let window = self.get_window(event.window)?;
                 let [atom, ..] = event.data.as_data32();
                 let mut state = self.0.borrow_mut();
 
                 if atom == state.atoms.WM_DELETE_WINDOW {
-                    // window "x" button clicked by user, we gracefully exit
-                    let window_ref = state.windows.remove(&event.window)?;
-
-                    state.loop_handle.remove(window_ref.refresh_event_token);
-                    window_ref.window.destroy();
-
-                    if state.windows.is_empty() {
-                        state.common.signal.stop();
+                    // window "x" button clicked by user
+                    if window.should_close() {
+                        let window_ref = state.windows.remove(&event.window)?;
+                        state.loop_handle.remove(window_ref.refresh_event_token);
+                        // Rest of the close logic is handled in drop_window()
                     }
                 }
             }
@@ -424,6 +440,8 @@ impl LinuxClient for X11Client {
         let x_window = state.xcb_connection.generate_id().unwrap();
 
         let window = X11Window::new(
+            X11ClientStatePtr(Rc::downgrade(&self.0)),
+            state.common.foreground_executor.clone(),
             params,
             &state.xcb_connection,
             state.x_root_index,
@@ -492,7 +510,7 @@ impl LinuxClient for X11Client {
             .expect("Failed to initialize refresh timer");
 
         let window_ref = WindowRef {
-            window: window.clone(),
+            window: window.0.clone(),
             refresh_event_token,
         };
 

crates/gpui/src/platform/linux/x11/window.rs

@@ -2,10 +2,10 @@
 #![allow(unused)]
 
 use crate::{
-    platform::blade::BladeRenderer, size, Bounds, DevicePixels, Modifiers, Pixels, PlatformAtlas,
-    PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point, PromptLevel,
-    Scene, Size, WindowAppearance, WindowBackgroundAppearance, WindowOptions, WindowParams,
-    X11Client, X11ClientState,
+    platform::blade::BladeRenderer, size, Bounds, DevicePixels, ForegroundExecutor, Modifiers,
+    Pixels, Platform, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler,
+    PlatformWindow, Point, PromptLevel, Scene, Size, WindowAppearance, WindowBackgroundAppearance,
+    WindowOptions, WindowParams, X11Client, X11ClientState, X11ClientStatePtr,
 };
 use blade_graphics as gpu;
 use parking_lot::Mutex;
@@ -77,6 +77,8 @@ pub struct Callbacks {
 }
 
 pub(crate) struct X11WindowState {
+    client: X11ClientStatePtr,
+    executor: ForegroundExecutor,
     atoms: XcbAtoms,
     raw: RawWindow,
     bounds: Bounds<i32>,
@@ -88,7 +90,7 @@ pub(crate) struct X11WindowState {
 }
 
 #[derive(Clone)]
-pub(crate) struct X11Window {
+pub(crate) struct X11WindowStatePtr {
     pub(crate) state: Rc<RefCell<X11WindowState>>,
     pub(crate) callbacks: Rc<RefCell<Callbacks>>,
     xcb_connection: Rc<XCBConnection>,
@@ -124,6 +126,8 @@ impl rwh::HasDisplayHandle for X11Window {
 
 impl X11WindowState {
     pub fn new(
+        client: X11ClientStatePtr,
+        executor: ForegroundExecutor,
         params: WindowParams,
         xcb_connection: &Rc<XCBConnection>,
         x_main_screen_index: usize,
@@ -224,6 +228,8 @@ impl X11WindowState {
         let gpu_extent = query_render_extent(xcb_connection, x_window);
 
         Self {
+            client,
+            executor,
             display: Rc::new(X11Display::new(xcb_connection, x_screen_index).unwrap()),
             raw,
             bounds: params.bounds.map(|v| v.0),
@@ -244,16 +250,47 @@ impl X11WindowState {
     }
 }
 
+pub(crate) struct X11Window(pub X11WindowStatePtr);
+
+impl Drop for X11Window {
+    fn drop(&mut self) {
+        let mut state = self.0.state.borrow_mut();
+        state.renderer.destroy();
+
+        self.0.xcb_connection.unmap_window(self.0.x_window).unwrap();
+        self.0
+            .xcb_connection
+            .destroy_window(self.0.x_window)
+            .unwrap();
+        self.0.xcb_connection.flush().unwrap();
+
+        let this_ptr = self.0.clone();
+        let client_ptr = state.client.clone();
+        state
+            .executor
+            .spawn(async move {
+                this_ptr.close();
+                client_ptr.drop_window(this_ptr.x_window);
+            })
+            .detach();
+        drop(state);
+    }
+}
+
 impl X11Window {
     pub fn new(
+        client: X11ClientStatePtr,
+        executor: ForegroundExecutor,
         params: WindowParams,
         xcb_connection: &Rc<XCBConnection>,
         x_main_screen_index: usize,
         x_window: xproto::Window,
         atoms: &XcbAtoms,
     ) -> Self {
-        X11Window {
+        Self(X11WindowStatePtr {
             state: Rc::new(RefCell::new(X11WindowState::new(
+                client,
+                executor,
                 params,
                 xcb_connection,
                 x_main_screen_index,
@@ -263,20 +300,27 @@ impl X11Window {
             callbacks: Rc::new(RefCell::new(Callbacks::default())),
             xcb_connection: xcb_connection.clone(),
             x_window,
+        })
+    }
+}
+
+impl X11WindowStatePtr {
+    pub fn should_close(&self) -> bool {
+        let mut cb = self.callbacks.borrow_mut();
+        if let Some(mut should_close) = cb.should_close.take() {
+            let result = (should_close)();
+            cb.should_close = Some(should_close);
+            result
+        } else {
+            true
         }
     }
 
-    pub fn destroy(&self) {
-        let mut state = self.state.borrow_mut();
-        state.renderer.destroy();
-        drop(state);
-
-        self.xcb_connection.unmap_window(self.x_window).unwrap();
-        self.xcb_connection.destroy_window(self.x_window).unwrap();
-        if let Some(fun) = self.callbacks.borrow_mut().close.take() {
-            fun();
+    pub fn close(&self) {
+        let mut callbacks = self.callbacks.borrow_mut();
+        if let Some(fun) = callbacks.close.take() {
+            fun()
         }
-        self.xcb_connection.flush().unwrap();
     }
 
     pub fn refresh(&self) {
@@ -345,7 +389,7 @@ impl X11Window {
 
 impl PlatformWindow for X11Window {
     fn bounds(&self) -> Bounds<DevicePixels> {
-        self.state.borrow_mut().bounds.map(|v| v.into())
+        self.0.state.borrow_mut().bounds.map(|v| v.into())
     }
 
     // todo(linux)
@@ -359,11 +403,11 @@ impl PlatformWindow for X11Window {
     }
 
     fn content_size(&self) -> Size<Pixels> {
-        self.state.borrow_mut().content_size()
+        self.0.state.borrow_mut().content_size()
     }
 
     fn scale_factor(&self) -> f32 {
-        self.state.borrow_mut().scale_factor
+        self.0.state.borrow_mut().scale_factor
     }
 
     // todo(linux)
@@ -372,13 +416,14 @@ impl PlatformWindow for X11Window {
     }
 
     fn display(&self) -> Rc<dyn PlatformDisplay> {
-        self.state.borrow().display.clone()
+        self.0.state.borrow().display.clone()
     }
 
     fn mouse_position(&self) -> Point<Pixels> {
         let reply = self
+            .0
             .xcb_connection
-            .query_pointer(self.x_window)
+            .query_pointer(self.0.x_window)
             .unwrap()
             .reply()
             .unwrap();
@@ -395,11 +440,11 @@ impl PlatformWindow for X11Window {
     }
 
     fn set_input_handler(&mut self, input_handler: PlatformInputHandler) {
-        self.state.borrow_mut().input_handler = Some(input_handler);
+        self.0.state.borrow_mut().input_handler = Some(input_handler);
     }
 
     fn take_input_handler(&mut self) -> Option<PlatformInputHandler> {
-        self.state.borrow_mut().input_handler.take()
+        self.0.state.borrow_mut().input_handler.take()
     }
 
     fn prompt(
@@ -414,8 +459,9 @@ impl PlatformWindow for X11Window {
 
     fn activate(&self) {
         let win_aux = xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::ABOVE);
-        self.xcb_connection
-            .configure_window(self.x_window, &win_aux)
+        self.0
+            .xcb_connection
+            .configure_window(self.0.x_window, &win_aux)
             .log_err();
     }
 
@@ -425,22 +471,24 @@ impl PlatformWindow for X11Window {
     }
 
     fn set_title(&mut self, title: &str) {
-        self.xcb_connection
+        self.0
+            .xcb_connection
             .change_property8(
                 xproto::PropMode::REPLACE,
-                self.x_window,
+                self.0.x_window,
                 xproto::AtomEnum::WM_NAME,
                 xproto::AtomEnum::STRING,
                 title.as_bytes(),
             )
             .unwrap();
 
-        self.xcb_connection
+        self.0
+            .xcb_connection
             .change_property8(
                 xproto::PropMode::REPLACE,
-                self.x_window,
-                self.state.borrow().atoms._NET_WM_NAME,
-                self.state.borrow().atoms.UTF8_STRING,
+                self.0.x_window,
+                self.0.state.borrow().atoms._NET_WM_NAME,
+                self.0.state.borrow().atoms.UTF8_STRING,
                 title.as_bytes(),
             )
             .unwrap();
@@ -484,39 +532,39 @@ impl PlatformWindow for X11Window {
     }
 
     fn on_request_frame(&self, callback: Box<dyn FnMut()>) {
-        self.callbacks.borrow_mut().request_frame = Some(callback);
+        self.0.callbacks.borrow_mut().request_frame = Some(callback);
     }
 
     fn on_input(&self, callback: Box<dyn FnMut(PlatformInput) -> crate::DispatchEventResult>) {
-        self.callbacks.borrow_mut().input = Some(callback);
+        self.0.callbacks.borrow_mut().input = Some(callback);
     }
 
     fn on_active_status_change(&self, callback: Box<dyn FnMut(bool)>) {
-        self.callbacks.borrow_mut().active_status_change = Some(callback);
+        self.0.callbacks.borrow_mut().active_status_change = Some(callback);
     }
 
     fn on_resize(&self, callback: Box<dyn FnMut(Size<Pixels>, f32)>) {
-        self.callbacks.borrow_mut().resize = Some(callback);
+        self.0.callbacks.borrow_mut().resize = Some(callback);
     }
 
     fn on_fullscreen(&self, callback: Box<dyn FnMut(bool)>) {
-        self.callbacks.borrow_mut().fullscreen = Some(callback);
+        self.0.callbacks.borrow_mut().fullscreen = Some(callback);
     }
 
     fn on_moved(&self, callback: Box<dyn FnMut()>) {
-        self.callbacks.borrow_mut().moved = Some(callback);
+        self.0.callbacks.borrow_mut().moved = Some(callback);
     }
 
     fn on_should_close(&self, callback: Box<dyn FnMut() -> bool>) {
-        self.callbacks.borrow_mut().should_close = Some(callback);
+        self.0.callbacks.borrow_mut().should_close = Some(callback);
     }
 
     fn on_close(&self, callback: Box<dyn FnOnce()>) {
-        self.callbacks.borrow_mut().close = Some(callback);
+        self.0.callbacks.borrow_mut().close = Some(callback);
     }
 
     fn on_appearance_changed(&self, callback: Box<dyn FnMut()>) {
-        self.callbacks.borrow_mut().appearance_changed = Some(callback);
+        self.0.callbacks.borrow_mut().appearance_changed = Some(callback);
     }
 
     // todo(linux)
@@ -525,12 +573,12 @@ impl PlatformWindow for X11Window {
     }
 
     fn draw(&self, scene: &Scene) {
-        let mut inner = self.state.borrow_mut();
+        let mut inner = self.0.state.borrow_mut();
         inner.renderer.draw(scene);
     }
 
     fn sprite_atlas(&self) -> sync::Arc<dyn PlatformAtlas> {
-        let inner = self.state.borrow();
+        let inner = self.0.state.borrow();
         inner.renderer.sprite_atlas().clone()
     }
 }