diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 0b3dd26f554ef76d2e2e87e3a29d7d60b1b58584..1e4448de98ef5017845c0d552b20778f456c6a89 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1113,7 +1113,7 @@ impl MutableAppContext { { let subscription_id = post_inc(&mut self.next_subscription_id); let type_id = TypeId::of::(); - self.pending_effects.push_back(Effect::SubscribeGlobal { + self.pending_effects.push_back(Effect::GlobalSubscription { type_id, subscription_id, callback: Box::new(move |payload, cx| { @@ -1150,7 +1150,7 @@ impl MutableAppContext { { let subscription_id = post_inc(&mut self.next_subscription_id); let emitter = handle.downgrade(); - self.pending_effects.push_back(Effect::Subscribe { + self.pending_effects.push_back(Effect::Subscription { entity_id: handle.id(), subscription_id, callback: Box::new(move |payload, cx| { @@ -1176,25 +1176,23 @@ impl MutableAppContext { H: Handle, F: 'static + FnMut(H, &mut Self) -> bool, { - let id = post_inc(&mut self.next_subscription_id); + let subscription_id = post_inc(&mut self.next_subscription_id); let observed = handle.downgrade(); - self.observations - .lock() - .entry(handle.id()) - .or_default() - .insert( - id, - Some(Box::new(move |cx| { - if let Some(observed) = H::upgrade_from(&observed, cx) { - callback(observed, cx) - } else { - false - } - })), - ); + let entity_id = handle.id(); + self.pending_effects.push_back(Effect::Observation { + entity_id, + subscription_id, + callback: Box::new(move |cx| { + if let Some(observed) = H::upgrade_from(&observed, cx) { + callback(observed, cx) + } else { + false + } + }), + }); Subscription::Observation { - id, - entity_id: handle.id(), + id: subscription_id, + entity_id, observations: Some(Arc::downgrade(&self.observations)), } } @@ -1650,20 +1648,27 @@ impl MutableAppContext { loop { if let Some(effect) = self.pending_effects.pop_front() { match effect { - Effect::Subscribe { + Effect::Subscription { entity_id, subscription_id, callback, - } => self.handle_subscribe_effect(entity_id, subscription_id, callback), + } => self.handle_subscription_effect(entity_id, subscription_id, callback), Effect::Event { entity_id, payload } => self.emit_event(entity_id, payload), - Effect::SubscribeGlobal { + Effect::GlobalSubscription { type_id, subscription_id, callback, - } => { - self.handle_subscribe_global_effect(type_id, subscription_id, callback) - } + } => self.handle_global_subscription_effect( + type_id, + subscription_id, + callback, + ), Effect::GlobalEvent { payload } => self.emit_global_event(payload), + Effect::Observation { + entity_id, + subscription_id, + callback, + } => self.handle_observation_effect(entity_id, subscription_id, callback), Effect::ModelNotification { model_id } => { self.notify_model_observers(model_id) } @@ -1778,7 +1783,7 @@ impl MutableAppContext { } } - fn handle_subscribe_effect( + fn handle_subscription_effect( &mut self, entity_id: usize, subscription_id: usize, @@ -1829,7 +1834,7 @@ impl MutableAppContext { } } - fn handle_subscribe_global_effect( + fn handle_global_subscription_effect( &mut self, type_id: TypeId, subscription_id: usize, @@ -1879,6 +1884,30 @@ impl MutableAppContext { } } + fn handle_observation_effect( + &mut self, + entity_id: usize, + subscription_id: usize, + callback: ObservationCallback, + ) { + match self + .observations + .lock() + .entry(entity_id) + .or_default() + .entry(subscription_id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(Some(callback)); + } + // Observation was dropped before effect was processed + btree_map::Entry::Occupied(entry) => { + debug_assert!(entry.get().is_none()); + entry.remove(); + } + } + } + fn notify_model_observers(&mut self, observed_id: usize) { let callbacks = self.observations.lock().remove(&observed_id); if let Some(callbacks) = callbacks { @@ -2289,7 +2318,7 @@ pub struct WindowInvalidation { } pub enum Effect { - Subscribe { + Subscription { entity_id: usize, subscription_id: usize, callback: SubscriptionCallback, @@ -2298,7 +2327,7 @@ pub enum Effect { entity_id: usize, payload: Box, }, - SubscribeGlobal { + GlobalSubscription { type_id: TypeId, subscription_id: usize, callback: GlobalSubscriptionCallback, @@ -2306,6 +2335,11 @@ pub enum Effect { GlobalEvent { payload: Box, }, + Observation { + entity_id: usize, + subscription_id: usize, + callback: ObservationCallback, + }, ModelNotification { model_id: usize, }, @@ -2335,7 +2369,7 @@ pub enum Effect { impl Debug for Effect { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Effect::Subscribe { + Effect::Subscription { entity_id, subscription_id, .. @@ -2348,7 +2382,7 @@ impl Debug for Effect { .debug_struct("Effect::Event") .field("entity_id", entity_id) .finish(), - Effect::SubscribeGlobal { + Effect::GlobalSubscription { type_id, subscription_id, .. @@ -2361,6 +2395,15 @@ impl Debug for Effect { .debug_struct("Effect::GlobalEvent") .field("type_id", &(&*payload).type_id()) .finish(), + Effect::Observation { + entity_id, + subscription_id, + .. + } => f + .debug_struct("Effect::Observation") + .field("entity_id", entity_id) + .field("subscription_id", subscription_id) + .finish(), Effect::ModelNotification { model_id } => f .debug_struct("Effect::ModelNotification") .field("model_id", model_id) @@ -4548,6 +4591,37 @@ mod tests { assert_eq!(handle_1.read(cx).events, vec![7, 5, 10]) } + #[crate::test(self)] + fn test_model_notify_before_observe_in_same_update_cycle(cx: &mut MutableAppContext) { + #[derive(Default)] + struct Model; + + impl Entity for Model { + type Event = (); + } + + let events = Rc::new(RefCell::new(Vec::new())); + cx.add_model(|cx| { + drop(cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("dropped before flush") + })); + cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("before notify") + }) + .detach(); + cx.notify(); + cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("after notify") + }) + .detach(); + Model + }); + assert_eq!(*events.borrow(), ["before notify"]); + } + #[crate::test(self)] fn test_view_handles(cx: &mut MutableAppContext) { struct View { @@ -4843,7 +4917,9 @@ mod tests { } #[crate::test(self)] - fn test_global_events_emitted_before_subscription(cx: &mut MutableAppContext) { + fn test_global_events_emitted_before_subscription_in_same_update_cycle( + cx: &mut MutableAppContext, + ) { let events = Rc::new(RefCell::new(Vec::new())); cx.update(|cx| { { @@ -5050,6 +5126,47 @@ mod tests { assert_eq!(view.read(cx).events, vec![11]); } + #[crate::test(self)] + fn test_view_notify_before_observe_in_same_update_cycle(cx: &mut MutableAppContext) { + #[derive(Default)] + struct TestView; + + impl Entity for TestView { + type Event = (); + } + + impl View for TestView { + fn ui_name() -> &'static str { + "TestView" + } + + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + Empty::new().boxed() + } + } + + let events = Rc::new(RefCell::new(Vec::new())); + cx.add_window(Default::default(), |cx| { + drop(cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("dropped before flush") + })); + cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("before notify") + }) + .detach(); + cx.notify(); + cx.observe(&cx.handle(), { + let events = events.clone(); + move |_, _, _| events.borrow_mut().push("after notify") + }) + .detach(); + TestView + }); + assert_eq!(*events.borrow(), ["before notify"]); + } + #[crate::test(self)] fn test_dropping_observers(cx: &mut MutableAppContext) { struct View; diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 9527df8e1f4634331ae301127d62067872d40c04..53d1315662ec7cd585be71667d3be9ac06f81cce 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -4505,10 +4505,10 @@ mod tests { cx_a.foreground().run_until_parked(); // Ensure leader updates don't change the active pane of followers - workspace_a.read_with(cx_a, |workspace, cx| { + workspace_a.read_with(cx_a, |workspace, _| { assert_ne!(*workspace.active_pane(), pane_a1); }); - workspace_b.read_with(cx_b, |workspace, cx| { + workspace_b.read_with(cx_b, |workspace, _| { assert_ne!(*workspace.active_pane(), pane_b1); });