Cache view's type id and keymap context into a separate map

Antonio Scandurra created 2 years ago

During `layout`, we now pass a mutable reference to the element's
parent view. This is a recursive process that causes the view to
be removed from `AppContext` and then re-inserted back into it once
the layout is complete.

As such, querying parent views during `layout` does not work as such
views will have been removed from `AppContext` and not yet re-inserted
into it. This caused a bug in `KeystrokeLabel`, which used the `keystrokes_for_action`
method to query its ancestors to determine their type id and keymap context.

Now, any time a view notifies, we will cache its keymap context so that
we don't need to query the parent view during `layout`.

Change summary

crates/gpui/src/app.rs        | 39 ++++++++++++++++++++++++------------
crates/gpui/src/app/window.rs | 16 +++++++++++---
2 files changed, 38 insertions(+), 17 deletions(-)

Detailed changes

crates/gpui/src/app.rs

@@ -440,6 +440,7 @@ type WindowShouldCloseSubscriptionCallback = Box<dyn FnMut(&mut AppContext) -> b
 pub struct AppContext {
     models: HashMap<usize, Box<dyn AnyModel>>,
     views: HashMap<(usize, usize), Box<dyn AnyView>>,
+    views_metadata: HashMap<(usize, usize), ViewMetadata>,
     pub(crate) parents: HashMap<(usize, usize), ParentId>,
     windows: HashMap<usize, Window>,
     globals: HashMap<TypeId, Box<dyn Any>>,
@@ -502,6 +503,7 @@ impl AppContext {
         Self {
             models: Default::default(),
             views: Default::default(),
+            views_metadata: Default::default(),
             parents: Default::default(),
             windows: Default::default(),
             globals: Default::default(),
@@ -727,9 +729,9 @@ impl AppContext {
     }
 
     pub fn view_type_id(&self, window_id: usize, view_id: usize) -> Option<TypeId> {
-        self.views
+        self.views_metadata
             .get(&(window_id, view_id))
-            .map(|view| view.as_any().type_id())
+            .map(|metadata| metadata.type_id)
     }
 
     pub fn active_labeled_tasks<'a>(
@@ -1045,9 +1047,10 @@ impl AppContext {
                 .read_window(window_id, |cx| {
                     if let Some(focused_view_id) = cx.focused_view_id() {
                         for view_id in cx.ancestors(focused_view_id) {
-                            if let Some(view) = cx.views.get(&(window_id, view_id)) {
-                                let view_type = view.as_any().type_id();
-                                if let Some(actions) = cx.actions.get(&view_type) {
+                            if let Some(view_metadata) =
+                                cx.views_metadata.get(&(window_id, view_id))
+                            {
+                                if let Some(actions) = cx.actions.get(&view_metadata.type_id) {
                                     if actions.contains_key(&action_type) {
                                         return true;
                                     }
@@ -1448,6 +1451,7 @@ impl AppContext {
             for (window_id, view_id) in dropped_views {
                 self.subscriptions.remove(view_id);
                 self.observations.remove(view_id);
+                self.views_metadata.remove(&(window_id, view_id));
                 let mut view = self.views.remove(&(window_id, view_id)).unwrap();
                 view.release(self);
                 let change_focus_to = self.windows.get_mut(&window_id).and_then(|window| {
@@ -1779,9 +1783,11 @@ impl AppContext {
         observed_window_id: usize,
         observed_view_id: usize,
     ) {
-        if self
+        let view_key = (observed_window_id, observed_view_id);
+        if let Some((view, mut view_metadata)) = self
             .views
-            .contains_key(&(observed_window_id, observed_view_id))
+            .remove(&view_key)
+            .zip(self.views_metadata.remove(&view_key))
         {
             if let Some(window) = self.windows.get_mut(&observed_window_id) {
                 window
@@ -1791,6 +1797,10 @@ impl AppContext {
                     .insert(observed_view_id);
             }
 
+            view_metadata.keymap_context = view.keymap_context(self);
+            self.views.insert(view_key, view);
+            self.views_metadata.insert(view_key, view_metadata);
+
             let mut observations = self.observations.clone();
             observations.emit(observed_view_id, |callback| callback(self));
         }
@@ -2037,6 +2047,11 @@ pub enum ParentId {
     Root,
 }
 
+struct ViewMetadata {
+    type_id: TypeId,
+    keymap_context: KeymapContext,
+}
+
 #[derive(Default, Clone)]
 pub struct WindowInvalidation {
     pub updated: HashSet<usize>,
@@ -2437,11 +2452,10 @@ where
             cx.handle().into_any()
         } else {
             let focused_type = cx
-                .views
+                .views_metadata
                 .get(&(cx.window_id, focused_id))
                 .unwrap()
-                .as_any()
-                .type_id();
+                .type_id;
             AnyViewHandle::new(
                 cx.window_id,
                 focused_id,
@@ -2458,11 +2472,10 @@ where
             cx.handle().into_any()
         } else {
             let blurred_type = cx
-                .views
+                .views_metadata
                 .get(&(cx.window_id, blurred_id))
                 .unwrap()
-                .as_any()
-                .type_id();
+                .type_id;
             AnyViewHandle::new(
                 cx.window_id,
                 blurred_id,

crates/gpui/src/app/window.rs

@@ -34,7 +34,7 @@ use std::{
 use util::ResultExt;
 use uuid::Uuid;
 
-use super::Reference;
+use super::{Reference, ViewMetadata};
 
 pub struct Window {
     pub(crate) root_view: Option<AnyViewHandle>,
@@ -369,13 +369,13 @@ impl<'a> WindowContext<'a> {
         let mut contexts = Vec::new();
         let mut handler_depth = None;
         for (i, view_id) in self.ancestors(view_id).enumerate() {
-            if let Some(view) = self.views.get(&(window_id, view_id)) {
-                if let Some(actions) = self.actions.get(&view.as_any().type_id()) {
+            if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) {
+                if let Some(actions) = self.actions.get(&view_metadata.type_id) {
                     if actions.contains_key(&action.as_any().type_id()) {
                         handler_depth = Some(i);
                     }
                 }
-                contexts.push(view.keymap_context(self));
+                contexts.push(view_metadata.keymap_context.clone());
             }
         }
 
@@ -1177,6 +1177,14 @@ impl<'a> WindowContext<'a> {
         self.parents.insert((window_id, view_id), parent_id);
         let mut cx = ViewContext::mutable(self, view_id);
         let handle = if let Some(view) = build_view(&mut cx) {
+            let keymap_context = view.keymap_context(cx.app_context());
+            self.views_metadata.insert(
+                (window_id, view_id),
+                ViewMetadata {
+                    type_id: TypeId::of::<T>(),
+                    keymap_context,
+                },
+            );
             self.views.insert((window_id, view_id), Box::new(view));
             self.window
                 .invalidation