From ca251babcd408d4a6944652e524bfed45cfc6a9e Mon Sep 17 00:00:00 2001
From: Antonio Scandurra <me@as-cii.com>
Date: Mon, 19 Feb 2024 15:38:50 +0100
Subject: [PATCH] Drop `Box<dyn PlatformWindow>` when the OS closes the native
 window (#8016)

Closes #7973

This fixes a leak in GPUI when the user didn't override
`on_should_close_window`.

Release Notes:

- N/A

---------

Co-authored-by: Thorsten <thorsten@zed.dev>
---
 crates/gpui/src/platform/mac/window.rs  | 28 +++++++++++++++----------
 crates/gpui/src/platform/test/window.rs |  4 +---
 crates/gpui/src/window.rs               | 18 +++++++---------
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs
index 715845bb65..0773b156bd 100644
--- a/crates/gpui/src/platform/mac/window.rs
+++ b/crates/gpui/src/platform/mac/window.rs
@@ -319,6 +319,7 @@ struct MacWindowState {
     handle: AnyWindowHandle,
     executor: ForegroundExecutor,
     native_window: id,
+    native_window_was_closed: bool,
     native_view: NonNull<Object>,
     display_link: Option<DisplayLink>,
     renderer: renderer::Renderer,
@@ -565,6 +566,7 @@ impl MacWindow {
                 handle,
                 executor,
                 native_window,
+                native_window_was_closed: false,
                 native_view: NonNull::new_unchecked(native_view),
                 display_link: None,
                 renderer: renderer::new_renderer(
@@ -732,13 +734,18 @@ impl Drop for MacWindow {
         this.renderer.destroy();
         let window = this.native_window;
         this.display_link.take();
-        this.executor
-            .spawn(async move {
-                unsafe {
-                    window.close();
-                }
-            })
-            .detach();
+        unsafe {
+            this.native_window.setDelegate_(nil);
+        }
+        if !this.native_window_was_closed {
+            this.executor
+                .spawn(async move {
+                    unsafe {
+                        window.close();
+                    }
+                })
+                .detach();
+        }
     }
 }
 
@@ -1511,10 +1518,9 @@ extern "C" fn close_window(this: &Object, _: Sel) {
     unsafe {
         let close_callback = {
             let window_state = get_window_state(this);
-            window_state
-                .as_ref()
-                .try_lock()
-                .and_then(|mut window_state| window_state.close_callback.take())
+            let mut lock = window_state.as_ref().lock();
+            lock.native_window_was_closed = true;
+            lock.close_callback.take()
         };
 
         if let Some(callback) = close_callback {
diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs
index 23e5cdeea7..940a3e44c5 100644
--- a/crates/gpui/src/platform/test/window.rs
+++ b/crates/gpui/src/platform/test/window.rs
@@ -272,9 +272,7 @@ impl PlatformWindow for TestWindow {
         self.0.lock().should_close_handler = Some(callback);
     }
 
-    fn on_close(&self, _callback: Box<dyn FnOnce()>) {
-        unimplemented!()
-    }
+    fn on_close(&self, _callback: Box<dyn FnOnce()>) {}
 
     fn on_appearance_changed(&self, _callback: Box<dyn FnMut()>) {}
 
diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs
index f7f53689f9..dc80fcd347 100644
--- a/crates/gpui/src/window.rs
+++ b/crates/gpui/src/window.rs
@@ -341,6 +341,12 @@ impl Window {
         let next_frame_callbacks: Rc<RefCell<Vec<FrameCallback>>> = Default::default();
         let last_input_timestamp = Rc::new(Cell::new(Instant::now()));
 
+        platform_window.on_close(Box::new({
+            let mut cx = cx.to_async();
+            move || {
+                let _ = handle.update(&mut cx, |_, cx| cx.remove_window());
+            }
+        }));
         platform_window.on_request_frame(Box::new({
             let mut cx = cx.to_async();
             let dirty = dirty.clone();
@@ -1595,17 +1601,7 @@ impl<'a> WindowContext<'a> {
         let mut this = self.to_async();
         self.window
             .platform_window
-            .on_should_close(Box::new(move || {
-                this.update(|cx| {
-                    // Ensure that the window is removed from the app if it's been closed
-                    // by always pre-empting the system close event.
-                    if f(cx) {
-                        cx.remove_window();
-                    }
-                    false
-                })
-                .unwrap_or(true)
-            }))
+            .on_should_close(Box::new(move || this.update(|cx| f(cx)).unwrap_or(true)))
     }
 
     pub(crate) fn parent_view_id(&self) -> EntityId {