Fix seg fault when using the WindowContext::on_window_should_close() API

Mikayla created 2 years ago

Change summary

crates/collab_ui/src/collab_ui.rs         |  1 -
crates/editor/src/display_map/wrap_map.rs |  2 +-
crates/gpui/src/platform/mac/window.rs    |  4 +---
crates/gpui/src/window.rs                 | 11 ++++++++++-
crates/zed/src/zed.rs                     |  1 +
5 files changed, 13 insertions(+), 6 deletions(-)

Detailed changes

crates/collab_ui/src/collab_ui.rs

@@ -111,7 +111,6 @@ fn notification_window_options(
     let screen_bounds = screen.bounds();
     let size: Size<GlobalPixels> = window_size.into();
 
-    // todo!() use content bounds instead of screen.bounds and get rid of magics in point's 2nd argument.
     let bounds = gpui::Bounds::<GlobalPixels> {
         origin: screen_bounds.upper_right()
             - point(

crates/editor/src/display_map/wrap_map.rs

@@ -1043,7 +1043,7 @@ mod tests {
 
     #[gpui::test(iterations = 100)]
     async fn test_random_wraps(cx: &mut gpui::TestAppContext, mut rng: StdRng) {
-        // todo!() this test is flaky
+        // todo this test is flaky
         init_test(cx);
 
         cx.background_executor.set_block_on_ticks(0..=50);

crates/gpui/src/platform/mac/window.rs

@@ -269,6 +269,7 @@ unsafe fn build_window_class(name: &'static str, superclass: &Class) -> *const C
         sel!(windowShouldClose:),
         window_should_close as extern "C" fn(&Object, Sel, id) -> BOOL,
     );
+
     decl.add_method(sel!(close), close_window as extern "C" fn(&Object, Sel));
 
     decl.add_method(
@@ -685,9 +686,6 @@ impl Drop for MacWindow {
         this.executor
             .spawn(async move {
                 unsafe {
-                    // todo!() this panic()s when you click the red close button
-                    // unless should_close returns false.
-                    // (luckliy in zed it always returns false)
                     window.close();
                 }
             })

crates/gpui/src/window.rs

@@ -1904,7 +1904,16 @@ impl<'a> WindowContext<'a> {
         let mut this = self.to_async();
         self.window
             .platform_window
-            .on_should_close(Box::new(move || this.update(|_, cx| f(cx)).unwrap_or(true)))
+            .on_should_close(Box::new(move || {
+                this.update(|_, cx| {
+                    // Ensure that the window is removed from the app if it's been closed.
+                    if f(cx) {
+                        cx.remove_window();
+                    }
+                    false
+                })
+                .unwrap_or(true)
+            }))
     }
 }
 

crates/zed/src/zed.rs

@@ -148,6 +148,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
         cx.on_window_should_close(move |cx| {
             handle
                 .update(cx, |workspace, cx| {
+                    // We'll handle closing asynchoronously
                     workspace.close_window(&Default::default(), cx);
                     false
                 })