diff --git a/crates/webui-handler/src/lib.rs b/crates/webui-handler/src/lib.rs index e9bc7103..7b836b4b 100644 --- a/crates/webui-handler/src/lib.rs +++ b/crates/webui-handler/src/lib.rs @@ -479,6 +479,9 @@ struct LocalValueSources<'ctx, 'protocol, 'state> { borrowed: &'ctx BorrowedScope<'protocol, 'state>, } +/// Route level an `` matches against. +pub(crate) type RouteChildren<'protocol> = Cow<'protocol, [webui_protocol::WebUiFragmentRoute]>; + #[derive(Default)] struct BorrowedScope<'protocol, 'state> { inline: [Option<(&'protocol str, &'state Value)>; INLINE_SCOPE_SLOTS], @@ -825,6 +828,27 @@ fn component_attr_source(attribute: &webui_protocol::WebUIFragmentAttribute) -> } } +/// Route level one step below `children[index]`. +/// +/// A borrowed level yields a borrowed sublevel, so descending through a route +/// tree during a render never copies a `WebUiFragmentRoute`. A level a +/// streaming continuation already materialized transfers its child level. +fn descend_into<'protocol>( + children: &mut RouteChildren<'protocol>, + index: usize, +) -> RouteChildren<'protocol> { + match children { + Cow::Borrowed(routes) => match routes.get(index) { + Some(route) => Cow::Borrowed(&route.children), + None => Cow::Borrowed(&[]), + }, + Cow::Owned(routes) => match routes.get_mut(index) { + Some(route) => Cow::Owned(std::mem::take(&mut route.children)), + None => Cow::Borrowed(&[]), + }, + } +} + /// Fragment ID a fragment descends into, or `None` when it renders inline. fn fragment_target_id(fragment: &WebUIFragment) -> Option<&str> { match fragment.fragment.as_ref()? { @@ -884,7 +908,12 @@ pub(crate) struct WebUIProcessContext<'protocol, 'state, 'output> { pub(crate) plugin: Option>, /// Current position in the route tree for outlet-based rendering. /// Contains the children of the currently matched route fragment. - pub(crate) route_children: Vec, + /// + /// A matched route's `children` is a recursive prost subtree, so the + /// borrowed variant avoids deep-cloning the whole remaining route tree on + /// every matched route of every request. Only a suspended streaming + /// continuation, which outlives the borrow, materializes the owned variant. + pub(crate) route_children: RouteChildren<'protocol>, /// Entry fragment ID — used to compute the initial inventory at head_end. /// Borrowed from `RenderOptions<'a>::entry_id` — zero-copy. pub(crate) entry_id: &'protocol str, @@ -2338,7 +2367,15 @@ impl WebUIHandler { /// Matches children from the currently active route's `children` field /// against the request path, renders the matched child `` /// elements directly at this position (no wrapper element). - fn process_outlet(&self, context: &mut WebUIProcessContext) -> Result<()> { + fn process_outlet<'protocol>( + &self, + context: &mut WebUIProcessContext<'protocol, '_, '_>, + ) -> Result<()> { + // Moved out so the matched child can render with the context pointing + // at its grandchildren. The level is deliberately not put back, which + // preserves the previous behavior exactly: a second `` at + // this level renders nothing. That is a latent bug tracked by #515, not + // a property this function needs; fixing it belongs in its own change. let mut children = std::mem::take(&mut context.route_children); if children.is_empty() { return Ok(()); @@ -2364,17 +2401,11 @@ impl WebUIHandler { } } - // Extract grandchildren from the matched child to avoid cloning. - // We swap out the children vec so we can move it into context without - // cloning, then swap an empty vec back for the sibling rendering pass. - let grandchildren = if let Some((idx, _)) = &best { - std::mem::take(&mut children[*idx].children) - } else { - Vec::new() - }; - if let Some((idx, ref rm)) = best { - let matched_child = &children[idx]; + let descended = descend_into(&mut children, idx); + let Some(matched_child) = children.get(idx) else { + return Ok(()); + }; let comp = &matched_child.fragment_id; if !comp.is_empty() { @@ -2385,9 +2416,7 @@ impl WebUIHandler { ); std::mem::replace(&mut context.route_base, Cow::Owned(base)) }); - let saved_route_children = std::mem::take(&mut context.route_children); - - context.route_children = grandchildren; + context.route_children = descended; // Emit matched context.writer.write("` with matched/hidden state. - fn process_route( + fn process_route<'protocol>( &self, - route_frag: &webui_protocol::WebUiFragmentRoute, + route_frag: &'protocol webui_protocol::WebUiFragmentRoute, best_route: &Option<(String, route_matcher::RouteMatch)>, - context: &mut WebUIProcessContext, + context: &mut WebUIProcessContext<'protocol, '_, '_>, ) -> Result<()> { let is_matched = best_route .as_ref() @@ -2873,8 +2905,10 @@ impl WebUIHandler { route_matcher::compute_route_base(context.request_path, rm.consumed_segments); std::mem::replace(&mut context.route_base, Cow::Owned(base)) }); - let saved_route_children = std::mem::take(&mut context.route_children); - context.route_children = route_frag.children.clone(); + let saved_route_children = std::mem::replace( + &mut context.route_children, + Cow::Borrowed(&route_frag.children), + ); if !route_frag.content_fragment_id.is_empty() { self.process_fragment_target(None, &route_frag.content_fragment_id, context)?; @@ -3975,7 +4009,7 @@ impl WebUIHandler { route_base: Cow::Borrowed("/"), rendered_components: HashSet::new(), plugin: self.plugin_factory.map(|f| f()), - route_children: Vec::new(), + route_children: Cow::Borrowed(&[]), entry_id: options.entry_id, // Same defensive normalisation as `handle()`. See the // doc-comment there for the CSP-outage rationale. @@ -4147,6 +4181,50 @@ mod tests { ) } + #[test] + fn route_descent_borrows_protocol_children() { + let routes = vec![WebUiFragmentRoute { + children: vec![WebUiFragmentRoute { + fragment_id: "child".to_string(), + ..Default::default() + }], + ..Default::default() + }]; + let mut level = RouteChildren::Borrowed(&routes); + + let descended = descend_into(&mut level, 0); + + let Cow::Borrowed(children) = descended else { + panic!("protocol-owned route children should remain borrowed"); + }; + assert!(std::ptr::eq(children, routes[0].children.as_slice())); + } + + #[test] + fn route_descent_moves_parked_children() { + let mut level = RouteChildren::Owned(vec![WebUiFragmentRoute { + children: vec![WebUiFragmentRoute { + fragment_id: "child".to_string(), + ..Default::default() + }], + ..Default::default() + }]); + + let descended = descend_into(&mut level, 0); + + let Cow::Owned(children) = descended else { + panic!("parked route children must not borrow session-owned storage"); + }; + assert_eq!(children[0].fragment_id, "child"); + let Cow::Owned(source) = level else { + panic!("the parked source level must stay owned"); + }; + assert!( + source[0].children.is_empty(), + "owned route children must move out instead of being cloned" + ); + } + #[test] fn borrowed_collection_reuses_state_array() { let state = test_json!({ diff --git a/crates/webui-handler/src/streaming/inventory.rs b/crates/webui-handler/src/streaming/inventory.rs index e7849795..1cfcaf2d 100644 --- a/crates/webui-handler/src/streaming/inventory.rs +++ b/crates/webui-handler/src/streaming/inventory.rs @@ -420,7 +420,7 @@ mod tests { route_base: std::borrow::Cow::Borrowed("/account"), rendered_components: std::collections::HashSet::new(), plugin: None, - route_children: Vec::new(), + route_children: std::borrow::Cow::Borrowed(&[]), entry_id: "index.html", nonce: None, component_index, diff --git a/crates/webui-handler/src/streaming/session.rs b/crates/webui-handler/src/streaming/session.rs index 932a565b..b195e309 100644 --- a/crates/webui-handler/src/streaming/session.rs +++ b/crates/webui-handler/src/streaming/session.rs @@ -365,7 +365,7 @@ pub(crate) struct SessionCore { document_style_resources: HashSet, shadow_style_roots: Vec, plugin: Option>, - route_children: Vec, + route_children: crate::RouteChildren<'static>, head_end_emitted: bool, body_start_emitted: bool, component_asset_styles_emitted: bool, @@ -408,7 +408,7 @@ impl SessionCore { document_style_resources: HashSet::new(), shadow_style_roots: Vec::new(), plugin: handler.plugin_factory.map(|factory| factory()), - route_children: Vec::new(), + route_children: Cow::Owned(Vec::new()), head_end_emitted: false, body_start_emitted: false, component_asset_styles_emitted: false, @@ -730,7 +730,7 @@ impl SessionCore { }; self.rendered_components = std::mem::take(&mut context.rendered_components); self.plugin = context.plugin.take(); - self.route_children = std::mem::take(&mut context.route_children); + self.route_children = Cow::Owned(std::mem::take(&mut context.route_children).into_owned()); self.head_end_emitted = context.head_end_emitted; self.body_start_emitted = context.body_start_emitted; self.component_asset_styles_emitted = context.component_asset_styles_emitted; @@ -953,6 +953,42 @@ mod tests { RenderOptions::new("index.html", "/") } + #[test] + fn session_parks_owned_routes_and_retains_scope_pool() -> Result<()> { + let protocol = boundary_protocol(2, StateProjectionMode::Keys); + let handler = WebUIHandler::new(); + let state = test_json!({ "count": 3, "title": "pooled" }); + let render_options = options(); + let mut sink = TestSink { + output: String::new(), + }; + let mut response = handler.stream_response(&protocol, &render_options, &mut sink)?; + + let first = response.start(&state)?; + assert!(matches!(response.core.route_children, Cow::Owned(_))); + let Some(boundary) = first.boundary else { + panic!("the first boundary should suspend"); + }; + + response.resume(boundary.instance_id, &state, BoundaryMode::Final)?; + assert!(matches!(response.core.route_children, Cow::Owned(_))); + let pooled_capacity = response + .core + .scope_pool + .first() + .map(HashMap::capacity) + .unwrap_or_else(|| panic!("the completed component should recycle its scope map")); + + response.advance()?; + assert!(matches!(response.core.route_children, Cow::Owned(_))); + assert_eq!( + response.core.scope_pool.first().map(HashMap::capacity), + Some(pooled_capacity), + "a suspension step must preserve the scope-map pool" + ); + Ok(()) + } + #[test] fn render_streaming_projects_full_state_once_per_response() -> Result<()> { // Full-state protocols retain the caller's whole tree. Committing each diff --git a/crates/webui-handler/src/streaming/vm.rs b/crates/webui-handler/src/streaming/vm.rs index 68fad471..13d96a4d 100644 --- a/crates/webui-handler/src/streaming/vm.rs +++ b/crates/webui-handler/src/streaming/vm.rs @@ -456,7 +456,7 @@ impl ContinuationVm { } => { context.writer.write("")?; context.route_base = Cow::Owned(saved_route_base.into_string()); - context.route_children = saved_route_children; + context.route_children = Cow::Owned(saved_route_children); } Frame::Outlet(frame) => self.step_outlet(frame, handler, protocol, context)?, } @@ -1319,8 +1319,11 @@ impl ContinuationVm { ) .into_owned() .into_boxed_str(); - let saved_route_children = - std::mem::replace(&mut context.route_children, route.children.clone()); + let saved_route_children = std::mem::replace( + &mut context.route_children, + Cow::Owned(route.children.clone()), + ) + .into_owned(); self.push(Frame::RouteEnd { saved_route_base, saved_route_children, @@ -1338,7 +1341,7 @@ impl ContinuationVm { } fn begin_outlet(&mut self, context: &mut WebUIProcessContext<'_, '_, '_>) -> Result<()> { - let routes = std::mem::take(&mut context.route_children); + let routes = std::mem::take(&mut context.route_children).into_owned(); if routes.is_empty() { return Ok(()); } diff --git a/crates/webui-handler/tests/streaming.rs b/crates/webui-handler/tests/streaming.rs index 12834e1c..53795617 100644 --- a/crates/webui-handler/tests/streaming.rs +++ b/crates/webui-handler/tests/streaming.rs @@ -739,6 +739,48 @@ fn selected_route_component_can_suspend_inside_generated_host() { .contains(r#""#)); } +#[test] +fn boundary_free_component_descends_owned_nested_routes() { + let protocol = parsed_protocol( + &document(concat!( + r#""#, + r#""#, + r#""#, + r#""#, + "", + )), + &[ + ( + "route-shell", + concat!( + "", + r#"

ready

"#, + ), + ), + ("layout-shell", "
"), + ( + "section-page", + "

Section

", + ), + ("topic-page", "

Topic

"), + ("lesson-page", "

Lesson

"), + ], + ); + let mut session = new_session(protocol, "/sections/alpha/topics/beta/lessons/gamma"); + + let start = session.start(&test_json!({})).unwrap(); + + assert_eq!(start.boundary.as_ref().unwrap().name.as_ref(), "ready"); + let html = String::from_utf8(start.bytes).unwrap(); + let section = html.find("

Section

").unwrap(); + let topic = html.find("

Topic

").unwrap(); + let lesson = html.find("

Lesson

").unwrap(); + assert!(section < topic && topic < lesson, "{html}"); + assert!(html.contains(r#"