Set location on `ActiveCall` even before there's a room

Antonio Scandurra created 3 years ago

We will automatically call `Room::set_location` once a room has been
assigned.

Change summary

crates/call/src/call.rs                      | 32 ++++++++--
crates/call/src/room.rs                      |  2 
crates/collab/src/integration_tests.rs       | 61 ++++++++++++++++-----
crates/collab_ui/src/collab_titlebar_item.rs | 14 +---
crates/collab_ui/src/contact_list.rs         | 24 +------
5 files changed, 80 insertions(+), 53 deletions(-)

Detailed changes

crates/call/src/call.rs

@@ -5,7 +5,7 @@ use anyhow::{anyhow, Result};
 use client::{proto, Client, TypedEnvelope, User, UserStore};
 use gpui::{
     AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext,
-    Subscription, Task,
+    Subscription, Task, WeakModelHandle,
 };
 pub use participant::ParticipantLocation;
 use postage::watch;
@@ -27,6 +27,7 @@ pub struct IncomingCall {
 }
 
 pub struct ActiveCall {
+    location: Option<WeakModelHandle<Project>>,
     room: Option<(ModelHandle<Room>, Vec<Subscription>)>,
     incoming_call: (
         watch::Sender<Option<IncomingCall>>,
@@ -49,6 +50,7 @@ impl ActiveCall {
     ) -> Self {
         Self {
             room: None,
+            location: None,
             incoming_call: watch::channel(),
             _subscriptions: vec![
                 client.add_request_handler(cx.handle(), Self::handle_incoming_call),
@@ -132,7 +134,9 @@ impl ActiveCall {
                         Room::create(recipient_user_id, initial_project, client, user_store, cx)
                     })
                     .await?;
-                this.update(&mut cx, |this, cx| this.set_room(Some(room), cx));
+
+                this.update(&mut cx, |this, cx| this.set_room(Some(room.clone()), cx))
+                    .await?;
             };
 
             Ok(())
@@ -180,7 +184,8 @@ impl ActiveCall {
         let join = Room::join(&call, self.client.clone(), self.user_store.clone(), cx);
         cx.spawn(|this, mut cx| async move {
             let room = join.await?;
-            this.update(&mut cx, |this, cx| this.set_room(Some(room.clone()), cx));
+            this.update(&mut cx, |this, cx| this.set_room(Some(room.clone()), cx))
+                .await?;
             Ok(())
         })
     }
@@ -223,35 +228,46 @@ impl ActiveCall {
         project: Option<&ModelHandle<Project>>,
         cx: &mut ModelContext<Self>,
     ) -> Task<Result<()>> {
+        self.location = project.map(|project| project.downgrade());
         if let Some((room, _)) = self.room.as_ref() {
             room.update(cx, |room, cx| room.set_location(project, cx))
         } else {
-            Task::ready(Err(anyhow!("no active call")))
+            Task::ready(Ok(()))
         }
     }
 
-    fn set_room(&mut self, room: Option<ModelHandle<Room>>, cx: &mut ModelContext<Self>) {
+    fn set_room(
+        &mut self,
+        room: Option<ModelHandle<Room>>,
+        cx: &mut ModelContext<Self>,
+    ) -> Task<Result<()>> {
         if room.as_ref() != self.room.as_ref().map(|room| &room.0) {
+            cx.notify();
             if let Some(room) = room {
                 if room.read(cx).status().is_offline() {
                     self.room = None;
+                    Task::ready(Ok(()))
                 } else {
                     let subscriptions = vec![
                         cx.observe(&room, |this, room, cx| {
                             if room.read(cx).status().is_offline() {
-                                this.set_room(None, cx);
+                                this.set_room(None, cx).detach_and_log_err(cx);
                             }
 
                             cx.notify();
                         }),
                         cx.subscribe(&room, |_, _, event, cx| cx.emit(event.clone())),
                     ];
-                    self.room = Some((room, subscriptions));
+                    self.room = Some((room.clone(), subscriptions));
+                    let location = self.location.and_then(|location| location.upgrade(cx));
+                    room.update(cx, |room, cx| room.set_location(location.as_ref(), cx))
                 }
             } else {
                 self.room = None;
+                Task::ready(Ok(()))
             }
-            cx.notify();
+        } else {
+            Task::ready(Ok(()))
         }
     }
 

crates/call/src/room.rs

@@ -567,7 +567,7 @@ impl Room {
         })
     }
 
-    pub fn set_location(
+    pub(crate) fn set_location(
         &mut self,
         project: Option<&ModelHandle<Project>>,
         cx: &mut ModelContext<Self>,

crates/collab/src/integration_tests.rs

@@ -1074,15 +1074,9 @@ async fn test_room_location(
     client_a.fs.insert_tree("/a", json!({})).await;
     client_b.fs.insert_tree("/b", json!({})).await;
 
-    let (project_a, _) = client_a.build_local_project("/a", cx_a).await;
-    let (project_b, _) = client_b.build_local_project("/b", cx_b).await;
-
-    server
-        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
-        .await;
-
     let active_call_a = cx_a.read(ActiveCall::global);
-    let room_a = active_call_a.read_with(cx_a, |call, _| call.room().unwrap().clone());
+    let active_call_b = cx_b.read(ActiveCall::global);
+
     let a_notified = Rc::new(Cell::new(false));
     cx_a.update({
         let notified = a_notified.clone();
@@ -1092,8 +1086,6 @@ async fn test_room_location(
         }
     });
 
-    let active_call_b = cx_b.read(ActiveCall::global);
-    let room_b = active_call_b.read_with(cx_b, |call, _| call.room().unwrap().clone());
     let b_notified = Rc::new(Cell::new(false));
     cx_b.update({
         let b_notified = b_notified.clone();
@@ -1103,10 +1095,18 @@ async fn test_room_location(
         }
     });
 
-    room_a
-        .update(cx_a, |room, cx| room.set_location(Some(&project_a), cx))
+    let (project_a, _) = client_a.build_local_project("/a", cx_a).await;
+    active_call_a
+        .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
         .await
         .unwrap();
+    let (project_b, _) = client_b.build_local_project("/b", cx_b).await;
+
+    server
+        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+    let room_a = active_call_a.read_with(cx_a, |call, _| call.room().unwrap().clone());
+    let room_b = active_call_b.read_with(cx_b, |call, _| call.room().unwrap().clone());
     deterministic.run_until_parked();
     assert!(a_notified.take());
     assert_eq!(
@@ -1161,8 +1161,8 @@ async fn test_room_location(
         )]
     );
 
-    room_b
-        .update(cx_b, |room, cx| room.set_location(Some(&project_b), cx))
+    active_call_b
+        .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx))
         .await
         .unwrap();
     deterministic.run_until_parked();
@@ -1187,8 +1187,8 @@ async fn test_room_location(
         )]
     );
 
