Remove new types

Mikayla Maki created

Change summary

crates/gpui/src/app.rs                      | 27 +++----
crates/gpui/src/app/async_context.rs        |  9 +-
crates/gpui/src/app/test_context.rs         |  2 
crates/gpui/src/executor.rs                 | 50 ++++++--------
crates/gpui/src/platform.rs                 | 79 +++++-----------------
crates/gpui/src/platform/mac/dispatcher.rs  |  8 -
crates/gpui/src/platform/test/dispatcher.rs | 10 +-
7 files changed, 64 insertions(+), 121 deletions(-)

Detailed changes

crates/gpui/src/app.rs 🔗

@@ -34,15 +34,15 @@ use util::{ResultExt, debug_panic};
 #[cfg(any(feature = "inspector", debug_assertions))]
 use crate::InspectorElementRegistry;
 use crate::{
-    Action, ActionBuildError, ActionRegistry, Any, AnyView, AnyWindowHandle, AppContext,
-    AppLiveness, Asset, AssetSource, BackgroundExecutor, Bounds, ClipboardItem, CursorStyle,
-    DispatchPhase, DisplayId, EventEmitter, FocusHandle, FocusMap, ForegroundExecutor, Global,
-    KeyBinding, KeyContext, Keymap, Keystroke, LayoutId, Menu, MenuItem, OwnedMenu,
-    PathPromptOptions, Pixels, Platform, PlatformDisplay, PlatformKeyboardLayout,
-    PlatformKeyboardMapper, Point, Priority, PromptBuilder, PromptButton, PromptHandle,
-    PromptLevel, Render, RenderImage, RenderablePromptHandle, Reservation, ScreenCaptureSource,
-    SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextSystem, Window,
-    WindowAppearance, WindowHandle, WindowId, WindowInvalidator,
+    Action, ActionBuildError, ActionRegistry, Any, AnyView, AnyWindowHandle, AppContext, Asset,
+    AssetSource, BackgroundExecutor, Bounds, ClipboardItem, CursorStyle, DispatchPhase, DisplayId,
+    EventEmitter, FocusHandle, FocusMap, ForegroundExecutor, Global, KeyBinding, KeyContext,
+    Keymap, Keystroke, LayoutId, Menu, MenuItem, OwnedMenu, PathPromptOptions, Pixels, Platform,
+    PlatformDisplay, PlatformKeyboardLayout, PlatformKeyboardMapper, Point, Priority,
+    PromptBuilder, PromptButton, PromptHandle, PromptLevel, Render, RenderImage,
+    RenderablePromptHandle, Reservation, ScreenCaptureSource, SharedString, SubscriberSet,
+    Subscription, SvgRenderer, Task, TextSystem, Window, WindowAppearance, WindowHandle, WindowId,
+    WindowInvalidator,
     colors::{Colors, GlobalColors},
     current_platform, hash, init_app_menus,
 };
