Ensure we drop the last pane item

Nathan Sobo created 3 years ago

Previously, we weren't updating the toolbar's active item when emptying out a pane. This enhances an integration test to ensure that we don't hold references to any editors or buffers once we close everything.

Change summary

crates/gpui/src/app.rs       | 52 +++++++++++++++++++++++++++++++++++++
crates/workspace/src/pane.rs |  1 
crates/zed/src/zed.rs        | 37 +++++++++++++++++++--------
3 files changed, 78 insertions(+), 12 deletions(-)

Detailed changes

crates/gpui/src/app.rs

@@ -597,6 +597,15 @@ impl TestAppContext {
     pub fn leak_detector(&self) -> Arc<Mutex<LeakDetector>> {
         self.cx.borrow().leak_detector()
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn assert_dropped(&self, handle: impl WeakHandle) {
+        self.cx
+            .borrow()
+            .leak_detector()
+            .lock()
+            .assert_dropped(handle.id())
+    }
 }
 
 impl AsyncAppContext {
@@ -3302,6 +3311,10 @@ pub trait Handle<T> {
         Self: Sized;
 }
 
+pub trait WeakHandle {
+    fn id(&self) -> usize;
+}
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub enum EntityLocation {
     Model(usize),
@@ -3576,6 +3589,12 @@ pub struct WeakModelHandle<T> {
     model_type: PhantomData<T>,
 }
 
+impl<T> WeakHandle for WeakModelHandle<T> {
+    fn id(&self) -> usize {
+        self.model_id
+    }
+}
+
 unsafe impl<T> Send for WeakModelHandle<T> {}
 unsafe impl<T> Sync for WeakModelHandle<T> {}
 
@@ -4145,6 +4164,12 @@ pub struct WeakViewHandle<T> {
     view_type: PhantomData<T>,
 }
 
+impl<T> WeakHandle for WeakViewHandle<T> {
+    fn id(&self) -> usize {
+        self.view_id
+    }
+}
+
 impl<T: View> WeakViewHandle<T> {
     fn new(window_id: usize, view_id: usize) -> Self {
         Self {
@@ -4471,11 +4496,36 @@ impl LeakDetector {
         }
     }
 
+    pub fn assert_dropped(&mut self, entity_id: usize) {
+        if let Some((type_name, backtraces)) = self.handle_backtraces.get_mut(&entity_id) {
+            for trace in backtraces.values_mut() {
+                if let Some(trace) = trace {
+                    trace.resolve();
+                    eprintln!("{:?}", crate::util::CwdBacktrace(trace));
+                }
+            }
+
+            let hint = if *LEAK_BACKTRACE {
+                ""
+            } else {
+                " – set LEAK_BACKTRACE=1 for more information"
+            };
+
+            panic!(
+                "{} handles to {} {} still exist{}",
+                backtraces.len(),
+                type_name.unwrap_or("entity"),
+                entity_id,
+                hint
+            );
+        }
+    }
+
     pub fn detect(&mut self) {
         let mut found_leaks = false;
         for (id, (type_name, backtraces)) in self.handle_backtraces.iter_mut() {
             eprintln!(
-                "leaked {} handles to {:?} {}",
+                "leaked {} handles to {} {}",
                 backtraces.len(),
                 type_name.unwrap_or("entity"),
                 id

crates/workspace/src/pane.rs

@@ -580,6 +580,7 @@ impl Pane {
                         let item = pane.items.remove(item_ix);
                         if pane.items.is_empty() {
                             item.deactivated(cx);
+                            pane.update_toolbar(cx);
                             cx.emit(Event::Remove);
                         }
 

crates/zed/src/zed.rs

@@ -725,32 +725,47 @@ mod tests {
             .update(cx, |w, cx| w.open_path(file1.clone(), cx))
             .await
             .unwrap();
-        cx.read(|cx| {
-            assert_eq!(
-                pane_1.read(cx).active_item().unwrap().project_path(cx),
-                Some(file1.clone())
-            );
+
+        let (editor_1, buffer) = pane_1.update(cx, |pane_1, cx| {
+            let editor = pane_1.active_item().unwrap().downcast::<Editor>().unwrap();
+            assert_eq!(editor.project_path(cx), Some(file1.clone()));
+            let buffer = editor.update(cx, |editor, cx| {
+                editor.insert("dirt", cx);
+                editor.buffer().downgrade()
+            });
+            (editor.downgrade(), buffer)
         });
 
         cx.dispatch_action(window_id, pane::Split(SplitDirection::Right));
-        cx.update(|cx| {
+        let editor_2 = cx.update(|cx| {
             let pane_2 = workspace.read(cx).active_pane().clone();
             assert_ne!(pane_1, pane_2);
 
             let pane2_item = pane_2.read(cx).active_item().unwrap();
             assert_eq!(pane2_item.project_path(cx.as_ref()), Some(file1.clone()));
 
-            cx.dispatch_action(
-                window_id,
-                vec![workspace.id(), pane_2.id()],
-                &workspace::CloseActiveItem,
-            );
+            pane2_item.downcast::<Editor>().unwrap().downgrade()
         });
+        cx.dispatch_action(window_id, workspace::CloseActiveItem);
+
         cx.foreground().run_until_parked();
         workspace.read_with(cx, |workspace, _| {
             assert_eq!(workspace.panes().len(), 1);
             assert_eq!(workspace.active_pane(), &pane_1);
         });
+
+        cx.dispatch_action(window_id, workspace::CloseActiveItem);
+        cx.foreground().run_until_parked();
+        cx.simulate_prompt_answer(window_id, 1);
+        cx.foreground().run_until_parked();
+
+        workspace.read_with(cx, |workspace, cx| {
+            assert!(workspace.active_item(cx).is_none());
+        });
+
+        cx.assert_dropped(editor_1);
+        cx.assert_dropped(editor_2);
+        cx.assert_dropped(buffer);
     }
 
     #[gpui::test]