Merge pull request #1951 from zed-industries/dock-bugfix

Fix infinite loop in dock position when deserializing

crates/gpui/src/app.rs

@@ -1431,8 +1431,8 @@ impl MutableAppContext {
         true
     }
 
-    // Returns an iterator over all of the view ids from the passed view up to the root of the window
-    // Includes the passed view itself
+    /// Returns an iterator over all of the view ids from the passed view up to the root of the window
+    /// Includes the passed view itself
     fn ancestors(&self, window_id: usize, mut view_id: usize) -> impl Iterator<Item = usize> + '_ {
         std::iter::once(view_id)
             .into_iter()
@@ -3695,6 +3695,7 @@ impl<'a, T: View> ViewContext<'a, T> {
             return false;
         }
         self.ancestors(view.window_id, view.view_id)
+            .skip(1) // Skip self id
             .any(|parent| parent == self.view_id)
     }
 

crates/workspace/src/dock.rs

@@ -458,14 +458,26 @@ impl StatusItemView for ToggleDockButton {
 
 #[cfg(test)]
 mod tests {
-    use std::ops::{Deref, DerefMut};
+    use std::{
+        ops::{Deref, DerefMut},
+        path::PathBuf,
+    };
 
     use gpui::{AppContext, TestAppContext, UpdateView, ViewContext};
     use project::{FakeFs, Project};
     use settings::Settings;
 
     use super::*;
-    use crate::{item::test::TestItem, sidebar::Sidebar, ItemHandle, Workspace};
+    use crate::{
+        dock,
+        item::test::TestItem,
+        persistence::model::{
+            SerializedItem, SerializedPane, SerializedPaneGroup, SerializedWorkspace,
+        },
+        register_deserializable_item,
+        sidebar::Sidebar,
+        ItemHandle, Workspace,
+    };
 
     pub fn default_item_factory(
         _workspace: &mut Workspace,
@@ -474,6 +486,51 @@ mod tests {
         Some(Box::new(cx.add_view(|_| TestItem::new())))
     }
 
+    #[gpui::test]
+    async fn test_dock_workspace_infinite_loop(cx: &mut TestAppContext) {
+        cx.foreground().forbid_parking();
+        Settings::test_async(cx);
+
+        cx.update(|cx| {
+            register_deserializable_item::<TestItem>(cx);
+        });
+
+        let serialized_workspace = SerializedWorkspace {
+            id: 0,
+            location: Vec::<PathBuf>::new().into(),
+            dock_position: dock::DockPosition::Shown(DockAnchor::Expanded),
+            center_group: SerializedPaneGroup::Pane(SerializedPane {
+                active: false,
+                children: vec![],
+            }),
+            dock_pane: SerializedPane {
+                active: true,
+                children: vec![SerializedItem {
+                    active: true,
+                    item_id: 0,
+                    kind: "test".into(),
+                }],
+            },
+            left_sidebar_open: false,
+        };
+
+        let fs = FakeFs::new(cx.background());
+        let project = Project::test(fs, [], cx).await;
+
+        let (_, _workspace) = cx.add_window(|cx| {
+            Workspace::new(
+                Some(serialized_workspace),
+                0,
+                project.clone(),
+                default_item_factory,
+                cx,
+            )
+        });
+
+        cx.foreground().run_until_parked();
+        //Should terminate
+    }
+
     #[gpui::test]
     async fn test_dock_hides_when_pane_empty(cx: &mut TestAppContext) {
         let mut cx = DockTestContext::new(cx).await;

crates/workspace/src/item.rs

@@ -681,6 +681,7 @@ pub(crate) mod test {
     use super::{Item, ItemEvent};
 
     pub struct TestItem {
+        pub workspace_id: WorkspaceId,
         pub state: String,
         pub label: String,
         pub save_count: usize,
@@ -716,6 +717,7 @@ pub(crate) mod test {
                 nav_history: None,
                 tab_descriptions: None,
                 tab_detail: Default::default(),
+                workspace_id: self.workspace_id,
             }
         }
     }
@@ -736,9 +738,16 @@ pub(crate) mod test {
                 nav_history: None,
                 tab_descriptions: None,
                 tab_detail: Default::default(),
+                workspace_id: 0,
             }
         }
 
+        pub fn new_deserialized(id: WorkspaceId) -> Self {
+            let mut this = Self::new();
+            this.workspace_id = id;
+            this
+        }
+
         pub fn with_label(mut self, state: &str) -> Self {
             self.label = state.to_string();
             self
@@ -893,11 +902,12 @@ pub(crate) mod test {
         fn deserialize(
             _project: ModelHandle<Project>,
             _workspace: WeakViewHandle<Workspace>,
-            _workspace_id: WorkspaceId,
+            workspace_id: WorkspaceId,
             _item_id: ItemId,
-            _cx: &mut ViewContext<Pane>,
+            cx: &mut ViewContext<Pane>,
         ) -> Task<anyhow::Result<ViewHandle<Self>>> {
-            unreachable!("Cannot deserialize test item")
+            let view = cx.add_view(|_cx| Self::new_deserialized(workspace_id));
+            Task::Ready(Some(anyhow::Ok(view)))
         }
     }
 

crates/workspace/src/workspace.rs

@@ -2371,7 +2371,12 @@ impl Workspace {
                         workspace.toggle_sidebar(SidebarSide::Left, cx);
                     }
 
-                    // Dock::set_dock_position(workspace, serialized_workspace.dock_position, cx);
+                    // Note that without after_window, the focus_self() and
+                    // the focus the dock generates start generating alternating
+                    // focus due to the deferred execution each triggering each other
+                    cx.after_window_update(move |workspace, cx| {
+                        Dock::set_dock_position(workspace, serialized_workspace.dock_position, cx);
+                    });
 
                     cx.notify();
                 });
@@ -2537,7 +2542,7 @@ impl View for Workspace {
         } else {
             for pane in self.panes() {
                 let view = view.clone();
-                if pane.update(cx, |_, cx| cx.is_child(view)) {
+                if pane.update(cx, |_, cx| view.id() == cx.view_id() || cx.is_child(view)) {
                     self.handle_pane_focused(pane.clone(), cx);
                     break;
                 }
@@ -2695,7 +2700,7 @@ mod tests {
         _workspace: &mut Workspace,
         _cx: &mut ViewContext<Workspace>,
     ) -> Option<Box<dyn ItemHandle>> {
-        unimplemented!();
+        unimplemented!()
     }
 
     #[gpui::test]