Ensure that on_release callbacks are called even if view outlives its window

Max Brunsfeld created 2 years ago

Change summary

crates/go_to_line2/src/go_to_line.rs | 28 ++++++++++++++++------------
crates/gpui2/src/window.rs           |  8 ++++++--
crates/vim2/src/editor_events.rs     |  6 +++---
crates/workspace2/src/workspace2.rs  |  2 +-
crates/zed2/src/open_listener.rs     |  2 +-
5 files changed, 27 insertions(+), 19 deletions(-)

Detailed changes

crates/go_to_line2/src/go_to_line.rs

@@ -1,8 +1,8 @@
 use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Editor};
 use gpui::{
-    actions, div, prelude::*, AppContext, DismissEvent, Div, EventEmitter, FocusHandle,
-    FocusableView, Render, SharedString, Styled, Subscription, View, ViewContext, VisualContext,
-    WindowContext,
+    actions, div, prelude::*, AnyWindowHandle, AppContext, DismissEvent, Div, EventEmitter,
+    FocusHandle, FocusableView, Render, SharedString, Styled, Subscription, View, ViewContext,
+    VisualContext,
 };
 use text::{Bias, Point};
 use theme::ActiveTheme;
@@ -74,15 +74,19 @@ impl GoToLine {
         }
     }
 
-    fn release(&mut self, cx: &mut WindowContext) {
-        let scroll_position = self.prev_scroll_position.take();
-        self.active_editor.update(cx, |editor, cx| {
-            editor.highlight_rows(None);
-            if let Some(scroll_position) = scroll_position {
-                editor.set_scroll_position(scroll_position, cx);
-            }
-            cx.notify();
-        })
+    fn release(&mut self, window: AnyWindowHandle, cx: &mut AppContext) {
+        window
+            .update(cx, |_, cx| {
+                let scroll_position = self.prev_scroll_position.take();
+                self.active_editor.update(cx, |editor, cx| {
+                    editor.highlight_rows(None);
+                    if let Some(scroll_position) = scroll_position {
+                        editor.set_scroll_position(scroll_position, cx);
+                    }
+                    cx.notify();
+                })
+            })
+            .ok();
     }
 
     fn on_line_editor_event(

crates/gpui2/src/window.rs

@@ -2422,16 +2422,20 @@ impl<'a, V: 'static> ViewContext<'a, V> {
         subscription
     }
 
+    /// Register a callback to be invoked when the view is released.
+    ///
+    /// The callback receives a handle to the view's window. This handle may be
+    /// invalid, if the window was closed before the view was released.
     pub fn on_release(
         &mut self,
-        on_release: impl FnOnce(&mut V, &mut WindowContext) + 'static,
+        on_release: impl FnOnce(&mut V, AnyWindowHandle, &mut AppContext) + 'static,
     ) -> Subscription {
         let window_handle = self.window.handle;
         let (subscription, activate) = self.app.release_listeners.insert(
             self.view.model.entity_id,
             Box::new(move |this, cx| {
                 let this = this.downcast_mut().expect("invalid entity type");
-                let _ = window_handle.update(cx, |_, cx| on_release(this, cx));
+                on_release(this, window_handle, cx)
             }),
         );
         activate();

crates/vim2/src/editor_events.rs

@@ -13,7 +13,7 @@ pub fn init(cx: &mut AppContext) {
         .detach();
 
         let id = cx.view().entity_id();
-        cx.on_release(move |_, cx| released(id, cx)).detach();
+        cx.on_release(move |_, _, cx| released(id, cx)).detach();
     })
     .detach();
 }
@@ -51,8 +51,8 @@ fn blurred(editor: View<Editor>, cx: &mut WindowContext) {
     });
 }
 
-fn released(entity_id: EntityId, cx: &mut WindowContext) {
-    Vim::update(cx, |vim, _| {
+fn released(entity_id: EntityId, cx: &mut AppContext) {
+    cx.update_global(|vim: &mut Vim, _| {
         if vim
             .active_editor
             .as_ref()

crates/workspace2/src/workspace2.rs

@@ -642,7 +642,7 @@ impl Workspace {
                 this.serialize_workspace(cx);
                 cx.notify();
             }),
-            cx.on_release(|this, cx| {
+            cx.on_release(|this, _, cx| {
                 this.app_state.workspace_store.update(cx, |store, _| {
                     store.workspaces.remove(&this.window_self);
                 })

crates/zed2/src/open_listener.rs

@@ -253,7 +253,7 @@ pub async fn handle_cli_connection(
                                         let (done_tx, done_rx) = oneshot::channel();
                                         let _subscription =
                                             workspace.update(&mut cx, |workspace, cx| {
-                                                cx.on_release(move |_, _| {
+                                                cx.on_release(move |_, _, _| {
                                                     let _ = done_tx.send(());
                                                 })
                                             });