-    room_b
-        .update(cx_b, |room, cx| room.set_location(None, cx))
+    active_call_b
+        .update(cx_b, |call, cx| call.set_location(None, cx))
         .await
         .unwrap();
     deterministic.run_until_parked();
@@ -5070,6 +5070,7 @@ async fn test_following(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
         .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
         .await;
     let active_call_a = cx_a.read(ActiveCall::global);
+    let active_call_b = cx_b.read(ActiveCall::global);
 
     client_a
         .fs
@@ -5083,11 +5084,20 @@ async fn test_following(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
         )
         .await;
     let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await;
+    active_call_a
+        .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
+        .await
+        .unwrap();
+
     let project_id = active_call_a
         .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
         .await
         .unwrap();
     let project_b = client_b.build_remote_project(project_id, cx_b).await;
+    active_call_b
+        .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx))
+        .await
+        .unwrap();
 
     // Client A opens some editors.
     let workspace_a = client_a.build_workspace(&project_a, cx_a);
@@ -5281,6 +5291,7 @@ async fn test_peers_following_each_other(cx_a: &mut TestAppContext, cx_b: &mut T
         .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
         .await;
     let active_call_a = cx_a.read(ActiveCall::global);
+    let active_call_b = cx_b.read(ActiveCall::global);
 
     // Client A shares a project.
     client_a
@@ -5296,6 +5307,10 @@ async fn test_peers_following_each_other(cx_a: &mut TestAppContext, cx_b: &mut T
         )
         .await;
     let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await;
+    active_call_a
+        .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
+        .await
+        .unwrap();
     let project_id = active_call_a
         .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
         .await
@@ -5303,6 +5318,10 @@ async fn test_peers_following_each_other(cx_a: &mut TestAppContext, cx_b: &mut T
 
     // Client B joins the project.
     let project_b = client_b.build_remote_project(project_id, cx_b).await;
+    active_call_b
+        .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx))
+        .await
+        .unwrap();
 
     // Client A opens some editors.
     let workspace_a = client_a.build_workspace(&project_a, cx_a);
