Move path prompting methods to MainThreadPlatform

Nathan Sobo and Max Brunsfeld created

They need to call back into the main thread, meaning the callbacks can't be Send + Sync.

Co-Authored-By: Max Brunsfeld <maxbrunsfeld@gmail.com>

Change summary

gpui/src/app.rs                   |  66 +++++++++--------
gpui/src/platform.rs              |  25 +++---
gpui/src/platform/mac/platform.rs | 126 ++++++++++++++++----------------
gpui/src/platform/test.rs         |  71 +++++++++--------
4 files changed, 148 insertions(+), 140 deletions(-)

Detailed changes

gpui/src/app.rs 🔗

@@ -105,7 +105,10 @@ pub struct App(Rc<RefCell<MutableAppContext>>);
 pub struct AsyncAppContext(Rc<RefCell<MutableAppContext>>);
 
 #[derive(Clone)]
-pub struct TestAppContext(Rc<RefCell<MutableAppContext>>, Rc<platform::test::Platform>);
+pub struct TestAppContext {
+    cx: Rc<RefCell<MutableAppContext>>,
+    main_thread_platform: Rc<platform::test::MainThreadPlatform>,
+}
 
 impl App {
     pub fn test<T, A: AssetSource, F: FnOnce(&mut MutableAppContext) -> T>(
@@ -134,16 +137,16 @@ impl App {
         let platform = Rc::new(platform::test::platform());
         let main_thread_platform = Rc::new(platform::test::main_thread_platform());
         let foreground = Rc::new(executor::Foreground::test());
-        let cx = TestAppContext(
-            Rc::new(RefCell::new(MutableAppContext::new(
+        let cx = TestAppContext {
+            cx: Rc::new(RefCell::new(MutableAppContext::new(
                 foreground.clone(),
-                platform.clone(),
-                main_thread_platform,
+                platform,
+                main_thread_platform.clone(),
                 asset_source,
             ))),
-            platform,
-        );
-        cx.0.borrow_mut().weak_self = Some(Rc::downgrade(&cx.0));
+            main_thread_platform,
+        };
+        cx.cx.borrow_mut().weak_self = Some(Rc::downgrade(&cx.cx));
 
         let future = f(cx);
         smol::block_on(foreground.run(future))
@@ -265,7 +268,7 @@ impl TestAppContext {
         name: &str,
         arg: T,
     ) {
-        self.0.borrow_mut().dispatch_action_any(
+        self.cx.borrow_mut().dispatch_action_any(
             window_id,
             &responder_chain,
             name,
@@ -279,7 +282,7 @@ impl TestAppContext {
         responder_chain: Vec<usize>,
         keystroke: &Keystroke,
     ) -> Result<bool> {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.dispatch_keystroke(window_id, responder_chain, keystroke)
     }
 
@@ -288,7 +291,7 @@ impl TestAppContext {
         T: Entity,
         F: FnOnce(&mut ModelContext<T>) -> T,
     {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.pending_flushes += 1;
         let handle = state.add_model(build_model);
         state.flush_effects();
@@ -300,15 +303,15 @@ impl TestAppContext {
         T: View,
         F: FnOnce(&mut ViewContext<T>) -> T,
     {
-        self.0.borrow_mut().add_window(build_root_view)
+        self.cx.borrow_mut().add_window(build_root_view)
     }
 
     pub fn window_ids(&self) -> Vec<usize> {
-        self.0.borrow().window_ids().collect()
+        self.cx.borrow().window_ids().collect()
     }
 
     pub fn root_view<T: View>(&self, window_id: usize) -> Option<ViewHandle<T>> {
-        self.0.borrow().root_view(window_id)
+        self.cx.borrow().root_view(window_id)
     }
 
     pub fn add_view<T, F>(&mut self, window_id: usize, build_view: F) -> ViewHandle<T>
@@ -316,7 +319,7 @@ impl TestAppContext {
         T: View,
         F: FnOnce(&mut ViewContext<T>) -> T,
     {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.pending_flushes += 1;
         let handle = state.add_view(window_id, build_view);
         state.flush_effects();
@@ -332,7 +335,7 @@ impl TestAppContext {
         T: View,
         F: FnOnce(&mut ViewContext<T>) -> Option<T>,
     {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.pending_flushes += 1;
         let handle = state.add_option_view(window_id, build_view);
         state.flush_effects();
@@ -340,11 +343,11 @@ impl TestAppContext {
     }
 
     pub fn read<T, F: FnOnce(&AppContext) -> T>(&self, callback: F) -> T {
-        callback(self.0.borrow().as_ref())
+        callback(self.cx.borrow().as_ref())
     }
 
     pub fn update<T, F: FnOnce(&mut MutableAppContext) -> T>(&mut self, callback: F) -> T {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         // Don't increment pending flushes in order to effects to be flushed before the callback
         // completes, which is helpful in tests.
         let result = callback(&mut *state);
@@ -355,23 +358,24 @@ impl TestAppContext {
     }
 
     pub fn font_cache(&self) -> Arc<FontCache> {
-        self.0.borrow().cx.font_cache.clone()
+        self.cx.borrow().cx.font_cache.clone()
     }
 
     pub fn platform(&self) -> Rc<dyn platform::Platform> {
-        self.0.borrow().platform.clone()
+        self.cx.borrow().platform.clone()
     }
 
     pub fn simulate_new_path_selection(&self, result: impl FnOnce(PathBuf) -> Option<PathBuf>) {
-        self.1.as_ref().simulate_new_path_selection(result);
+        self.main_thread_platform
+            .simulate_new_path_selection(result);
     }
 
     pub fn did_prompt_for_new_path(&self) -> bool {
-        self.1.as_ref().did_prompt_for_new_path()
+        self.main_thread_platform.as_ref().did_prompt_for_new_path()
     }
 
     pub fn simulate_prompt_answer(&self, window_id: usize, answer: usize) {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         let (_, window) = state
             .presenters_and_platform_windows
             .get_mut(&window_id)
@@ -472,7 +476,7 @@ impl UpdateModel for TestAppContext {
         T: Entity,
         F: FnOnce(&mut T, &mut ModelContext<T>) -> S,
     {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.pending_flushes += 1;
         let result = state.update_model(handle, update);
         state.flush_effects();
@@ -486,7 +490,7 @@ impl ReadModelWith for TestAppContext {
         handle: &ModelHandle<E>,
         read: F,
     ) -> T {
-        let cx = self.0.borrow();
+        let cx = self.cx.borrow();
         let cx = cx.as_ref();
         read(handle.read(cx), cx)
     }
@@ -498,7 +502,7 @@ impl UpdateView for TestAppContext {
         T: View,
         F: FnOnce(&mut T, &mut ViewContext<T>) -> S,
     {
-        let mut state = self.0.borrow_mut();
+        let mut state = self.cx.borrow_mut();
         state.pending_flushes += 1;
         let result = state.update_view(handle, update);
         state.flush_effects();
@@ -512,7 +516,7 @@ impl ReadViewWith for TestAppContext {
         V: View,
         F: FnOnce(&V, &AppContext) -> T,
     {
-        let cx = self.0.borrow();
+        let cx = self.cx.borrow();
         let cx = cx.as_ref();
         read(handle.read(cx), cx)
     }
@@ -751,7 +755,7 @@ impl MutableAppContext {
     {
         let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
         let foreground = self.foreground.clone();
-        self.platform().prompt_for_paths(
+        self.main_thread_platform.prompt_for_paths(
             options,
             Box::new(move |paths| {
                 foreground
@@ -767,7 +771,7 @@ impl MutableAppContext {
     {
         let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
         let foreground = self.foreground.clone();
-        self.platform().prompt_for_new_path(
+        self.main_thread_platform.prompt_for_new_path(
             directory,
             Box::new(move |path| {
                 foreground
@@ -2097,7 +2101,7 @@ impl<T: Entity> ModelHandle<T> {
     ) -> impl Future<Output = ()> {
         let (tx, mut rx) = mpsc::channel(1024);
 
-        let mut cx = cx.0.borrow_mut();
+        let mut cx = cx.cx.borrow_mut();
         self.update(&mut *cx, |_, cx| {
             cx.observe(self, {
                 let mut tx = tx.clone();
@@ -2301,7 +2305,7 @@ impl<T: View> ViewHandle<T> {
     ) -> impl Future<Output = ()> {
         let (tx, mut rx) = mpsc::channel(1024);
 
-        let mut cx = cx.0.borrow_mut();
+        let mut cx = cx.cx.borrow_mut();
         self.update(&mut *cx, |_, cx| {
             cx.observe_view(self, {
                 let mut tx = tx.clone();

gpui/src/platform.rs 🔗

@@ -28,13 +28,24 @@ use std::{
 };
 
 pub(crate) trait MainThreadPlatform {
-    fn on_menu_command(&self, callback: Box<dyn FnMut(&str, Option<&dyn Any>)>);
     fn on_become_active(&self, callback: Box<dyn FnMut()>);
     fn on_resign_active(&self, callback: Box<dyn FnMut()>);
     fn on_event(&self, callback: Box<dyn FnMut(Event) -> bool>);
     fn on_open_files(&self, callback: Box<dyn FnMut(Vec<PathBuf>)>);
-    fn set_menus(&self, menus: Vec<Menu>);
     fn run(&self, on_finish_launching: Box<dyn FnOnce() -> ()>);
+
+    fn on_menu_command(&self, callback: Box<dyn FnMut(&str, Option<&dyn Any>)>);
+    fn set_menus(&self, menus: Vec<Menu>);
+    fn prompt_for_paths(
+        &self,
+        options: PathPromptOptions,
+        done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
+    );
+    fn prompt_for_new_path(
+        &self,
+        directory: &Path,
+        done_fn: Box<dyn FnOnce(Option<std::path::PathBuf>)>,
+    );
 }
 
 pub trait Platform {
@@ -49,16 +60,6 @@ pub trait Platform {
         executor: Rc<executor::Foreground>,
     ) -> Box<dyn Window>;
     fn key_window_id(&self) -> Option<usize>;
-    fn prompt_for_paths(
-        &self,
-        options: PathPromptOptions,
-        done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
-    );
-    fn prompt_for_new_path(
-        &self,
-        directory: &Path,
-        done_fn: Box<dyn FnOnce(Option<std::path::PathBuf>)>,
-    );
     fn quit(&self);
     fn write_to_clipboard(&self, item: ClipboardItem);
     fn read_from_clipboard(&self) -> Option<ClipboardItem>;

gpui/src/platform/mac/platform.rs 🔗

@@ -183,21 +183,10 @@ impl platform::MainThreadPlatform for MacMainThreadPlatform {
         self.0.borrow_mut().event = Some(callback);
     }
 
-    fn on_menu_command(&self, callback: Box<dyn FnMut(&str, Option<&dyn Any>)>) {
-        self.0.borrow_mut().menu_command = Some(callback);
-    }
-
     fn on_open_files(&self, callback: Box<dyn FnMut(Vec<PathBuf>)>) {
         self.0.borrow_mut().open_files = Some(callback);
     }
 
-    fn set_menus(&self, menus: Vec<Menu>) {
-        unsafe {
-            let app: id = msg_send![APP_CLASS, sharedApplication];
-            app.setMainMenu_(self.create_menu_bar(menus));
-        }
-    }
-
     fn run(&self, on_finish_launching: Box<dyn FnOnce() -> ()>) {
         self.0.borrow_mut().finish_launching = Some(on_finish_launching);
 
@@ -218,65 +207,18 @@ impl platform::MainThreadPlatform for MacMainThreadPlatform {
             (*app.delegate()).set_ivar(MAC_PLATFORM_IVAR, null_mut::<c_void>());
         }
     }
-}
-
-pub struct MacPlatform {
-    dispatcher: Arc<Dispatcher>,
-    fonts: Arc<FontSystem>,
-    pasteboard: id,
-    text_hash_pasteboard_type: id,
-    metadata_pasteboard_type: id,
-}
-
-impl MacPlatform {
-    pub fn new() -> Self {
-        Self {
-            dispatcher: Arc::new(Dispatcher),
-            fonts: Arc::new(FontSystem::new()),
-            pasteboard: unsafe { NSPasteboard::generalPasteboard(nil) },
-            text_hash_pasteboard_type: unsafe { ns_string("zed-text-hash") },
-            metadata_pasteboard_type: unsafe { ns_string("zed-metadata") },
-        }
-    }
 
-    unsafe fn read_from_pasteboard(&self, kind: id) -> Option<&[u8]> {
-        let data = self.pasteboard.dataForType(kind);
-        if data == nil {
-            None
-        } else {
-            Some(slice::from_raw_parts(
-                data.bytes() as *mut u8,
-                data.length() as usize,
-            ))
-        }
-    }
-}
-
-impl platform::Platform for MacPlatform {
-    fn dispatcher(&self) -> Arc<dyn platform::Dispatcher> {
-        self.dispatcher.clone()
+    fn on_menu_command(&self, callback: Box<dyn FnMut(&str, Option<&dyn Any>)>) {
+        self.0.borrow_mut().menu_command = Some(callback);
     }
 
-    fn activate(&self, ignoring_other_apps: bool) {
+    fn set_menus(&self, menus: Vec<Menu>) {
         unsafe {
-            let app = NSApplication::sharedApplication(nil);
-            app.activateIgnoringOtherApps_(ignoring_other_apps.to_objc());
+            let app: id = msg_send![APP_CLASS, sharedApplication];
+            app.setMainMenu_(self.create_menu_bar(menus));
         }
     }
 
-    fn open_window(
-        &self,
-        id: usize,
-        options: platform::WindowOptions,
-        executor: Rc<executor::Foreground>,
-    ) -> Box<dyn platform::Window> {
-        Box::new(Window::open(id, options, executor, self.fonts()))
-    }
-
-    fn key_window_id(&self) -> Option<usize> {
-        Window::key_window_id()
-    }
-
     fn prompt_for_paths(
         &self,
         options: platform::PathPromptOptions,
@@ -353,6 +295,64 @@ impl platform::Platform for MacPlatform {
             let _: () = msg_send![panel, beginWithCompletionHandler: block];
         }
     }
+}
+
+pub struct MacPlatform {
+    dispatcher: Arc<Dispatcher>,
+    fonts: Arc<FontSystem>,
+    pasteboard: id,
+    text_hash_pasteboard_type: id,
+    metadata_pasteboard_type: id,
+}
+
+impl MacPlatform {
+    pub fn new() -> Self {
+        Self {
+            dispatcher: Arc::new(Dispatcher),
+            fonts: Arc::new(FontSystem::new()),
+            pasteboard: unsafe { NSPasteboard::generalPasteboard(nil) },
+            text_hash_pasteboard_type: unsafe { ns_string("zed-text-hash") },
+            metadata_pasteboard_type: unsafe { ns_string("zed-metadata") },
+        }
+    }
+
+    unsafe fn read_from_pasteboard(&self, kind: id) -> Option<&[u8]> {
+        let data = self.pasteboard.dataForType(kind);
+        if data == nil {
+            None
+        } else {
+            Some(slice::from_raw_parts(
+                data.bytes() as *mut u8,
+                data.length() as usize,
+            ))
+        }
+    }
+}
+
+impl platform::Platform for MacPlatform {
+    fn dispatcher(&self) -> Arc<dyn platform::Dispatcher> {
+        self.dispatcher.clone()
+    }
+
+    fn activate(&self, ignoring_other_apps: bool) {
+        unsafe {
+            let app = NSApplication::sharedApplication(nil);
+            app.activateIgnoringOtherApps_(ignoring_other_apps.to_objc());
+        }
+    }
+
+    fn open_window(
+        &self,
+        id: usize,
+        options: platform::WindowOptions,
+        executor: Rc<executor::Foreground>,
+    ) -> Box<dyn platform::Window> {
+        Box::new(Window::open(id, options, executor, self.fonts()))
+    }
+
+    fn key_window_id(&self) -> Option<usize> {
+        Window::key_window_id()
+    }
 
     fn fonts(&self) -> Arc<dyn platform::FontSystem> {
         self.fonts.clone()

gpui/src/platform/test.rs 🔗

@@ -12,10 +12,12 @@ pub(crate) struct Platform {
     dispatcher: Arc<dyn super::Dispatcher>,
     fonts: Arc<dyn super::FontSystem>,
     current_clipboard_item: RefCell<Option<ClipboardItem>>,
-    last_prompt_for_new_path_args: RefCell<Option<(PathBuf, Box<dyn FnOnce(Option<PathBuf>)>)>>,
 }
 
-pub(crate) struct MainThreadPlatform;
+#[derive(Default)]
+pub(crate) struct MainThreadPlatform {
+    last_prompt_for_new_path_args: RefCell<Option<(PathBuf, Box<dyn FnOnce(Option<PathBuf>)>)>>,
+}
 
 struct Dispatcher;
 
@@ -29,9 +31,24 @@ pub struct Window {
     pub(crate) last_prompt: RefCell<Option<Box<dyn FnOnce(usize)>>>,
 }
 
-impl super::MainThreadPlatform for MainThreadPlatform {
-    fn on_menu_command(&self, _: Box<dyn FnMut(&str, Option<&dyn Any>)>) {}
+impl MainThreadPlatform {
+    pub(crate) fn simulate_new_path_selection(
+        &self,
+        result: impl FnOnce(PathBuf) -> Option<PathBuf>,
+    ) {
+        let (dir_path, callback) = self
+            .last_prompt_for_new_path_args
+            .take()
+            .expect("prompt_for_new_path was not called");
+        callback(result(dir_path));
+    }
+
+    pub(crate) fn did_prompt_for_new_path(&self) -> bool {
+        self.last_prompt_for_new_path_args.borrow().is_some()
+    }
+}
 
+impl super::MainThreadPlatform for MainThreadPlatform {
     fn on_become_active(&self, _: Box<dyn FnMut()>) {}
 
     fn on_resign_active(&self, _: Box<dyn FnMut()>) {}
@@ -40,11 +57,24 @@ impl super::MainThreadPlatform for MainThreadPlatform {
 
     fn on_open_files(&self, _: Box<dyn FnMut(Vec<std::path::PathBuf>)>) {}
 
-    fn set_menus(&self, _: Vec<crate::Menu>) {}
-
     fn run(&self, _on_finish_launching: Box<dyn FnOnce() -> ()>) {
         unimplemented!()
     }
+
+    fn on_menu_command(&self, _: Box<dyn FnMut(&str, Option<&dyn Any>)>) {}
+
+    fn set_menus(&self, _: Vec<crate::Menu>) {}
+
+    fn prompt_for_paths(
+        &self,
+        _: super::PathPromptOptions,
+        _: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
+    ) {
+    }
+
+    fn prompt_for_new_path(&self, path: &Path, f: Box<dyn FnOnce(Option<std::path::PathBuf>)>) {
+        *self.last_prompt_for_new_path_args.borrow_mut() = Some((path.to_path_buf(), f));
+    }
 }
 
 impl Platform {
@@ -53,24 +83,8 @@ impl Platform {
             dispatcher: Arc::new(Dispatcher),
             fonts: Arc::new(super::current::FontSystem::new()),
             current_clipboard_item: Default::default(),
-            last_prompt_for_new_path_args: Default::default(),
         }
     }
-
-    pub(crate) fn simulate_new_path_selection(
-        &self,
-        result: impl FnOnce(PathBuf) -> Option<PathBuf>,
-    ) {
-        let (dir_path, callback) = self
-            .last_prompt_for_new_path_args
-            .take()
-            .expect("prompt_for_new_path was not called");
-        callback(result(dir_path));
-    }
-
-    pub(crate) fn did_prompt_for_new_path(&self) -> bool {
-        self.last_prompt_for_new_path_args.borrow().is_some()
-    }
 }
 
 impl super::Platform for Platform {
@@ -99,17 +113,6 @@ impl super::Platform for Platform {
 
     fn quit(&self) {}
 
-    fn prompt_for_paths(
-        &self,
-        _: super::PathPromptOptions,
-        _: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
-    ) {
-    }
-
-    fn prompt_for_new_path(&self, path: &Path, f: Box<dyn FnOnce(Option<std::path::PathBuf>)>) {
-        *self.last_prompt_for_new_path_args.borrow_mut() = Some((path.to_path_buf(), f));
-    }
-
     fn write_to_clipboard(&self, item: ClipboardItem) {
         *self.current_clipboard_item.borrow_mut() = Some(item);
     }
@@ -180,7 +183,7 @@ impl super::Window for Window {
 }
 
 pub(crate) fn main_thread_platform() -> MainThreadPlatform {
-    MainThreadPlatform
+    MainThreadPlatform::default()
 }
 
 pub(crate) fn platform() -> Platform {