Avoid using global for `Room` and extract that logic into `ActiveCall`

Antonio Scandurra created

Change summary

crates/collab_ui/src/contacts_popover.rs | 40 +++++---------
crates/room/src/active_call.rs           | 67 ++++++++++++++++++-------
crates/room/src/room.rs                  | 33 ------------
3 files changed, 63 insertions(+), 77 deletions(-)

Detailed changes

crates/collab_ui/src/contacts_popover.rs 🔗

@@ -9,7 +9,7 @@ use gpui::{
     ViewHandle,
 };
 use menu::{Confirm, SelectNext, SelectPrev};
-use room::Room;
+use room::{ActiveCall, Room};
 use settings::Settings;
 use theme::IconButton;
 
@@ -80,7 +80,7 @@ pub enum Event {
 }
 
 pub struct ContactsPopover {
-    room: Option<(ModelHandle<Room>, Subscription)>,
+    room_subscription: Option<Subscription>,
     entries: Vec<ContactEntry>,
     match_candidates: Vec<StringMatchCandidate>,
     list_state: ListState,
@@ -161,18 +161,20 @@ impl ContactsPopover {
             }
         });
 
+        let active_call = ActiveCall::global(cx);
         let mut subscriptions = Vec::new();
         subscriptions.push(cx.observe(&user_store, |this, _, cx| this.update_entries(cx)));
