From 1908b8b4ea3a974889c0ca0086580e52b742bc5b Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 2 Sep 2026 16:31:05 +0800 Subject: [PATCH] perf(response): defer redundant body text copies --- Cargo.lock | 1 - moli-page-types/Cargo.toml | 1 - moli-page-types/src/lib.rs | 284 ++++++------------ .../src/renderer_transport_memory.rs | 14 +- moli-protocol/src/conn/body_spool.rs | 6 +- moli-protocol/src/domains/network/activity.rs | 4 +- moli-protocol/src/domains/network/backlog.rs | 4 +- .../src/domains/network/output_queue.rs | 18 +- .../domains/network/tests/response_body.rs | 2 +- moli-protocol/src/domains/page.rs | 11 +- moli-renderer-v8/src/app_manifest.rs | 5 +- .../context_host/child_documents/loads.rs | 5 +- .../context_host/resource_loading.rs | 33 +- .../context_host/websockets/registration.rs | 2 +- .../src/network_host/body_source.rs | 5 +- .../src/network_host/preflight_events.rs | 2 +- moli-renderer-v8/src/runtime/page_surface.rs | 4 +- .../src/script_vm/subresource_fetch.rs | 6 +- .../src/script_vm/tests/browser_api/misc.rs | 12 +- .../src/script_vm/tests/dom_xhr/xhr.rs | 4 +- moli-renderer-v8/src/script_vm/tests/mod.rs | 5 +- .../src/worker/global_scope/fetch.rs | 2 +- .../src/worker/global_scope/mod.rs | 4 +- .../src/worker/thread/tests/network.rs | 10 +- moli/src/network_trace.rs | 3 +- 25 files changed, 179 insertions(+), 268 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab355661b..54f9083b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2695,7 +2695,6 @@ dependencies = [ "moli-dom", "moli-fetch", "moli-web-mime", - "parking_lot", "regex", "serde", "serde_json", diff --git a/moli-page-types/Cargo.toml b/moli-page-types/Cargo.toml index e38b087fd..34113d356 100644 --- a/moli-page-types/Cargo.toml +++ b/moli-page-types/Cargo.toml @@ -14,7 +14,6 @@ moli-dom = { path = "../moli-dom" } moli-fetch = { path = "../moli-fetch" } moli-web-mime = { path = "../moli-web-mime" } http-auth = { version = "0.1.10", default-features = false } -parking_lot = "0.12" regex = "1.12.3" serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.145" diff --git a/moli-page-types/src/lib.rs b/moli-page-types/src/lib.rs index 017982347..6dc30a332 100644 --- a/moli-page-types/src/lib.rs +++ b/moli-page-types/src/lib.rs @@ -26,7 +26,6 @@ use std::{ #[cfg(unix)] use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt}; -use parking_lot::Mutex; use regex::Regex; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -1477,15 +1476,17 @@ pub struct SubresourceResponseBody { #[derive(Debug)] enum SubresourceResponseBodyInner { - Memory { - text: String, - bytes: Vec, - }, - File { - path: PathBuf, - len: usize, - text_cache: Mutex>, - }, + Memory(Vec), + File { path: PathBuf, len: usize }, +} + +impl SubresourceResponseBodyInner { + fn in_memory_bytes(&self) -> Option<&[u8]> { + match self { + Self::Memory(bytes) => Some(bytes), + Self::File { .. } => None, + } + } } impl Drop for SubresourceResponseBodyInner { @@ -1501,19 +1502,13 @@ impl PartialEq for SubresourceResponseBody { if Arc::ptr_eq(&self.inner, &other.inner) { return true; } - match (self.inner.as_ref(), other.inner.as_ref()) { - ( - SubresourceResponseBodyInner::Memory { - text: left_text, - bytes: left_bytes, - }, - SubresourceResponseBodyInner::Memory { - text: right_text, - bytes: right_bytes, - }, - ) => left_text == right_text && left_bytes == right_bytes, - _ => false, - } + let Some(left_bytes) = self.inner.in_memory_bytes() else { + return false; + }; + let Some(right_bytes) = other.inner.in_memory_bytes() else { + return false; + }; + left_bytes == right_bytes } } @@ -1593,7 +1588,6 @@ impl SubresourceResponseBodyWriter { inner: Arc::new(SubresourceResponseBodyInner::File { path, len: self.len, - text_cache: Mutex::new(None), }), }; } @@ -1603,10 +1597,7 @@ impl SubresourceResponseBodyWriter { if let Some(path) = self.path.take() { let _ = fs::remove_file(path); } - SubresourceResponseBody::from_text_and_bytes( - String::from_utf8_lossy(&self.memory).into_owned(), - std::mem::take(&mut self.memory), - ) + SubresourceResponseBody::from_bytes(std::mem::take(&mut self.memory)) } fn ensure_file(&mut self) -> io::Result<()> { @@ -1659,115 +1650,22 @@ impl Drop for SubresourceResponseBodyWriter { } impl SubresourceResponseBody { - pub fn from_text(text: String) -> Self { - let bytes = text.as_bytes().to_vec(); - Self::from_text_and_bytes(text, bytes) - } - - pub fn from_text_and_bytes(text: String, bytes: Vec) -> Self { + pub fn from_bytes(bytes: Vec) -> Self { Self { - inner: Arc::new(SubresourceResponseBodyInner::Memory { text, bytes }), + inner: Arc::new(SubresourceResponseBodyInner::Memory(bytes)), } } - pub fn from_materialized_body(body: ResponseBody) -> Self { - let (text, bytes) = body - .try_into_lossy_materialized_text() - .expect("SubresourceResponseBody should be built from a materialized body"); - Self::from_text_and_bytes(text, bytes) - } - - /// Builds the neutral subresource body carrier from a materialized fetch - /// response without exposing a loose `(String, Vec)` pair to callers. + /// Copies the exact bytes from a materialized fetch response into the + /// renderer-neutral subresource body carrier. pub fn from_fetch_response(response: &Response) -> Self { - Self::from_materialized_body(response.materialized_body()) + Self::from_bytes(response.body_bytes().to_vec()) } - /// Builds the neutral subresource body carrier from a materialized - /// navigation response at the explicit compatibility boundary. + /// Copies the exact bytes from a materialized navigation response into the + /// renderer-neutral subresource body carrier. pub fn from_navigation_response(response: &NavigationResponse) -> Self { - Self::from_materialized_body(response.materialized_body()) - } - - /// Builds a materialized navigation response at a compatibility boundary - /// that still needs both the lossy text view and exact bytes. - pub fn to_navigation_response(&self, head: ResponseHead) -> NavigationResponse { - self.diagnostic_navigation_response(head) - } - - /// Best-effort navigation response for diagnostics and legacy tests. - /// Production protocol paths should use `try_to_navigation_response`. - pub fn diagnostic_navigation_response(&self, head: ResponseHead) -> NavigationResponse { - NavigationResponse::from_head_and_materialized_body( - head, - self.diagnostic_materialized_body(), - ) - } - - /// Fallible variant of `to_navigation_response` for callers that can - /// surface file-backed body source errors instead of treating them as an - /// empty body. - pub fn try_to_navigation_response(&self, head: ResponseHead) -> io::Result { - self.try_materialized_body() - .map(|body| NavigationResponse::from_head_and_materialized_body(head, body)) - } - - /// Builds a materialized response body at an explicit compatibility - /// boundary. File-backed bodies are read once so callers that need both the - /// text view and exact bytes do not duplicate spool I/O. - pub fn materialized_body(&self) -> ResponseBody { - self.diagnostic_materialized_body() - } - - /// Best-effort materialized body for diagnostics and legacy tests. - /// Production protocol paths should use `try_materialized_body`. - pub fn diagnostic_materialized_body(&self) -> ResponseBody { - self.try_materialized_body() - .unwrap_or_else(|_| ResponseBody::materialized_text(String::new(), Vec::new())) - } - - /// Fallible materialization for production paths that need to distinguish - /// source read failure from a legitimate empty body. - pub fn try_materialized_body(&self) -> io::Result { - match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { text, bytes } => { - Ok(ResponseBody::materialized_text(text.clone(), bytes.clone())) - } - SubresourceResponseBodyInner::File { text_cache, .. } => { - let bytes = self.materialize_bytes()?; - let mut cache = text_cache.lock(); - let text = cache - .get_or_insert_with(|| String::from_utf8_lossy(&bytes).into_owned()) - .clone(); - Ok(ResponseBody::materialized_text(text, bytes)) - } - } - } - - pub fn text(&self) -> Cow<'_, str> { - self.diagnostic_text() - } - - /// Best-effort text view for diagnostics and legacy tests. Production - /// protocol paths should use `try_text` so source read failures remain - /// visible instead of becoming an empty string. - pub fn diagnostic_text(&self) -> Cow<'_, str> { - self.try_text() - .unwrap_or_else(|_| Cow::Owned(String::new())) - } - - pub fn try_text(&self) -> io::Result> { - match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { text, .. } => Ok(Cow::Borrowed(text)), - SubresourceResponseBodyInner::File { text_cache, .. } => { - let mut cache = text_cache.lock(); - if cache.is_none() { - let bytes = self.materialize_bytes()?; - *cache = Some(String::from_utf8_lossy(&bytes).into_owned()); - } - Ok(Cow::Owned(cache.as_deref().unwrap_or_default().to_owned())) - } - } + Self::from_bytes(response.body_bytes().to_vec()) } pub fn bytes(&self) -> Cow<'_, [u8]> { @@ -1782,7 +1680,7 @@ impl SubresourceResponseBody { pub fn try_bytes(&self) -> io::Result> { match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { bytes, .. } => Ok(Cow::Borrowed(bytes)), + SubresourceResponseBodyInner::Memory(bytes) => Ok(Cow::Borrowed(bytes)), SubresourceResponseBodyInner::File { .. } => self.materialize_bytes().map(Cow::Owned), } } @@ -1829,7 +1727,7 @@ impl SubresourceResponseBody { pub fn materialize_bytes_from(&self, offset: usize) -> io::Result> { match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { bytes, .. } => { + SubresourceResponseBodyInner::Memory(bytes) => { Ok(bytes.get(offset..).map(<[u8]>::to_vec).unwrap_or_default()) } SubresourceResponseBodyInner::File { path, len, .. } => { @@ -1850,7 +1748,7 @@ impl SubresourceResponseBody { return Ok(Vec::new()); } match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { bytes, .. } => { + SubresourceResponseBodyInner::Memory(bytes) => { let Some(remaining) = bytes.get(offset..) else { return Ok(Vec::new()); }; @@ -1873,7 +1771,7 @@ impl SubresourceResponseBody { pub fn write_bytes_to(&self, writer: &mut W) -> io::Result<()> { match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { bytes, .. } => writer.write_all(bytes), + SubresourceResponseBodyInner::Memory(bytes) => writer.write_all(bytes), SubresourceResponseBodyInner::File { path, .. } => { let mut file = File::open(path)?; let mut buffer = [0; 64 * 1024]; @@ -1891,7 +1789,7 @@ impl SubresourceResponseBody { pub fn len(&self) -> usize { match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { bytes, .. } => bytes.len(), + SubresourceResponseBodyInner::Memory(bytes) => bytes.len(), SubresourceResponseBodyInner::File { len, .. } => *len, } } @@ -1998,11 +1896,12 @@ impl SubresourceResponseWaitCriteria { return Ok(!self.requires_response_body()); }; - let response_body_text = if self.requires_response_body() { - Some(response_body.try_text()?) + let response_body_bytes = if self.requires_response_body() { + Some(response_body.try_bytes()?) } else { None }; + let response_body_text = response_body_bytes.as_deref().map(String::from_utf8_lossy); if let Some(needle) = self.body_contains.as_deref() && !response_body_text @@ -2244,7 +2143,7 @@ impl SubresourceNetworkRecord { final_url, status, response_headers, - SubresourceResponseBody::from_text(response_body), + SubresourceResponseBody::from_bytes(response_body.into_bytes()), cookie_set_reports, ) } @@ -3852,7 +3751,7 @@ mod tests { ); let body = SubresourceBodyFinished::ready( handle, - SubresourceResponseBody::from_text_and_bytes(String::new(), vec![1, 2, 3]), + SubresourceResponseBody::from_bytes(vec![1, 2, 3]), ); let mut report = ScriptExecutionReport::default(); report.extend_network_output(ScriptNetworkOutput::from_items([ @@ -4059,6 +3958,55 @@ mod tests { ); } + #[test] + fn subresource_response_body_stores_only_exact_response_bytes() { + let response = Response::from_head_and_body( + ResponseHead { + final_url: test_url("/utf8.txt"), + status: 200, + headers: Vec::new(), + request_cookie_report: None, + cookie_set_reports: Vec::new(), + redirected: false, + redirect_chain: Vec::new(), + from_cache: false, + negotiated_http_version: None, + }, + "hello".to_owned(), + b"hello".to_vec(), + ); + let body = SubresourceResponseBody::from_fetch_response(&response); + let SubresourceResponseBodyInner::Memory(bytes) = body.inner.as_ref() else { + panic!("fetch response should use in-memory byte storage"); + }; + + assert_eq!(bytes, b"hello"); + assert_eq!(body.try_bytes().unwrap().as_ref(), b"hello"); + } + + #[test] + fn subresource_response_body_preserves_non_utf8_response_bytes() { + let response = Response::from_head_and_body( + ResponseHead { + final_url: test_url("/legacy.txt"), + status: 200, + headers: Vec::new(), + request_cookie_report: None, + cookie_set_reports: Vec::new(), + redirected: false, + redirect_chain: Vec::new(), + from_cache: false, + negotiated_http_version: None, + }, + "é".to_owned(), + vec![0xe9], + ); + let body = SubresourceResponseBody::from_fetch_response(&response); + + assert_eq!(body.try_bytes().unwrap().as_ref(), &[0xe9]); + assert_eq!(String::from_utf8_lossy(&body.try_bytes().unwrap()), "�"); + } + #[test] fn subresource_response_body_writer_keeps_small_body_in_memory() { let mut writer = SubresourceResponseBodyWriter::new(16); @@ -4067,7 +4015,7 @@ mod tests { let body = writer.finish(); assert_eq!(body.len(), 11); - assert_eq!(body.diagnostic_text(), "hello world"); + assert_eq!(body.try_bytes().unwrap().as_ref(), b"hello world"); assert_eq!(body.clone_body_bytes(), b"hello world"); } @@ -4079,7 +4027,7 @@ mod tests { let body = writer.finish(); assert_eq!(body.len(), 11); - assert_eq!(body.diagnostic_text(), "hello world"); + assert_eq!(body.try_bytes().unwrap().as_ref(), b"hello world"); let mut copied = Vec::new(); body.write_bytes_to(&mut copied) @@ -4089,10 +4037,7 @@ mod tests { #[test] fn subresource_response_body_reads_memory_and_spooled_chunks_by_offset() { - let memory = SubresourceResponseBody::from_text_and_bytes( - "hello world".to_owned(), - b"hello world".to_vec(), - ); + let memory = SubresourceResponseBody::from_bytes(b"hello world".to_vec()); assert_eq!(memory.read_chunk(6, 3).unwrap(), b"wor"); assert_eq!(memory.materialize_bytes_from(6).unwrap(), b"world"); assert_eq!(memory.diagnostic_clone_body_bytes_from(6), b"world"); @@ -4117,35 +4062,13 @@ mod tests { inner: Arc::new(SubresourceResponseBodyInner::File { path: missing_path, len: 5, - text_cache: Mutex::new(None), }), }; assert!(body.materialize_bytes().is_err()); assert!(body.materialize_bytes_from(2).is_err()); - assert!(body.try_materialized_body().is_err()); - assert!(body.try_text().is_err()); assert!(body.try_bytes().is_err()); - assert!( - body.try_to_navigation_response(ResponseHead { - final_url: Url::parse("https://example.test/missing").unwrap(), - status: 200, - headers: Vec::new(), - request_cookie_report: None, - cookie_set_reports: Vec::new(), - redirected: false, - redirect_chain: Vec::new(), - from_cache: false, - negotiated_http_version: None, - }) - .is_err() - ); assert_eq!(body.diagnostic_clone_body_bytes(), Vec::::new()); - assert_eq!( - body.diagnostic_materialized_body().as_materialized_bytes(), - Some(&[][..]) - ); - assert_eq!(body.diagnostic_text(), ""); assert_eq!(body.diagnostic_bytes().as_ref(), &[] as &[u8]); let record = SubresourceNetworkRecord::success_with_body( @@ -4231,10 +4154,7 @@ mod tests { Url::parse("https://example.test/api").unwrap(), 200, vec![("content-type".to_owned(), "text/plain".to_owned())], - SubresourceResponseBody::from_text_and_bytes( - "order #42 ready".to_owned(), - b"order #42 ready".to_vec(), - ), + SubresourceResponseBody::from_bytes(b"order #42 ready".to_vec()), Vec::new(), ); @@ -4334,14 +4254,12 @@ mod tests { inner: Arc::new(SubresourceResponseBodyInner::File { path: missing_path_a, len: 5, - text_cache: Mutex::new(None), }), }; let body_b = SubresourceResponseBody { inner: Arc::new(SubresourceResponseBodyInner::File { path: missing_path_b, len: 5, - text_cache: Mutex::new(None), }), }; @@ -4427,28 +4345,16 @@ mod tests { } #[test] - fn subresource_response_body_to_navigation_response_materializes_spooled_body_once() { + fn subresource_response_body_preserves_invalid_bytes_when_spooled() { let mut writer = SubresourceResponseBodyWriter::new(2); writer.append(b"hi "); writer.append(&[0xff, b'!']); let body = writer.finish(); - let navigation = body.diagnostic_navigation_response(ResponseHead { - final_url: Url::parse("https://example.test/data").unwrap(), - status: 200, - headers: vec![( - "content-type".to_owned(), - "application/octet-stream".to_owned(), - )], - request_cookie_report: None, - cookie_set_reports: Vec::new(), - redirected: false, - redirect_chain: Vec::new(), - from_cache: false, - negotiated_http_version: None, - }); - assert_eq!(navigation.body_bytes(), &[b'h', b'i', b' ', 0xff, b'!']); - assert_eq!(navigation.body_text(), "hi \u{fffd}!"); + assert_eq!( + body.materialize_bytes().unwrap(), + &[b'h', b'i', b' ', 0xff, b'!'] + ); } #[test] diff --git a/moli-page-types/src/renderer_transport_memory.rs b/moli-page-types/src/renderer_transport_memory.rs index c3b226306..4df4eeaed 100644 --- a/moli-page-types/src/renderer_transport_memory.rs +++ b/moli-page-types/src/renderer_transport_memory.rs @@ -124,23 +124,13 @@ impl SubresourceResponseBody { #[doc(hidden)] pub fn renderer_transport_retained_memory_bytes(&self) -> usize { match self.inner.as_ref() { - SubresourceResponseBodyInner::Memory { text, bytes } => text + SubresourceResponseBodyInner::Memory(bytes) => bytes .capacity() - .saturating_add(bytes.capacity()) .saturating_add(std::mem::size_of::()), - SubresourceResponseBodyInner::File { - path, text_cache, .. - } => path + SubresourceResponseBodyInner::File { path, .. } => path .as_os_str() .len() .saturating_mul(2) - .saturating_add( - text_cache - .lock() - .as_ref() - .map(String::capacity) - .unwrap_or(0), - ) .saturating_add(std::mem::size_of::()), } } diff --git a/moli-protocol/src/conn/body_spool.rs b/moli-protocol/src/conn/body_spool.rs index a414a9f39..66435726c 100644 --- a/moli-protocol/src/conn/body_spool.rs +++ b/moli-protocol/src/conn/body_spool.rs @@ -558,10 +558,8 @@ mod tests { #[test] fn captured_body_from_subresource_response_body_reads_shared_source() -> Result<()> { - let subresource_body = moli_core::page::SubresourceResponseBody::from_text_and_bytes( - "hello world".to_owned(), - b"hello world".to_vec(), - ); + let subresource_body = + moli_core::page::SubresourceResponseBody::from_bytes(b"hello world".to_vec()); let body = CapturedBody::from_subresource_response_body(&subresource_body); assert!(matches!( diff --git a/moli-protocol/src/domains/network/activity.rs b/moli-protocol/src/domains/network/activity.rs index 474aa5d61..237d12eb4 100644 --- a/moli-protocol/src/domains/network/activity.rs +++ b/moli-protocol/src/domains/network/activity.rs @@ -622,7 +622,7 @@ mod tests { network_request_headers: None, response_status: 200, response_headers: vec![("content-type".to_owned(), "text/plain".to_owned())], - response_body: SubresourceResponseBody::from_text("prepared".to_owned()), + response_body: SubresourceResponseBody::from_bytes(b"prepared".to_vec()), from_cache: false, }); let action = crate::domains::activity::PreparedSubresourceContinueAction::capture_for_test( @@ -708,7 +708,7 @@ mod tests { network_request_headers: None, response_status: 200, response_headers: Vec::new(), - response_body: SubresourceResponseBody::from_text("old".to_owned()), + response_body: SubresourceResponseBody::from_bytes(b"old".to_vec()), from_cache: false, }); let old_action = diff --git a/moli-protocol/src/domains/network/backlog.rs b/moli-protocol/src/domains/network/backlog.rs index 0fa34640b..e36f083a6 100644 --- a/moli-protocol/src/domains/network/backlog.rs +++ b/moli-protocol/src/domains/network/backlog.rs @@ -1170,7 +1170,7 @@ mod tests { request_url.clone(), 200, Vec::new(), - SubresourceResponseBody::from_text("complete".to_owned()), + SubresourceResponseBody::from_bytes(b"complete".to_vec()), Vec::new(), ); assert_xhr_terminal_retains_internal_resource_type(vec![ @@ -1200,7 +1200,7 @@ mod tests { ); let body = SubresourceBodyFinished::ready( handle, - SubresourceResponseBody::from_text("staged".to_owned()), + SubresourceResponseBody::from_bytes(b"staged".to_vec()), ); assert_xhr_terminal_retains_internal_resource_type(vec![ ScriptNetworkOutputItem::SubresourceRequestStarted(Box::new(request)), diff --git a/moli-protocol/src/domains/network/output_queue.rs b/moli-protocol/src/domains/network/output_queue.rs index c7f3b6d96..8abee2eb0 100644 --- a/moli-protocol/src/domains/network/output_queue.rs +++ b/moli-protocol/src/domains/network/output_queue.rs @@ -3363,7 +3363,7 @@ mod tests { .with_from_cache(true); let body = SubresourceBodyFinished::ready( handle, - SubresourceResponseBody::from_text_and_bytes(String::new(), vec![1, 2, 3]), + SubresourceResponseBody::from_bytes(vec![1, 2, 3]), ); let items = vec![ ScriptNetworkOutputItem::SubresourceRequestStarted(Box::new(request)), @@ -3759,7 +3759,7 @@ mod tests { delivery_order_index, index, loader_id: "LOADER-1".to_owned(), - response_body: Some(SubresourceResponseBody::from_text(String::new())), + response_body: Some(SubresourceResponseBody::from_bytes(Vec::new())), request_handle: None, websocket_socket_id: (resource_type == SubresourceResourceType::WebSocket).then_some(7), frame_id: None, @@ -4286,8 +4286,9 @@ mod tests { success_output .response_body() .expect("success response should retain a body source") - .diagnostic_text(), - "api-body" + .diagnostic_bytes() + .as_ref(), + b"api-body" ); } @@ -4779,8 +4780,8 @@ mod tests { .expect("first subresource output should exist"); subresource_output.url = Url::parse("https://example.com/mutated.js").expect("test URL should parse"); - subresource_output.response_body = Some(SubresourceResponseBody::from_text( - "mutated-body".to_owned(), + subresource_output.response_body = Some(SubresourceResponseBody::from_bytes( + b"mutated-body".to_vec(), )); let TargetWebSocketDeliveryPlanRecord::Handshake(handshake) = output_queue .delivery_outputs @@ -4828,8 +4829,9 @@ mod tests { .metadata() .response_body() .expect("prepared success output should own its response body") - .diagnostic_text(), - "prepared-body", + .diagnostic_bytes() + .as_ref(), + b"prepared-body", "prepared subresource token must own prepare-time body source instead of rereading queue slots" ); diff --git a/moli-protocol/src/domains/network/tests/response_body.rs b/moli-protocol/src/domains/network/tests/response_body.rs index 282ddd2a8..6d33aa0d0 100644 --- a/moli-protocol/src/domains/network/tests/response_body.rs +++ b/moli-protocol/src/domains/network/tests/response_body.rs @@ -362,7 +362,7 @@ async fn get_response_body_returns_partial_body_after_staged_loading_failed() { let body = SubresourceBodyFinished::failed_with_partial_body( handle, "net::ERR_ABORTED".to_owned(), - SubresourceResponseBody::from_text("partial body".to_owned()), + SubresourceResponseBody::from_bytes(b"partial body".to_vec()), ); let items = vec![ ScriptNetworkOutputItem::SubresourceRequestStarted(Box::new(request)), diff --git a/moli-protocol/src/domains/page.rs b/moli-protocol/src/domains/page.rs index beb87b5ff..a3e48ba63 100644 --- a/moli-protocol/src/domains/page.rs +++ b/moli-protocol/src/domains/page.rs @@ -4114,10 +4114,9 @@ mod producer_tests { status: 200, response_headers: vec![("Content-Type".to_owned(), "text/html".to_owned())], encoded_data_length: 3, - response_body: Some(SubresourceResponseBody::from_text_and_bytes( - "\0\u{fffd}a".to_owned(), - vec![0x00, 0xff, b'a'], - )), + response_body: Some(SubresourceResponseBody::from_bytes(vec![ + 0x00, 0xff, b'a', + ])), from_cache: true, }), }], @@ -4251,8 +4250,8 @@ mod producer_tests { status: 200, response_headers: vec![("Content-Type".to_owned(), "text/html".to_owned())], encoded_data_length: 21, - response_body: Some(SubresourceResponseBody::from_text( - "historical child body".to_owned(), + response_body: Some(SubresourceResponseBody::from_bytes( + b"historical child body".to_vec(), )), from_cache: false, }, diff --git a/moli-renderer-v8/src/app_manifest.rs b/moli-renderer-v8/src/app_manifest.rs index bf19e4b1f..cc4215378 100644 --- a/moli-renderer-v8/src/app_manifest.rs +++ b/moli-renderer-v8/src/app_manifest.rs @@ -303,10 +303,7 @@ impl RendererPreparedAppManifestLoad { false, ) }; - let response_body = SubresourceResponseBody::from_text_and_bytes( - String::from_utf8_lossy(&response.body).into_owned(), - response.body, - ); + let response_body = SubresourceResponseBody::from_bytes(response.body); let record = SubresourceNetworkRecord::success_with_body( frame_id, document_url.clone(), diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs index 93ac98c57..46389fb7c 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs @@ -743,7 +743,10 @@ fn child_document_load_outcome_from_response( if child_document_response_should_ignore_navigation(head.status, &head.headers) { return Ok(ChildDocumentLoadOutcome::IgnoredNavigation); } - let response_body = SubresourceResponseBody::from_materialized_body(body); + let response_body = SubresourceResponseBody::from_bytes( + body.try_into_materialized_bytes() + .map_err(|_| "child document response body should remain materialized".to_owned())?, + ); let encoded_data_length = response_body.len(); let content_type = child_document_content_type_from_headers(&head.headers) .or_else(|| child_document_content_type_for_url(&head.final_url)); diff --git a/moli-renderer-v8/src/native_bridge/context_host/resource_loading.rs b/moli-renderer-v8/src/native_bridge/context_host/resource_loading.rs index 692d759ba..a47020801 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/resource_loading.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/resource_loading.rs @@ -28,6 +28,17 @@ enum ImageSubresourceFetchRegistration { Dispatched(moli_fetch::FetchCancelHandle), } +fn navigation_response_from_subresource_body( + body: &SubresourceResponseBody, + head: moli_fetch::ResponseHead, +) -> crate::types::NavigationResponse { + let bytes = body.materialize_bytes().unwrap_or_default(); + crate::types::NavigationResponse::from_head_and_materialized_body( + head, + moli_fetch::ResponseBody::materialized_bytes(bytes), + ) +} + impl JsContextHost { #[cfg(test)] pub(crate) fn has_pending_load_event_delaying_subresource_requests(&self) -> bool { @@ -2795,9 +2806,9 @@ impl JsContextHost { request_method: in_flight.request_method, request_headers: in_flight.request_headers, request_body: in_flight.request_body, - response: info - .response_body - .to_navigation_response(moli_fetch::ResponseHead { + response: navigation_response_from_subresource_body( + &info.response_body, + moli_fetch::ResponseHead { final_url: info.url.clone(), status: info.response_status, headers: info.response_headers.clone(), @@ -2807,8 +2818,9 @@ impl JsContextHost { redirect_chain: Vec::new(), from_cache: info.from_cache, negotiated_http_version: None, - }) - .with_network_request_headers(info.network_request_headers.clone()), + }, + ) + .with_network_request_headers(info.network_request_headers.clone()), }); self.record_pending_subresource_continue_event( PendingSubresourceContinueEvent::ResponsePaused(info), @@ -2838,9 +2850,9 @@ impl JsContextHost { request_body: in_flight.request_body, intercept_response: info.intercept_response, initial_network_request_headers: info.network_request_headers.clone(), - response: info - .response_body - .to_navigation_response(moli_fetch::ResponseHead { + response: navigation_response_from_subresource_body( + &info.response_body, + moli_fetch::ResponseHead { final_url: info.response_final_url.clone(), status: info.response_status, headers: info.response_headers.clone(), @@ -2850,8 +2862,9 @@ impl JsContextHost { redirect_chain: Vec::new(), from_cache: info.response_from_cache, negotiated_http_version: None, - }) - .with_network_request_headers(info.network_request_headers.clone()), + }, + ) + .with_network_request_headers(info.network_request_headers.clone()), }); self.record_pending_subresource_continue_event( PendingSubresourceContinueEvent::AuthRequired(info), diff --git a/moli-renderer-v8/src/native_bridge/context_host/websockets/registration.rs b/moli-renderer-v8/src/native_bridge/context_host/websockets/registration.rs index 20f9ff111..b7005eba8 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/websockets/registration.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/websockets/registration.rs @@ -255,7 +255,7 @@ impl JsContextHost { network_request_headers: None, response_status, response_headers, - response_body: SubresourceResponseBody::from_text(String::new()), + response_body: SubresourceResponseBody::from_bytes(Vec::new()), from_cache: false, }), ); diff --git a/moli-renderer-v8/src/network_host/body_source.rs b/moli-renderer-v8/src/network_host/body_source.rs index 2aac323e2..a80422e93 100644 --- a/moli-renderer-v8/src/network_host/body_source.rs +++ b/moli-renderer-v8/src/network_host/body_source.rs @@ -2057,10 +2057,7 @@ mod tests { #[test] fn network_body_source_subresource_reads_through_fallible_source() { - let body = SubresourceResponseBody::from_text_and_bytes( - "hello world".to_owned(), - b"hello world".to_vec(), - ); + let body = SubresourceResponseBody::from_bytes(b"hello world".to_vec()); let id = register_network_body_subresource_body(body); assert_eq!( diff --git a/moli-renderer-v8/src/network_host/preflight_events.rs b/moli-renderer-v8/src/network_host/preflight_events.rs index 1ea11020f..081ec6e9f 100644 --- a/moli-renderer-v8/src/network_host/preflight_events.rs +++ b/moli-renderer-v8/src/network_host/preflight_events.rs @@ -50,7 +50,7 @@ impl CorsPreflightNetworkObserver { response.final_url.clone(), response.status, response.headers.clone(), - SubresourceResponseBody::from_text(String::new()), + SubresourceResponseBody::from_bytes(Vec::new()), response.cookie_set_reports.clone(), ) .with_from_cache(response.from_cache) diff --git a/moli-renderer-v8/src/runtime/page_surface.rs b/moli-renderer-v8/src/runtime/page_surface.rs index fd364cc54..d0d1ee1a1 100644 --- a/moli-renderer-v8/src/runtime/page_surface.rs +++ b/moli-renderer-v8/src/runtime/page_surface.rs @@ -102,9 +102,7 @@ impl RendererSyntheticResponseBody { /// Converts a synthetic fulfill body into the renderer-neutral subresource /// record body without reopening a loose text/byte pair at each call site. pub fn into_subresource_response_body(self) -> crate::protocol_types::SubresourceResponseBody { - crate::protocol_types::SubresourceResponseBody::from_materialized_body( - self.into_response_body(), - ) + crate::protocol_types::SubresourceResponseBody::from_bytes(self.into_body_bytes()) } /// Builds a materialized fetch response for worker/Web compatibility paths diff --git a/moli-renderer-v8/src/script_vm/subresource_fetch.rs b/moli-renderer-v8/src/script_vm/subresource_fetch.rs index a7d4b5274..44ed87035 100644 --- a/moli-renderer-v8/src/script_vm/subresource_fetch.rs +++ b/moli-renderer-v8/src/script_vm/subresource_fetch.rs @@ -5539,14 +5539,14 @@ impl ScriptVm { ) { let materialize_started = moli_trace::cdp_runtime_trace_enabled().then(Instant::now); - match response_body.try_materialized_body() { - Ok(body) => { + match response_body.materialize_bytes() { + Ok(bytes) => { trace_async_subresource_stage( "async_subresource_streaming_xhr_body_materialized", trace_fields, materialize_started, ); - Some(body) + Some(moli_fetch::ResponseBody::materialized_bytes(bytes)) } Err(error) => { let error_text = format!( diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs index 21aac8770..4202df96a 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs @@ -63,7 +63,7 @@ fn service_worker_csp_report_seen( && matches!( body.result(), crate::types::SubresourceBodyFinishedResult::Ready(response_body) - if response_body.diagnostic_text() == expected_body + if response_body.diagnostic_bytes().as_ref() == expected_body.as_bytes() ) ) }) @@ -82,7 +82,7 @@ fn service_worker_csp_report_seen( response_body, .. } if final_url.as_str() == report_url - && response_body.diagnostic_text() == expected_body + && response_body.diagnostic_bytes().as_ref() == expected_body.as_bytes() ) ) }); @@ -12993,7 +12993,7 @@ async fn navigator_service_worker_intercepts_element_resource_destinations_once( status: 200, response_body, .. - } if response_body.diagnostic_text() == expected_body + } if response_body.diagnostic_bytes().as_ref() == expected_body.as_bytes() ) ) }) { @@ -13025,7 +13025,7 @@ async fn navigator_service_worker_intercepts_element_resource_destinations_once( && matches!( body.result(), crate::types::SubresourceBodyFinishedResult::Ready(response_body) - if response_body.diagnostic_text() == expected_body + if response_body.diagnostic_bytes().as_ref() == expected_body.as_bytes() ) ) }) @@ -13236,8 +13236,8 @@ async fn navigator_service_worker_intercepts_stylesheet_font_face_destination() response_body, .. } if final_url.as_str() == expected_font_url - && response_body.diagnostic_text() - == "stylesheet-font:font:/app/fonts/demo.woff2" + && response_body.diagnostic_bytes().as_ref() + == b"stylesheet-font:font:/app/fonts/demo.woff2" ) }) }); diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs index 0a078f56e..4ec1fd547 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs @@ -2316,7 +2316,7 @@ async fn streaming_fetch_body_error_records_response_started_then_body_failed() partial_body, } => { assert_eq!(error_text, crate::network_host::ABORTED_ERROR_TEXT); - assert_eq!(partial_body.diagnostic_text(), "partial"); + assert_eq!(partial_body.diagnostic_bytes().as_ref(), b"partial"); } other => panic!("expected failed body with partial payload, got {other:?}"), } @@ -2679,7 +2679,7 @@ async fn streaming_fetch_body_cancel_records_response_started_then_body_failed() partial_body, } => { assert_eq!(error_text, crate::network_host::ABORTED_ERROR_TEXT); - assert_eq!(partial_body.diagnostic_text(), "partial"); + assert_eq!(partial_body.diagnostic_bytes().as_ref(), b"partial"); } other => panic!("expected failed body with partial payload, got {other:?}"), } diff --git a/moli-renderer-v8/src/script_vm/tests/mod.rs b/moli-renderer-v8/src/script_vm/tests/mod.rs index 4435180f0..ca6b7f07d 100644 --- a/moli-renderer-v8/src/script_vm/tests/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/mod.rs @@ -2648,7 +2648,10 @@ fn cancel_pending_window_fetch_auth_preserves_401_for_response_stage() { }; assert_eq!(info.internal_id, internal_id); assert_eq!(info.response_status, 401); - assert_eq!(info.response_body.text(), "auth required"); + assert_eq!( + info.response_body.try_bytes().unwrap().as_ref(), + b"auth required" + ); let pending = vm ._context_host diff --git a/moli-renderer-v8/src/worker/global_scope/fetch.rs b/moli-renderer-v8/src/worker/global_scope/fetch.rs index 2aebf9684..2d90ffaa4 100644 --- a/moli-renderer-v8/src/worker/global_scope/fetch.rs +++ b/moli-renderer-v8/src/worker/global_scope/fetch.rs @@ -2914,7 +2914,7 @@ pub(in crate::worker) fn drain_worker_fetch_completion_result( WorkerFetchResponseParts::Subresource { mut head, body } => { head.headers = filtered_headers; let body = if opaque_response_blocked { - SubresourceResponseBody::from_text(String::new()) + SubresourceResponseBody::from_bytes(Vec::new()) } else { body }; diff --git a/moli-renderer-v8/src/worker/global_scope/mod.rs b/moli-renderer-v8/src/worker/global_scope/mod.rs index dc76e7cad..c40c01611 100644 --- a/moli-renderer-v8/src/worker/global_scope/mod.rs +++ b/moli-renderer-v8/src/worker/global_scope/mod.rs @@ -1266,8 +1266,8 @@ impl WorkerXhrResponse { match self { Self::Materialized(response) => Ok(response.into_body()), Self::Streamed { head, body } => body - .try_materialized_body() - .map(|body| (*head, body)) + .materialize_bytes() + .map(|bytes| (*head, ResponseBody::materialized_bytes(bytes))) .map_err(|error| format!("failed to materialize worker XHR body: {error}")), } } diff --git a/moli-renderer-v8/src/worker/thread/tests/network.rs b/moli-renderer-v8/src/worker/thread/tests/network.rs index 7be4e3b20..bca868347 100644 --- a/moli-renderer-v8/src/worker/thread/tests/network.rs +++ b/moli-renderer-v8/src/worker/thread/tests/network.rs @@ -3081,7 +3081,10 @@ async fn worker_xhr_response_stage_interception_pauses_before_done() { assert_eq!(info.internal_id, 37); assert_eq!(info.resource_type, SubresourceResourceType::Xhr); assert_eq!(info.response_status, 200); - assert_eq!(info.response_body.text().as_ref(), "origin-worker-xhr"); + assert_eq!( + info.response_body.try_bytes().unwrap().as_ref(), + b"origin-worker-xhr" + ); handle.continue_pending_xhr_response( request, @@ -4207,7 +4210,10 @@ async fn worker_fetch_response_stage_interception_pauses_before_resolving_respon }; assert_eq!(info.internal_id, 23); assert_eq!(info.response_status, 200); - assert_eq!(info.response_body.text().as_ref(), "origin-worker-body"); + assert_eq!( + info.response_body.try_bytes().unwrap().as_ref(), + b"origin-worker-body" + ); handle.continue_pending_fetch_response( request, diff --git a/moli/src/network_trace.rs b/moli/src/network_trace.rs index 5527833d8..67a06e8f3 100644 --- a/moli/src/network_trace.rs +++ b/moli/src/network_trace.rs @@ -375,7 +375,8 @@ fn render_subresource_network_record_with_body_option( payload.insert("body_length".to_owned(), json!(response_body.len())); // Trace diagnostics are textual hints. Keep exact protocol bytes in // the subresource carrier and derive the lossy view only here. - let response_body_text = response_body.diagnostic_text(); + let response_body_bytes = response_body.diagnostic_bytes(); + let response_body_text = String::from_utf8_lossy(response_body_bytes.as_ref()); if include_body_text { payload.insert("body_text".to_owned(), json!(response_body_text.as_ref())); }