Fix flickering when interacting with the language server logs (cherry-pick #9390) (#9435)

gcp-cherry-pick-bot[bot] and Antonio Scandurra created 2 years ago

Cherry-picked Fix flickering when interacting with the language server
logs (#9390)

Fixes https://github.com/zed-industries/zed/issues/9340

The flickering was caused by the pane trying to restore focus on a
`FocusHandle` that wasn't being rendered anymore. This commit uses the
new `WeakFocusHandle` to avoid retaining a reference to focus handles
that don't exist anymore.

Release Notes:

- Fixed a bug that caused flickering when interacting with the language
server logs
([#9340](https://github.com/zed-industries/zed/issues/9340)).

Co-authored-by: Antonio Scandurra <[e-mail]>

Change summary

crates/gpui/src/window.rs    | 45 +++++++++++++++++++++++++++++++++++++
crates/workspace/src/pane.rs | 20 +++++++++-------
2 files changed, 55 insertions(+), 10 deletions(-)

Detailed changes

crates/gpui/src/window.rs

@@ -29,7 +29,7 @@ use std::{
     rc::Rc,
     sync::{
         atomic::{AtomicUsize, Ordering::SeqCst},
-        Arc,
+        Arc, Weak,
     },
     time::{Duration, Instant},
 };
@@ -155,6 +155,14 @@ impl FocusHandle {
         }
     }
 
+    /// Converts this focus handle into a weak variant, which does not prevent it from being released.
+    pub fn downgrade(&self) -> WeakFocusHandle {
+        WeakFocusHandle {
+            id: self.id,
+            handles: Arc::downgrade(&self.handles),
+        }
+    }
+
     /// Moves the focus to the element associated with this handle.
     pub fn focus(&self, cx: &mut WindowContext) {
         cx.focus(self)
@@ -207,6 +215,41 @@ impl Drop for FocusHandle {
     }
 }
 
+/// A weak reference to a focus handle.
+#[derive(Clone, Debug)]
+pub struct WeakFocusHandle {
+    pub(crate) id: FocusId,
+    handles: Weak<RwLock<SlotMap<FocusId, AtomicUsize>>>,
+}
+
+impl WeakFocusHandle {
+    /// Attempts to upgrade the [WeakFocusHandle] to a [FocusHandle].
+    pub fn upgrade(&self) -> Option<FocusHandle> {
+        let handles = self.handles.upgrade()?;
+        FocusHandle::for_id(self.id, &handles)
+    }
+}
+
+impl PartialEq for WeakFocusHandle {
+    fn eq(&self, other: &WeakFocusHandle) -> bool {
+        self.id == other.id
+    }
+}
+
+impl Eq for WeakFocusHandle {}
+
+impl PartialEq<FocusHandle> for WeakFocusHandle {
+    fn eq(&self, other: &FocusHandle) -> bool {
+        self.id == other.id
+    }
+}
+
+impl PartialEq<WeakFocusHandle> for FocusHandle {
+    fn eq(&self, other: &WeakFocusHandle) -> bool {
+        self.id == other.id
+    }
+}
+
 /// FocusableView allows users of your view to easily
 /// focus it (using cx.focus_view(view))
 pub trait FocusableView: 'static + Render {

crates/workspace/src/pane.rs

@@ -12,7 +12,7 @@ use gpui::{
     AsyncWindowContext, ClickEvent, DismissEvent, Div, DragMoveEvent, EntityId, EventEmitter,
     ExternalPaths, FocusHandle, FocusableView, Model, MouseButton, NavigationDirection, Pixels,
     Point, PromptLevel, Render, ScrollHandle, Subscription, Task, View, ViewContext, VisualContext,
-    WeakView, WindowContext,
+    WeakFocusHandle, WeakView, WindowContext,
 };
 use parking_lot::Mutex;
 use project::{Project, ProjectEntryId, ProjectPath};
@@ -166,7 +166,7 @@ pub struct Pane {
     zoomed: bool,
     was_focused: bool,
     active_item_index: usize,
-    last_focused_view_by_item: HashMap<EntityId, FocusHandle>,
+    last_focus_handle_by_item: HashMap<EntityId, WeakFocusHandle>,
     nav_history: NavHistory,
     toolbar: View<Toolbar>,
     new_item_menu: Option<View<ContextMenu>>,
@@ -262,7 +262,7 @@ impl Pane {
             was_focused: false,
             zoomed: false,
             active_item_index: 0,
-            last_focused_view_by_item: Default::default(),
+            last_focus_handle_by_item: Default::default(),
             nav_history: NavHistory(Arc::new(Mutex::new(NavHistoryState {
                 mode: NavigationMode::Normal,
                 backward_stack: Default::default(),
@@ -380,18 +380,20 @@ impl Pane {
             if self.focus_handle.is_focused(cx) {
                 // Pane was focused directly. We need to either focus a view inside the active item,
                 // or focus the active item itself
-                if let Some(weak_last_focused_view) =
-                    self.last_focused_view_by_item.get(&active_item.item_id())
+                if let Some(weak_last_focus_handle) =
+                    self.last_focus_handle_by_item.get(&active_item.item_id())
                 {
-                    weak_last_focused_view.focus(cx);
-                    return;
+                    if let Some(focus_handle) = weak_last_focus_handle.upgrade() {
+                        focus_handle.focus(cx);
+                        return;
+                    }
                 }
 
                 active_item.focus_handle(cx).focus(cx);
             } else if let Some(focused) = cx.focused() {
                 if !self.context_menu_focused(cx) {
-                    self.last_focused_view_by_item
-                        .insert(active_item.item_id(), focused);
+                    self.last_focus_handle_by_item
+                        .insert(active_item.item_id(), focused.downgrade());
                 }
             }
         }