From 06dfb74663f4c40732aacda8865177063ce230bf Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 14:53:58 +0200 Subject: [PATCH 1/5] Prevent `ChildView` from retaining an otherwise dropped view --- crates/gpui/src/app.rs | 4 ++++ crates/gpui/src/presenter.rs | 20 +++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 3b192000155f81afb1ae0fea90f83d94a6445e5b..f7672db9ae8bcfdca4c19586c2c509928ad41378 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -4732,6 +4732,10 @@ pub struct AnyWeakViewHandle { } impl AnyWeakViewHandle { + pub fn id(&self) -> usize { + self.view_id + } + pub fn upgrade(&self, cx: &impl UpgradeViewHandle) -> Option { cx.upgrade_any_view_handle(self) } diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index eaccd8f4109a046cd0f8e01e05d444effb486820..e4aae8fbd93adb5719b4eef795415adc196708df 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -12,10 +12,10 @@ use crate::{ UpOutRegionEvent, UpRegionEvent, }, text_layout::TextLayoutCache, - Action, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, Appearance, AssetCache, ElementBox, - Entity, FontSystem, ModelHandle, MouseButton, MouseMovedEvent, MouseRegion, MouseRegionId, - ParentId, ReadModel, ReadView, RenderContext, RenderParams, Scene, UpgradeModelHandle, - UpgradeViewHandle, View, ViewHandle, WeakModelHandle, WeakViewHandle, + Action, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, AnyWeakViewHandle, Appearance, + AssetCache, ElementBox, Entity, FontSystem, ModelHandle, MouseButton, MouseMovedEvent, + MouseRegion, MouseRegionId, ParentId, ReadModel, ReadView, RenderContext, RenderParams, Scene, + UpgradeModelHandle, UpgradeViewHandle, View, ViewHandle, WeakModelHandle, WeakViewHandle, }; use collections::{HashMap, HashSet}; use pathfinder_geometry::vector::{vec2f, Vector2F}; @@ -972,12 +972,14 @@ impl ToJson for SizeConstraint { } pub struct ChildView { - view: AnyViewHandle, + view: AnyWeakViewHandle, } impl ChildView { pub fn new(view: impl Into) -> Self { - Self { view: view.into() } + Self { + view: view.into().downgrade(), + } } } @@ -1039,7 +1041,11 @@ impl Element for ChildView { "type": "ChildView", "view_id": self.view.id(), "bounds": bounds.to_json(), - "view": self.view.debug_json(cx.app), + "view": if let Some(view) = self.view.upgrade(cx.app) { + view.debug_json(cx.app) + } else { + json!(null) + }, "child": if let Some(view) = cx.rendered_views.get(&self.view.id()) { view.debug(cx) } else { From edb61a9c8f924eda870865770b3cdf68f53f5f6c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 15:11:57 +0200 Subject: [PATCH 2/5] Avoid panicking if child view points to a view that was not rendered --- crates/gpui/src/presenter.rs | 38 ++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index e4aae8fbd93adb5719b4eef795415adc196708df..8cf9e99063e0d15f49ce3566fcd89f09f2cad17e 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -984,7 +984,7 @@ impl ChildView { } impl Element for ChildView { - type LayoutState = (); + type LayoutState = bool; type PaintState = (); fn layout( @@ -992,18 +992,27 @@ impl Element for ChildView { constraint: SizeConstraint, cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { - let size = cx.layout(self.view.id(), constraint); - (size, ()) + if cx.rendered_views.contains_key(&self.view.id()) { + let size = cx.layout(self.view.id(), constraint); + (size, true) + } else { + log::error!("layout called on a view that was not rendered"); + (Vector2F::zero(), false) + } } fn paint( &mut self, bounds: RectF, visible_bounds: RectF, - _: &mut Self::LayoutState, + view_is_valid: &mut Self::LayoutState, cx: &mut PaintContext, - ) -> Self::PaintState { - cx.paint(self.view.id(), bounds.origin(), visible_bounds); + ) { + if *view_is_valid { + cx.paint(self.view.id(), bounds.origin(), visible_bounds); + } else { + log::error!("paint called on a view that was not rendered"); + } } fn dispatch_event( @@ -1011,11 +1020,16 @@ impl Element for ChildView { event: &Event, _: RectF, _: RectF, - _: &mut Self::LayoutState, + view_is_valid: &mut Self::LayoutState, _: &mut Self::PaintState, cx: &mut EventContext, ) -> bool { - cx.dispatch_event(self.view.id(), event) + if *view_is_valid { + cx.dispatch_event(self.view.id(), event) + } else { + log::error!("dispatch_event called on a view that was not rendered"); + false + } } fn rect_for_text_range( @@ -1023,11 +1037,15 @@ impl Element for ChildView { range_utf16: Range, _: RectF, _: RectF, - _: &Self::LayoutState, + view_is_valid: &Self::LayoutState, _: &Self::PaintState, cx: &MeasurementContext, ) -> Option { - cx.rect_for_text_range(self.view.id(), range_utf16) + if *view_is_valid { + cx.rect_for_text_range(self.view.id(), range_utf16) + } else { + None + } } fn debug( From a5a60eb854e53a43150a5ec058e52bcf640a2570 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 15:40:21 +0200 Subject: [PATCH 3/5] Log view name alongside error in `ChildView` --- crates/chat_panel/src/chat_panel.rs | 4 +-- crates/collab/src/integration_tests.rs | 2 +- crates/collab_ui/src/collab_titlebar_item.rs | 2 +- crates/collab_ui/src/contact_finder.rs | 4 +-- crates/collab_ui/src/contact_list.rs | 2 +- crates/collab_ui/src/contacts_popover.rs | 4 +-- crates/command_palette/src/command_palette.rs | 8 ++--- crates/diagnostics/src/diagnostics.rs | 2 +- crates/editor/src/editor.rs | 4 +-- crates/file_finder/src/file_finder.rs | 4 +-- crates/go_to_line/src/go_to_line.rs | 2 +- crates/gpui/src/app.rs | 8 +++++ crates/gpui/src/presenter.rs | 31 ++++++++++++++++--- crates/outline/src/outline.rs | 4 +-- crates/picker/src/picker.rs | 2 +- crates/project_panel/src/project_panel.rs | 6 ++-- crates/project_symbols/src/project_symbols.rs | 4 +-- crates/search/src/buffer_search.rs | 2 +- crates/search/src/project_search.rs | 6 ++-- .../terminal/src/terminal_container_view.rs | 4 +-- crates/terminal/src/terminal_view.rs | 2 +- crates/theme_selector/src/theme_selector.rs | 4 +-- crates/workspace/src/dock.rs | 6 ++-- crates/workspace/src/pane.rs | 6 ++-- crates/workspace/src/pane_group.rs | 7 ++++- crates/workspace/src/sidebar.rs | 2 +- crates/workspace/src/status_bar.rs | 4 +-- crates/workspace/src/toolbar.rs | 6 ++-- crates/workspace/src/workspace.rs | 26 ++++++++++------ 29 files changed, 105 insertions(+), 63 deletions(-) diff --git a/crates/chat_panel/src/chat_panel.rs b/crates/chat_panel/src/chat_panel.rs index 6744ae9339ec027623673901218bd53d90d860d6..60eda235ac9fa5af5ca9f7e3bc4d404579c68f2f 100644 --- a/crates/chat_panel/src/chat_panel.rs +++ b/crates/chat_panel/src/chat_panel.rs @@ -200,7 +200,7 @@ impl ChatPanel { let theme = &cx.global::().theme; Flex::column() .with_child( - Container::new(ChildView::new(&self.channel_select).boxed()) + Container::new(ChildView::new(&self.channel_select, cx).boxed()) .with_style(theme.chat_panel.channel_select.container) .boxed(), ) @@ -265,7 +265,7 @@ impl ChatPanel { fn render_input_box(&self, cx: &AppContext) -> ElementBox { let theme = &cx.global::().theme; - Container::new(ChildView::new(&self.input_editor).boxed()) + Container::new(ChildView::new(&self.input_editor, cx).boxed()) .with_style(theme.chat_panel.input_editor.container) .boxed() } diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 8ab852810b5c513de6b5b7f66ff78fbcdb44f033..9dddb74977088c1f3c60b2a19ec4d9a99a60e01a 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -25,7 +25,7 @@ use gpui::{ }; use language::{ range_to_lsp, tree_sitter_rust, Diagnostic, DiagnosticEntry, FakeLspAdapter, Language, - LanguageConfig, LanguageRegistry, OffsetRangeExt, Rope, Point, + LanguageConfig, LanguageRegistry, OffsetRangeExt, Point, Rope, }; use lsp::{self, FakeLanguageServer}; use parking_lot::Mutex; diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index c78a50b86d391f6bbaa05bdce59929b3a353edb3..702d8a9121e27b8ddaf6da7774807eed86b1b202 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -223,7 +223,7 @@ impl CollabTitlebarItem { .with_children(badge) .with_children(self.contacts_popover.as_ref().map(|popover| { Overlay::new( - ChildView::new(popover) + ChildView::new(popover, cx) .contained() .with_margin_top(titlebar.height) .with_margin_left(titlebar.toggle_contacts_button.default.button_width) diff --git a/crates/collab_ui/src/contact_finder.rs b/crates/collab_ui/src/contact_finder.rs index 25726e381e6242496b83df734d75ef1b19b1b243..8835dc0b0e7811ecf86fbf9f5457c613827a9f1c 100644 --- a/crates/collab_ui/src/contact_finder.rs +++ b/crates/collab_ui/src/contact_finder.rs @@ -32,8 +32,8 @@ impl View for ContactFinder { "ContactFinder" } - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index c04f0fe72dbdf52ba509409b49fc09798a02fdca..e1d96cd1da78be7d739d83de1d7495db5d7400a2 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -1072,7 +1072,7 @@ impl View for ContactList { .with_child( Flex::row() .with_child( - ChildView::new(self.filter_editor.clone()) + ChildView::new(self.filter_editor.clone(), cx) .contained() .with_style(theme.contact_list.user_query_editor.container) .flex(1., true) diff --git a/crates/collab_ui/src/contacts_popover.rs b/crates/collab_ui/src/contacts_popover.rs index 73bcad9fd7e55dd6a9782ac6736ee0e1ecd8d4cd..075255d72750bee57f8372b39570013b15d3a8a2 100644 --- a/crates/collab_ui/src/contacts_popover.rs +++ b/crates/collab_ui/src/contacts_popover.rs @@ -88,8 +88,8 @@ impl View for ContactsPopover { fn render(&mut self, cx: &mut RenderContext) -> ElementBox { let theme = cx.global::().theme.clone(); let child = match &self.child { - Child::ContactList(child) => ChildView::new(child), - Child::ContactFinder(child) => ChildView::new(child), + Child::ContactList(child) => ChildView::new(child, cx), + Child::ContactFinder(child) => ChildView::new(child, cx), }; MouseEventHandler::::new(0, cx, |_, cx| { diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index c12e68a85417099e81f6f44af168e442db516ac5..125a4eeb021679e7e58bebcaad3cf566b38491ec 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -4,8 +4,8 @@ use gpui::{ actions, elements::{ChildView, Flex, Label, ParentElement}, keymap::Keystroke, - Action, AnyViewHandle, Element, Entity, MouseState, MutableAppContext, View, ViewContext, - ViewHandle, + Action, AnyViewHandle, Element, Entity, MouseState, MutableAppContext, RenderContext, View, + ViewContext, ViewHandle, }; use picker::{Picker, PickerDelegate}; use settings::Settings; @@ -131,8 +131,8 @@ impl View for CommandPalette { "CommandPalette" } - fn render(&mut self, _: &mut gpui::RenderContext<'_, Self>) -> gpui::ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> gpui::ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 3111d7a9f17ea47b759a6749a317d1860f76516c..8180a6c9f61169502ac9a7114cee422da68d6329 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -95,7 +95,7 @@ impl View for ProjectDiagnosticsEditor { .with_style(theme.container) .boxed() } else { - ChildView::new(&self.editor).boxed() + ChildView::new(&self.editor, cx).boxed() } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a7acc9f6095254dd66a55593ebe0333f8ce9e654..2f7431a6c9d9c2138301f1f4e576c73d6b5479a3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5206,7 +5206,7 @@ impl Editor { render: Arc::new({ let editor = rename_editor.clone(); move |cx: &mut BlockContext| { - ChildView::new(editor.clone()) + ChildView::new(editor.clone(), cx) .contained() .with_padding_left(cx.anchor_x) .boxed() @@ -6270,7 +6270,7 @@ impl View for Editor { .with_child( EditorElement::new(self.handle.clone(), style.clone(), self.cursor_shape).boxed(), ) - .with_child(ChildView::new(&self.mouse_context_menu).boxed()) + .with_child(ChildView::new(&self.mouse_context_menu, cx).boxed()) .boxed() } diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index aa2174b9598a8cb94e0917c58cde07a537852cf6..fe0fa3ae7790cd60aa14d1864c9649046c3bccd0 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -49,8 +49,8 @@ impl View for FileFinder { "FileFinder" } - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index 3ca50cee42584aaa0d73dc853c51d0cfaec6badd..eddb014b63a4715e6268070ccc9f96897f0e50d5 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/crates/go_to_line/src/go_to_line.rs @@ -165,7 +165,7 @@ impl View for GoToLine { Container::new( Flex::new(Axis::Vertical) .with_child( - Container::new(ChildView::new(&self.line_editor).boxed()) + Container::new(ChildView::new(&self.line_editor, cx).boxed()) .with_style(theme.input_editor.container) .boxed(), ) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f7672db9ae8bcfdca4c19586c2c509928ad41378..83936c617e5801b395aa5aef864d578ce516083c 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2571,6 +2571,10 @@ impl AppContext { .and_then(|window| window.focused_view_id) } + pub fn view_ui_name(&self, window_id: usize, view_id: usize) -> Option<&'static str> { + Some(self.views.get(&(window_id, view_id))?.ui_name()) + } + pub fn background(&self) -> &Arc { &self.background } @@ -4416,6 +4420,10 @@ impl AnyViewHandle { } } + pub fn window_id(&self) -> usize { + self.window_id + } + pub fn id(&self) -> usize { self.view_id } diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index 8cf9e99063e0d15f49ce3566fcd89f09f2cad17e..1d59905d7cc8a542505d0fcb3c19a94cca963e0b 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -973,12 +973,16 @@ impl ToJson for SizeConstraint { pub struct ChildView { view: AnyWeakViewHandle, + view_name: &'static str, } impl ChildView { - pub fn new(view: impl Into) -> Self { + pub fn new(view: impl Into, cx: &AppContext) -> Self { + let view = view.into(); + let view_name = cx.view_ui_name(view.window_id(), view.id()).unwrap(); Self { - view: view.into().downgrade(), + view: view.downgrade(), + view_name, } } } @@ -996,7 +1000,11 @@ impl Element for ChildView { let size = cx.layout(self.view.id(), constraint); (size, true) } else { - log::error!("layout called on a view that was not rendered"); + log::error!( + "layout called on a view (id: {}, name: {:?}) that was not rendered", + self.view.id(), + self.view_name + ); (Vector2F::zero(), false) } } @@ -1011,7 +1019,11 @@ impl Element for ChildView { if *view_is_valid { cx.paint(self.view.id(), bounds.origin(), visible_bounds); } else { - log::error!("paint called on a view that was not rendered"); + log::error!( + "paint called on a view (id: {}, name: {:?}) that was not rendered", + self.view.id(), + self.view_name + ); } } @@ -1027,7 +1039,11 @@ impl Element for ChildView { if *view_is_valid { cx.dispatch_event(self.view.id(), event) } else { - log::error!("dispatch_event called on a view that was not rendered"); + log::error!( + "dispatch_event called on a view (id: {}, name: {:?}) that was not rendered", + self.view.id(), + self.view_name + ); false } } @@ -1044,6 +1060,11 @@ impl Element for ChildView { if *view_is_valid { cx.rect_for_text_range(self.view.id(), range_utf16) } else { + log::error!( + "rect_for_text_range called on a view (id: {}, name: {:?}) that was not rendered", + self.view.id(), + self.view_name + ); None } } diff --git a/crates/outline/src/outline.rs b/crates/outline/src/outline.rs index f814276306515d7aab0eb2edbff31e49782597ee..afcfa263a86c7ea7fdc8311c36c64ee20f3e5b6d 100644 --- a/crates/outline/src/outline.rs +++ b/crates/outline/src/outline.rs @@ -48,8 +48,8 @@ impl View for OutlineView { "OutlineView" } - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 622dc13309a9b41171d8c45053fe8a835d9270d5..a093740e25a3d0a95ada8ac6694716019bc8f6b0 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -63,7 +63,7 @@ impl View for Picker { Flex::new(Axis::Vertical) .with_child( - ChildView::new(&self.query_editor) + ChildView::new(&self.query_editor, cx) .contained() .with_style(theme.input_editor.container) .boxed(), diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 8ba70ee4ebe5e6a38fa6b782e3c16d06282acecf..abd8f44603e67d325515d101ceedf2813ce87d47 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1012,7 +1012,7 @@ impl ProjectPanel { ) -> ElementBox { let kind = details.kind; let show_editor = details.is_editing && !details.is_processing; - MouseEventHandler::::new(entry_id.to_usize(), cx, |state, _| { + MouseEventHandler::::new(entry_id.to_usize(), cx, |state, cx| { let padding = theme.container.padding.left + details.depth as f32 * theme.indent_width; let mut style = theme.entry.style_for(state, details.is_selected).clone(); if details.is_ignored { @@ -1051,7 +1051,7 @@ impl ProjectPanel { .boxed(), ) .with_child(if show_editor { - ChildView::new(editor.clone()) + ChildView::new(editor.clone(), cx) .contained() .with_margin_left(theme.entry.default.icon_spacing) .aligned() @@ -1147,7 +1147,7 @@ impl View for ProjectPanel { }) .boxed(), ) - .with_child(ChildView::new(&self.context_menu).boxed()) + .with_child(ChildView::new(&self.context_menu, cx).boxed()) .boxed() } diff --git a/crates/project_symbols/src/project_symbols.rs b/crates/project_symbols/src/project_symbols.rs index c310cfb0437b6e433cc6c8fac7b9660de4dc0ecf..6aa866342a420657e938ff1e6b6c833c7229831c 100644 --- a/crates/project_symbols/src/project_symbols.rs +++ b/crates/project_symbols/src/project_symbols.rs @@ -47,8 +47,8 @@ impl View for ProjectSymbolsView { "ProjectSymbolsView" } - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 6f75888f480cca27c6ffa8ae41f46eb285b5ad4c..a43f3eb4867c7c9b1e27f1db278009cce26a6fcf 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -105,7 +105,7 @@ impl View for BufferSearchBar { .with_child( Flex::row() .with_child( - ChildView::new(&self.query_editor) + ChildView::new(&self.query_editor, cx) .aligned() .left() .flex(1., true) diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index d3c0743325c6dc8c1ea8fef8f92ec707986960d2..eb5bf7d699480e9a240f93718f169f1979d0549f 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -189,7 +189,9 @@ impl View for ProjectSearchView { }) .boxed() } else { - ChildView::new(&self.results_editor).flex(1., true).boxed() + ChildView::new(&self.results_editor, cx) + .flex(1., true) + .boxed() } } @@ -824,7 +826,7 @@ impl View for ProjectSearchBar { .with_child( Flex::row() .with_child( - ChildView::new(&search.query_editor) + ChildView::new(&search.query_editor, cx) .aligned() .left() .flex(1., true) diff --git a/crates/terminal/src/terminal_container_view.rs b/crates/terminal/src/terminal_container_view.rs index e0fe6ef6cbf275d768970da9d926b1adda7b509f..5cad16774d9f54ab2f173fb44678527bffaf55a5 100644 --- a/crates/terminal/src/terminal_container_view.rs +++ b/crates/terminal/src/terminal_container_view.rs @@ -162,8 +162,8 @@ impl View for TerminalContainer { fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> ElementBox { let child_view = match &self.content { - TerminalContainerContent::Connected(connected) => ChildView::new(connected), - TerminalContainerContent::Error(error) => ChildView::new(error), + TerminalContainerContent::Connected(connected) => ChildView::new(connected, cx), + TerminalContainerContent::Error(error) => ChildView::new(error, cx), }; if self.modal { let settings = cx.global::(); diff --git a/crates/terminal/src/terminal_view.rs b/crates/terminal/src/terminal_view.rs index 274207604507d36029b799bad08d19dbbd57cc29..675766b06601e7ab8670aac2f64b1d8fa10185fd 100644 --- a/crates/terminal/src/terminal_view.rs +++ b/crates/terminal/src/terminal_view.rs @@ -339,7 +339,7 @@ impl View for TerminalView { .contained() .boxed(), ) - .with_child(ChildView::new(&self.context_menu).boxed()) + .with_child(ChildView::new(&self.context_menu, cx).boxed()) .boxed() } diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 729cdad73911726de720247c1330663e61cbf9b0..a014fc8e927fb7e816da3f5fe6d095a02252d750 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -262,8 +262,8 @@ impl View for ThemeSelector { "ThemeSelector" } - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - ChildView::new(self.picker.clone()).boxed() + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + ChildView::new(self.picker.clone(), cx).boxed() } fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index dcfcc9e983f5e9939c6ae01517297683481c267b..cf6c8e287b397cdf38e00f28baa1d6967c28ecee 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -255,7 +255,7 @@ impl Dock { enum DockResizeHandle {} - let resizable = Container::new(ChildView::new(self.pane.clone()).boxed()) + let resizable = Container::new(ChildView::new(self.pane.clone(), cx).boxed()) .with_style(panel_style) .with_resize_handle::( resize_side as usize, @@ -285,8 +285,8 @@ impl Dock { enum ExpandedDockPane {} Container::new( MouseEventHandler::::new(0, cx, |_state, cx| { - MouseEventHandler::::new(0, cx, |_state, _cx| { - ChildView::new(self.pane.clone()).boxed() + MouseEventHandler::::new(0, cx, |_state, cx| { + ChildView::new(&self.pane, cx).boxed() }) .capture_all() .contained() diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 7e2cc0d599723850d07e8b51ec8ead1992bc1d87..c8019e1123a422a728fef5374cd7da3c18d8078f 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1439,8 +1439,8 @@ impl View for Pane { .flex(1., false) .named("tab bar") }) - .with_child(ChildView::new(&self.toolbar).expanded().boxed()) - .with_child(ChildView::new(active_item).flex(1., true).boxed()) + .with_child(ChildView::new(&self.toolbar, cx).expanded().boxed()) + .with_child(ChildView::new(active_item, cx).flex(1., true).boxed()) .boxed() } else { enum EmptyPane {} @@ -1480,7 +1480,7 @@ impl View for Pane { }) .boxed(), ) - .with_child(ChildView::new(&self.tab_bar_context_menu).boxed()) + .with_child(ChildView::new(&self.tab_bar_context_menu, cx).boxed()) .named("pane") } diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs index f09c31741ef2a60e5475057d0d0e9394d121d3c4..10fac09fff9376194374054da9b36f0e65af9cc9 100644 --- a/crates/workspace/src/pane_group.rs +++ b/crates/workspace/src/pane_group.rs @@ -222,7 +222,12 @@ impl Member { }; Stack::new() - .with_child(ChildView::new(pane).contained().with_border(border).boxed()) + .with_child( + ChildView::new(pane, cx) + .contained() + .with_border(border) + .boxed(), + ) .with_children(prompt) .boxed() } diff --git a/crates/workspace/src/sidebar.rs b/crates/workspace/src/sidebar.rs index 5cf986128a5254a17fd1b93c2c918c181aed579e..214f2277579f37dd057b7c75d3c8d4bf85cc0472 100644 --- a/crates/workspace/src/sidebar.rs +++ b/crates/workspace/src/sidebar.rs @@ -192,7 +192,7 @@ impl View for Sidebar { if let Some(active_item) = self.active_item() { enum ResizeHandleTag {} let style = &cx.global::().theme.workspace.sidebar; - ChildView::new(active_item.to_any()) + ChildView::new(active_item.to_any(), cx) .contained() .with_style(style.container) .with_resize_handle::( diff --git a/crates/workspace/src/status_bar.rs b/crates/workspace/src/status_bar.rs index f055168075ac5ae059eafb034206591d09cf189e..5261d22b6c8933ba9c382af85a6f025d092ed589 100644 --- a/crates/workspace/src/status_bar.rs +++ b/crates/workspace/src/status_bar.rs @@ -42,14 +42,14 @@ impl View for StatusBar { let theme = &cx.global::().theme.workspace.status_bar; Flex::row() .with_children(self.left_items.iter().map(|i| { - ChildView::new(i.as_ref()) + ChildView::new(i.as_ref(), cx) .aligned() .contained() .with_margin_right(theme.item_spacing) .boxed() })) .with_children(self.right_items.iter().rev().map(|i| { - ChildView::new(i.as_ref()) + ChildView::new(i.as_ref(), cx) .aligned() .contained() .with_margin_left(theme.item_spacing) diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index d1d666e031205239c825192f0d0b08ccec6a6244..7443f19003fabcf14771655a39d51bd07f8a6813 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -67,7 +67,7 @@ impl View for Toolbar { match *position { ToolbarItemLocation::Hidden => {} ToolbarItemLocation::PrimaryLeft { flex } => { - let left_item = ChildView::new(item.as_ref()) + let left_item = ChildView::new(item.as_ref(), cx) .aligned() .contained() .with_margin_right(spacing); @@ -78,7 +78,7 @@ impl View for Toolbar { } } ToolbarItemLocation::PrimaryRight { flex } => { - let right_item = ChildView::new(item.as_ref()) + let right_item = ChildView::new(item.as_ref(), cx) .aligned() .contained() .with_margin_left(spacing) @@ -91,7 +91,7 @@ impl View for Toolbar { } ToolbarItemLocation::Secondary => { secondary_item = Some( - ChildView::new(item.as_ref()) + ChildView::new(item.as_ref(), cx) .constrained() .with_height(theme.height) .boxed(), diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 1f2847fd8f6c58e0a7990076460e3e66bf4d117a..ece8cedfb1e516f8ff66fc8c76b3814c0fabd85b 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2126,7 +2126,7 @@ impl Workspace { enum TitleBar {} ConstrainedBox::new( - MouseEventHandler::::new(0, cx, |_, _| { + MouseEventHandler::::new(0, cx, |_, cx| { Container::new( Stack::new() .with_child( @@ -2138,7 +2138,7 @@ impl Workspace { .with_children( self.titlebar_item .as_ref() - .map(|item| ChildView::new(item).aligned().right().boxed()), + .map(|item| ChildView::new(item, cx).aligned().right().boxed()), ) .boxed(), ) @@ -2231,14 +2231,18 @@ impl Workspace { } } - fn render_notifications(&self, theme: &theme::Workspace) -> Option { + fn render_notifications( + &self, + theme: &theme::Workspace, + cx: &AppContext, + ) -> Option { if self.notifications.is_empty() { None } else { Some( Flex::column() .with_children(self.notifications.iter().map(|(_, _, notification)| { - ChildView::new(notification.as_ref()) + ChildView::new(notification.as_ref(), cx) .contained() .with_style(theme.notification) .boxed() @@ -2570,7 +2574,7 @@ impl View for Workspace { .with_children( if self.left_sidebar.read(cx).active_item().is_some() { Some( - ChildView::new(&self.left_sidebar) + ChildView::new(&self.left_sidebar, cx) .flex(0.8, false) .boxed(), ) @@ -2606,7 +2610,7 @@ impl View for Workspace { .with_children( if self.right_sidebar.read(cx).active_item().is_some() { Some( - ChildView::new(&self.right_sidebar) + ChildView::new(&self.right_sidebar, cx) .flex(0.8, false) .boxed(), ) @@ -2624,15 +2628,17 @@ impl View for Workspace { DockAnchor::Expanded, cx, )) - .with_children(self.modal.as_ref().map(|m| { - ChildView::new(m) + .with_children(self.modal.as_ref().map(|modal| { + ChildView::new(modal, cx) .contained() .with_style(theme.workspace.modal) .aligned() .top() .boxed() })) - .with_children(self.render_notifications(&theme.workspace)) + .with_children( + self.render_notifications(&theme.workspace, cx), + ) .boxed(), ) .boxed(), @@ -2640,7 +2646,7 @@ impl View for Workspace { .flex(1.0, true) .boxed(), ) - .with_child(ChildView::new(&self.status_bar).boxed()) + .with_child(ChildView::new(&self.status_bar, cx).boxed()) .contained() .with_background_color(theme.workspace.background) .boxed(), From 1bec8087eed53c95fb0e513217da8b4dcd3fc555 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 15:59:43 +0200 Subject: [PATCH 4/5] Add unit test for `ChildView` --- crates/gpui/src/app.rs | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 83936c617e5801b395aa5aef864d578ce516083c..76aae930a103dc37d4875755612c51196bfca0b4 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -7044,4 +7044,73 @@ mod tests { cx.simulate_window_activation(Some(window_3)); assert_eq!(mem::take(&mut *events.borrow_mut()), []); } + + #[crate::test(self)] + fn test_child_view(cx: &mut MutableAppContext) { + struct Child { + rendered: Rc>, + dropped: Rc>, + } + + impl super::Entity for Child { + type Event = (); + } + + impl super::View for Child { + fn ui_name() -> &'static str { + "child view" + } + + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + self.rendered.set(true); + Empty::new().boxed() + } + } + + impl Drop for Child { + fn drop(&mut self) { + self.dropped.set(true); + } + } + + struct Parent { + child: Option>, + } + + impl super::Entity for Parent { + type Event = (); + } + + impl super::View for Parent { + fn ui_name() -> &'static str { + "parent view" + } + + fn render(&mut self, cx: &mut RenderContext) -> ElementBox { + if let Some(child) = self.child.as_ref() { + ChildView::new(child, cx).boxed() + } else { + Empty::new().boxed() + } + } + } + + let child_rendered = Rc::new(Cell::new(false)); + let child_dropped = Rc::new(Cell::new(false)); + let (_, root_view) = cx.add_window(Default::default(), |cx| Parent { + child: Some(cx.add_view(|_| Child { + rendered: child_rendered.clone(), + dropped: child_dropped.clone(), + })), + }); + assert!(child_rendered.take()); + assert!(!child_dropped.take()); + + root_view.update(cx, |view, cx| { + view.child.take(); + cx.notify(); + }); + assert!(!child_rendered.take()); + assert!(child_dropped.take()); + } } From 9ebd5863505dbdd85cc37e7728a01f69461acb2d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 16:40:52 +0200 Subject: [PATCH 5/5] Improve error message when rendering a child view for a dropped view Co-Authored-By: Nathan Sobo --- crates/gpui/src/presenter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index 1d59905d7cc8a542505d0fcb3c19a94cca963e0b..4eef9c73985ae093ea1b57f76692881bf0a0fb21 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -1001,7 +1001,7 @@ impl Element for ChildView { (size, true) } else { log::error!( - "layout called on a view (id: {}, name: {:?}) that was not rendered", + "layout called on a ChildView element whose underlying view was dropped (view_id: {}, name: {:?})", self.view.id(), self.view_name ); @@ -1020,7 +1020,7 @@ impl Element for ChildView { cx.paint(self.view.id(), bounds.origin(), visible_bounds); } else { log::error!( - "paint called on a view (id: {}, name: {:?}) that was not rendered", + "paint called on a ChildView element whose underlying view was dropped (view_id: {}, name: {:?})", self.view.id(), self.view_name ); @@ -1040,7 +1040,7 @@ impl Element for ChildView { cx.dispatch_event(self.view.id(), event) } else { log::error!( - "dispatch_event called on a view (id: {}, name: {:?}) that was not rendered", + "dispatch_event called on a ChildView element whose underlying view was dropped (view_id: {}, name: {:?})", self.view.id(), self.view_name ); @@ -1061,7 +1061,7 @@ impl Element for ChildView { cx.rect_for_text_range(self.view.id(), range_utf16) } else { log::error!( - "rect_for_text_range called on a view (id: {}, name: {:?}) that was not rendered", + "rect_for_text_range called on a ChildView element whose underlying view was dropped (view_id: {}, name: {:?})", self.view.id(), self.view_name );