@@ -580,9 +580,7 @@ impl GpuiMode {
 /// You need a reference to an `App` to access the state of a [Entity].
 pub struct App {
     pub(crate) this: Weak<AppCell>,
-    /// Tracks whether this app is still alive. Used to cancel foreground tasks
-    /// when the app is dropped.
-    pub(crate) liveness: AppLiveness,
+    pub(crate) liveness: std::sync::Arc<()>,
     pub(crate) platform: Rc<dyn Platform>,
     pub(crate) mode: GpuiMode,
     text_system: Arc<TextSystem>,
@@ -661,7 +659,7 @@ impl App {
         let app = Rc::new_cyclic(|this| AppCell {
             app: RefCell::new(App {
                 this: this.clone(),
-                liveness: AppLiveness::new(),
+                liveness: std::sync::Arc::new(()),
                 platform: platform.clone(),
                 text_system,
                 mode: GpuiMode::Production,
@@ -1478,10 +1476,9 @@ impl App {
     /// Creates an `AsyncApp`, which can be cloned and has a static lifetime
     /// so it can be held across `await` points.
     pub fn to_async(&self) -> AsyncApp {
-        let liveness_token = self.liveness.token();
         AsyncApp {
             app: self.this.clone(),
-            liveness_token,
+            liveness_token: std::sync::Arc::downgrade(&self.liveness),
             background_executor: self.background_executor.clone(),
             foreground_executor: self.foreground_executor.clone(),
         }

crates/gpui/src/app/async_context.rs 🔗

@@ -1,8 +1,7 @@
 use crate::{
-    AnyView, AnyWindowHandle, App, AppCell, AppContext, AppLivenessToken, BackgroundExecutor,
-    BorrowAppContext, Entity, EventEmitter, Focusable, ForegroundExecutor, Global, PromptButton,
-    PromptLevel, Render, Reservation, Result, Subscription, Task, VisualContext, Window,
-    WindowHandle,
+    AnyView, AnyWindowHandle, App, AppCell, AppContext, BackgroundExecutor, BorrowAppContext,
+    Entity, EventEmitter, Focusable, ForegroundExecutor, Global, PromptButton, PromptLevel, Render,
+    Reservation, Result, Subscription, Task, VisualContext, Window, WindowHandle,
 };
 use anyhow::{Context as _, anyhow};
 use derive_more::{Deref, DerefMut};
@@ -17,7 +16,7 @@ use super::{Context, WeakEntity};
 #[derive(Clone)]
 pub struct AsyncApp {
     pub(crate) app: Weak<AppCell>,
-    pub(crate) liveness_token: AppLivenessToken,
+    pub(crate) liveness_token: std::sync::Weak<()>,
     pub(crate) background_executor: BackgroundExecutor,
     pub(crate) foreground_executor: ForegroundExecutor,
 }

crates/gpui/src/app/test_context.rs 🔗

@@ -405,7 +405,7 @@ impl TestAppContext {
     pub fn to_async(&self) -> AsyncApp {
         AsyncApp {
             app: Rc::downgrade(&self.app),
-            liveness_token: self.app.borrow().liveness.token(),
+            liveness_token: std::sync::Arc::downgrade(&self.app.borrow().liveness),
             background_executor: self.background_executor.clone(),
             foreground_executor: self.foreground_executor.clone(),
         }

crates/gpui/src/executor.rs 🔗

@@ -1,6 +1,4 @@
-use crate::{
-    App, AppLivenessToken, PlatformDispatcher, RunnableMeta, RunnableVariant, TaskTiming, profiler,
-};
+use crate::{App, PlatformDispatcher, RunnableMeta, RunnableVariant, TaskTiming, profiler};
 use async_task::Runnable;
 use futures::channel::mpsc;
 use parking_lot::{Condvar, Mutex};
@@ -787,7 +785,7 @@ impl ForegroundExecutor {
     #[track_caller]
     pub(crate) fn spawn_context<R>(
         &self,
-        app: AppLivenessToken,
+        app: std::sync::Weak<()>,
         future: impl Future<Output = R> + 'static,
     ) -> Task<R>
     where
@@ -799,7 +797,7 @@ impl ForegroundExecutor {
     #[track_caller]
     pub(crate) fn inner_spawn<R>(
         &self,
-        app: Option<AppLivenessToken>,
+        app: Option<std::sync::Weak<()>>,
         priority: Priority,
         future: impl Future<Output = R> + 'static,
     ) -> Task<R>
@@ -814,7 +812,7 @@ impl ForegroundExecutor {
             dispatcher: Arc<dyn PlatformDispatcher>,
             future: AnyLocalFuture<R>,
             location: &'static core::panic::Location<'static>,
-            app: Option<AppLivenessToken>,
+            app: Option<std::sync::Weak<()>>,
             priority: Priority,
         ) -> Task<R> {
             let (runnable, task) = spawn_local_with_source_location(
@@ -980,7 +978,7 @@ mod test {
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
 
         let task_ran = Rc::new(RefCell::new(false));
 
@@ -1015,7 +1013,7 @@ mod test {
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
         let app_weak = Rc::downgrade(&app);
 
         let task_ran = Rc::new(RefCell::new(false));
@@ -1052,7 +1050,7 @@ mod test {
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
         let app_weak = Rc::downgrade(&app);
 
         let outer_completed = Rc::new(RefCell::new(false));
@@ -1070,25 +1068,20 @@ mod test {
         let inner_executor = foreground_executor.clone();
         let inner_liveness_token = liveness_token.clone();
 
-        // Spawn outer task that will spawn and await an inner task
         foreground_executor
             .spawn_context(liveness_token, async move {
-                let inner_flag_clone = Rc::clone(&inner_flag);
-
-                // Spawn inner task that blocks on a channel
-                let inner_task = inner_executor.spawn_context(inner_liveness_token, async move {
-                    // Wait for signal (which will never come - we'll drop the app instead)
-                    rx.await.ok();
-                    *inner_flag_clone.borrow_mut() = true;
+                let inner_task = inner_executor.spawn_context(inner_liveness_token, {
+                    let inner_flag = Rc::clone(&inner_flag);
+                    async move {
+                        rx.await.ok();
+                        *inner_flag.borrow_mut() = true;
+                    }
                 });
 
-                // Mark that we've reached the await point
                 *await_flag.borrow_mut() = true;
 
-                // Await inner task - this should not panic when both are cancelled
                 inner_task.await;
 
-                // Mark outer as complete (should never reach here)
                 *outer_flag.borrow_mut() = true;
             })
             .detach();
@@ -1178,7 +1171,7 @@ mod test {
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
         let app_weak = Rc::downgrade(&app);
 
         let task = foreground_executor.spawn_context(liveness_token, async move { 42 });
@@ -1204,7 +1197,7 @@ mod test {
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
         let app_weak = Rc::downgrade(&app);
 
         let task = foreground_executor
@@ -1223,23 +1216,22 @@ mod test {
 
     #[test]
     fn test_app_liveness_token_can_be_dropped_on_background_thread() {
-        let dispatcher = TestDispatcher::new(StdRng::seed_from_u64(0));
-        let arc_dispatcher = Arc::new(dispatcher.clone());
-        let background_executor = BackgroundExecutor::new(arc_dispatcher.clone());
-        let foreground_executor = ForegroundExecutor::new(arc_dispatcher);
+        let dispatcher = Arc::new(TestDispatcher::new(StdRng::seed_from_u64(0)));
+        let background_executor = BackgroundExecutor::new(dispatcher.clone());
+        let foreground_executor = ForegroundExecutor::new(dispatcher);
 
         let platform = TestPlatform::new(background_executor, foreground_executor);
         let asset_source = Arc::new(());
         let http_client = http_client::FakeHttpClient::with_404_response();
 
         let app = App::new_app(platform, asset_source, http_client);
-        let liveness_token = app.borrow().liveness.token();
+        let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness);
 
-        // Drop the token on a real background thread.
+        // Dispatcher is single threaded when testing, so we need to spawn a real thread
         std::thread::spawn(move || {
             drop(liveness_token);
         })
         .join()
-        .expect("Dropping AppLivenessToken on background thread should not panic");
+        .expect("Dropping Weak<()> on background thread should not panic");
     }
 }

crates/gpui/src/platform.rs 🔗

@@ -572,74 +572,33 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle {
     }
 }
 
-/// Tracks whether an App is still alive. This is used to cancel foreground tasks
-/// when the app is dropped.
-///
-#[derive(Clone)]
+/// This type is public so that our test macro can generate and use it, but it should not
+/// be considered part of our public API.
 #[doc(hidden)]
-pub struct AppLiveness {
-    sentinel: std::sync::Arc<()>,
-}
-
-impl AppLiveness {
-    /// Creates a new AppLiveness, initially alive.
-    pub fn new() -> Self {
-        Self {
-            sentinel: std::sync::Arc::new(()),
-        }
-    }
-
-    /// Returns a token that can be stored in task metadata to check liveness.
-    pub fn token(&self) -> AppLivenessToken {
-        AppLivenessToken {
-            sentinel: std::sync::Arc::downgrade(&self.sentinel),
-        }
-    }
-}
-
-impl Default for AppLiveness {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl std::fmt::Debug for AppLiveness {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("AppLiveness").field("alive", &true).finish()
-    }
-}
-
-/// A token that can be stored in task metadata to check if the app is still alive.
-#[derive(Clone)]
-pub struct AppLivenessToken {
-    sentinel: std::sync::Weak<()>,
-}
-
-impl AppLivenessToken {
-    /// Returns true if the app is still alive.
-    pub fn is_alive(&self) -> bool {
-        self.sentinel.strong_count() > 0
-    }
+pub struct RunnableMeta {
+    /// Location of the runnable
+    pub location: &'static core::panic::Location<'static>,
+    /// Weak reference to check if the app is still alive before running this task
+    pub app: Option<std::sync::Weak<()>>,
 }
 
-impl std::fmt::Debug for AppLivenessToken {
+impl std::fmt::Debug for RunnableMeta {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("AppLivenessToken")
-            .field("alive", &self.is_alive())
+        f.debug_struct("RunnableMeta")
+            .field("location", &self.location)
+            .field("app_alive", &self.is_app_alive())
             .finish()
     }
 }
 
-/// This type is public so that our test macro can generate and use it, but it should not
-/// be considered part of our public API.
-#[doc(hidden)]
-#[derive(Debug)]
-pub struct RunnableMeta {
-    /// Location of the runnable
-    pub location: &'static core::panic::Location<'static>,
-    /// Token to check if the app is still alive before running.
-    /// This is `Some` for foreground tasks spawned with app tracking.
-    pub app: Option<AppLivenessToken>,
+impl RunnableMeta {
+    /// Returns true if the app is still alive (or if no app tracking is configured).
+    pub fn is_app_alive(&self) -> bool {
+        match &self.app {
+            Some(weak) => weak.strong_count() > 0,
+            None => true,
+        }
+    }
 }
 
 #[doc(hidden)]

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

@@ -254,11 +254,9 @@ extern "C" fn trampoline(runnable: *mut c_void) {
     let metadata = task.metadata();
     let location = metadata.location;
 
-    if let Some(ref app_token) = metadata.app {
-        if !app_token.is_alive() {
-            drop(task);
-            return;
-        }
+    if !metadata.is_app_alive() {
+        drop(task);
+        return;
     }
 
     let start = Instant::now();

crates/gpui/src/platform/test/dispatcher.rs 🔗

@@ -178,12 +178,10 @@ impl TestDispatcher {
         // todo(localcc): add timings to tests
         match runnable {
             RunnableVariant::Meta(runnable) => {
-                if let Some(ref app_token) = runnable.metadata().app {
-                    if !app_token.is_alive() {
-                        drop(runnable);
-                        self.state.lock().is_main_thread = was_main_thread;
-                        return true;
-                    }
+                if !runnable.metadata().is_app_alive() {
+                    drop(runnable);
+                    self.state.lock().is_main_thread = was_main_thread;
+                    return true;
                 }
                 runnable.run()
             }