@@ -5450,6 +5469,7 @@ async fn test_auto_unfollowing(cx_a: &mut TestAppContext, cx_b: &mut TestAppCont
         .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
         .await;
     let active_call_a = cx_a.read(ActiveCall::global);
+    let active_call_b = cx_b.read(ActiveCall::global);
 
     // Client A shares a project.
     client_a
@@ -5464,11 +5484,20 @@ async fn test_auto_unfollowing(cx_a: &mut TestAppContext, cx_b: &mut TestAppCont
         )
         .await;
     let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await;
+    active_call_a
+        .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
+        .await
+        .unwrap();
+
     let project_id = active_call_a
         .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
         .await
         .unwrap();
     let project_b = client_b.build_remote_project(project_id, cx_b).await;
+    active_call_b
+        .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx))
+        .await
+        .unwrap();
 
     // Client A opens some editors.
     let workspace_a = client_a.build_workspace(&project_a, cx_a);

crates/collab_ui/src/collab_titlebar_item.rs

@@ -120,19 +120,15 @@ impl CollabTitlebarItem {
     }
 
     fn window_activation_changed(&mut self, active: bool, cx: &mut ViewContext<Self>) {
-        let workspace = self.workspace.upgrade(cx);
-        let room = ActiveCall::global(cx).read(cx).room().cloned();
-        if let Some((workspace, room)) = workspace.zip(room) {
-            let workspace = workspace.read(cx);
+        if let Some(workspace) = self.workspace.upgrade(cx) {
             let project = if active {
-                Some(workspace.project().clone())
+                Some(workspace.read(cx).project().clone())
             } else {
                 None
             };
-            room.update(cx, |room, cx| {
-                room.set_location(project.as_ref(), cx)
-                    .detach_and_log_err(cx);
-            });
+            ActiveCall::global(cx)
+                .update(cx, |call, cx| call.set_location(project.as_ref(), cx))
+                .detach_and_log_err(cx);
         }
     }
 

crates/collab_ui/src/contact_list.rs

@@ -1162,25 +1162,11 @@ impl ContactList {
     fn call(&mut self, action: &Call, cx: &mut ViewContext<Self>) {
         let recipient_user_id = action.recipient_user_id;
         let initial_project = action.initial_project.clone();
-        let window_id = cx.window_id();
-
-        let active_call = ActiveCall::global(cx);
-        cx.spawn_weak(|_, mut cx| async move {
-            active_call
-                .update(&mut cx, |active_call, cx| {
-                    active_call.invite(recipient_user_id, initial_project.clone(), cx)
-                })
-                .await?;
-            if cx.update(|cx| cx.window_is_active(window_id)) {
-                active_call
-                    .update(&mut cx, |call, cx| {
-                        call.set_location(initial_project.as_ref(), cx)
-                    })
-                    .await?;
-            }
-            anyhow::Ok(())
-        })
-        .detach_and_log_err(cx);
+        ActiveCall::global(cx)
+            .update(cx, |call, cx| {
+                call.invite(recipient_user_id, initial_project.clone(), cx)
+            })
+            .detach_and_log_err(cx);
     }
 
     fn leave_call(&mut self, _: &LeaveCall, cx: &mut ViewContext<Self>) {