-
-        let weak_self = cx.weak_handle();
-        subscriptions.push(Room::observe(cx, move |room, cx| {
-            if let Some(this) = weak_self.upgrade(cx) {
-                this.update(cx, |this, cx| this.set_room(room, cx));
+        subscriptions.push(cx.observe(&active_call, |this, active_call, cx| {
+            if let Some(room) = active_call.read(cx).room().cloned() {
+                this.room_subscription = Some(cx.observe(&room, |_, _, cx| cx.notify()));
+            } else {
+                this.room_subscription = None;
             }
+            cx.notify();
         }));
 
         let mut this = Self {
-            room: None,
+            room_subscription: None,
             list_state,
             selection: None,
             collapsed_sections: Default::default(),
@@ -187,17 +189,6 @@ impl ContactsPopover {
         this
     }
 
-    fn set_room(&mut self, room: Option<ModelHandle<Room>>, cx: &mut ViewContext<Self>) {
-        if let Some(room) = room {
-            let observation = cx.observe(&room, |_, _, cx| cx.notify());
-            self.room = Some((room, observation));
-        } else {
-            self.room = None;
-        }
-
-        cx.notify();
-    }
-
     fn clear_filter(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
         let did_clear = self.filter_editor.update(cx, |editor, cx| {
             if editor.buffer().read(cx).len(cx) > 0 {
@@ -394,7 +385,7 @@ impl ContactsPopover {
     }
 
     fn render_active_call(&self, cx: &mut RenderContext<Self>) -> Option<ElementBox> {
-        let (room, _) = self.room.as_ref()?;
+        let room = ActiveCall::global(cx).read(cx).room()?;
         let theme = &cx.global::<Settings>().theme.contacts_panel;
 
         Some(
@@ -642,13 +633,12 @@ impl ContactsPopover {
     }
 
     fn call(&mut self, action: &Call, cx: &mut ViewContext<Self>) {
-        let client = self.client.clone();
-        let user_store = self.user_store.clone();
         let recipient_user_id = action.recipient_user_id;
+        let room = ActiveCall::global(cx).update(cx, |active_call, cx| {
+            active_call.get_or_create(&self.client, &self.user_store, cx)
+        });
         cx.spawn_weak(|_, mut cx| async move {
-            let room = cx
-                .update(|cx| Room::get_or_create(&client, &user_store, cx))
-                .await?;
+            let room = room.await?;
             room.update(&mut cx, |room, cx| room.call(recipient_user_id, cx))
                 .await?;
             anyhow::Ok(())

crates/room/src/active_call.rs 🔗

@@ -1,5 +1,9 @@
 use crate::Room;
-use gpui::{Entity, ModelHandle, MutableAppContext};
+use anyhow::{anyhow, Result};
+use client::{call::Call, Client, UserStore};
+use gpui::{Entity, ModelContext, ModelHandle, MutableAppContext, Task};
+use std::sync::Arc;
+use util::ResultExt;
 
 #[derive(Default)]
 pub struct ActiveCall {
@@ -13,43 +17,66 @@ impl Entity for ActiveCall {
 impl ActiveCall {
     pub fn global(cx: &mut MutableAppContext) -> ModelHandle<Self> {
         if cx.has_global::<ModelHandle<Self>>() {
+            cx.global::<ModelHandle<Self>>().clone()
+        } else {
             let active_call = cx.add_model(|_| ActiveCall::default());
             cx.set_global(active_call.clone());
             active_call
-        } else {
-            cx.global::<ModelHandle<Self>>().clone()
         }
     }
 
-    pub fn observe<F>(cx: &mut MutableAppContext, mut callback: F) -> gpui::Subscription
-    where
-        F: 'static + FnMut(Option<ModelHandle<Room>>, &mut MutableAppContext),
-    {
-        cx.observe_default_global::<Option<ModelHandle<Room>>, _>(move |cx| {
-            let room = cx.global::<Option<ModelHandle<Room>>>().clone();
-            callback(room, cx);
-        })
-    }
-
     pub fn get_or_create(
+        &mut self,
         client: &Arc<Client>,
         user_store: &ModelHandle<UserStore>,
-        cx: &mut MutableAppContext,
+        cx: &mut ModelContext<Self>,
     ) -> Task<Result<ModelHandle<Room>>> {
-        if let Some(room) = cx.global::<Option<ModelHandle<Room>>>() {
-            Task::ready(Ok(room.clone()))
+        if let Some(room) = self.room.clone() {
+            Task::ready(Ok(room))
         } else {
             let client = client.clone();
             let user_store = user_store.clone();
-            cx.spawn(|mut cx| async move {
+            cx.spawn(|this, mut cx| async move {
                 let room = cx.update(|cx| Room::create(client, user_store, cx)).await?;
-                cx.update(|cx| cx.set_global(Some(room.clone())));
+                this.update(&mut cx, |this, cx| {
+                    this.room = Some(room.clone());
+                    cx.notify();
+                });
                 Ok(room)
             })
         }
     }
 
-    pub fn clear(cx: &mut MutableAppContext) {
-        cx.set_global::<Option<ModelHandle<Room>>>(None);
+    pub fn join(
+        &mut self,
+        call: &Call,
+        client: &Arc<Client>,
+        user_store: &ModelHandle<UserStore>,
+        cx: &mut ModelContext<Self>,
+    ) -> Task<Result<ModelHandle<Room>>> {
+        if self.room.is_some() {
+            return Task::ready(Err(anyhow!("cannot join while on another call")));
+        }
+
+        let join = Room::join(call, client.clone(), user_store.clone(), cx);
+        cx.spawn(|this, mut cx| async move {
+            let room = join.await?;
+            this.update(&mut cx, |this, cx| {
+                this.room = Some(room.clone());
+                cx.notify();
+            });
+            Ok(room)
+        })
+    }
+
+    pub fn room(&self) -> Option<&ModelHandle<Room>> {
+        self.room.as_ref()
+    }
+
+    pub fn clear(&mut self, cx: &mut ModelContext<Self>) {
+        if let Some(room) = self.room.take() {
+            room.update(cx, |room, cx| room.leave(cx)).log_err();
+            cx.notify();
+        }
     }
 }

crates/room/src/room.rs 🔗

@@ -1,6 +1,7 @@
 mod active_call;
 mod participant;
 
+pub use active_call::ActiveCall;
 use anyhow::{anyhow, Result};
 use client::{call::Call, proto, Client, PeerId, TypedEnvelope, User, UserStore};
 use collections::HashMap;
@@ -32,38 +33,6 @@ impl Entity for Room {
 }
 
 impl Room {
-    pub fn observe<F>(cx: &mut MutableAppContext, mut callback: F) -> gpui::Subscription
-    where
-        F: 'static + FnMut(Option<ModelHandle<Self>>, &mut MutableAppContext),
-    {
-        cx.observe_default_global::<Option<ModelHandle<Self>>, _>(move |cx| {
-            let room = cx.global::<Option<ModelHandle<Self>>>().clone();
-            callback(room, cx);
-        })
-    }
-
-    pub fn get_or_create(
-        client: &Arc<Client>,
-        user_store: &ModelHandle<UserStore>,
-        cx: &mut MutableAppContext,
-    ) -> Task<Result<ModelHandle<Self>>> {
-        if let Some(room) = cx.global::<Option<ModelHandle<Self>>>() {
-            Task::ready(Ok(room.clone()))
-        } else {
-            let client = client.clone();
-            let user_store = user_store.clone();
-            cx.spawn(|mut cx| async move {
-                let room = cx.update(|cx| Room::create(client, user_store, cx)).await?;
-                cx.update(|cx| cx.set_global(Some(room.clone())));
-                Ok(room)
-            })
-        }
-    }
-
-    pub fn clear(cx: &mut MutableAppContext) {
-        cx.set_global::<Option<ModelHandle<Self>>>(None);
-    }
-
     fn new(
         id: u64,
         client: Arc<Client>,