From a27715a5a88f582b92b4c4487d6e9826e4565a10 Mon Sep 17 00:00:00 2001 From: Usman Yahaya Baba <91813795+uthmaniv@users.noreply.github.com> Date: Sat, 2 Aug 2025 11:41:53 +0900 Subject: [PATCH] Calculate and send the missing transferred_size and content_size to dev tools (#38216) The current behaviour in dev tools network monitor is missing data for the `Transferred` size and `Content` size. We currently have a fn `response_content` that constructs the `ResponseContentMsg` struct where the two missing data fields are defined and set to zero values. These current changes calculates the data in the `response_content` fn and sends a `NetworkEvent::HttpResponse` to the client when the final body is done. Currently, we have data appearing in the `Transferred` column of the network panel fixes: https://github.com/servo/servo/issues/38126 --------- Signed-off-by: uthmaniv --- components/devtools/actors/network_event.rs | 25 ++++++--- components/net/fetch/methods.rs | 21 +++++-- components/net/http_loader.rs | 62 ++++++++++++++++----- components/net/tests/fetch.rs | 6 +- components/net/tests/http_loader.rs | 17 +++++- 5 files changed, 98 insertions(+), 33 deletions(-) diff --git a/components/devtools/actors/network_event.rs b/components/devtools/actors/network_event.rs index 9f6e9ed7d90..79785aa7eee 100644 --- a/components/devtools/actors/network_event.rs +++ b/components/devtools/actors/network_event.rs @@ -9,7 +9,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use chrono::{Local, LocalResult, TimeZone}; use devtools_traits::{HttpRequest as DevtoolsHttpRequest, HttpResponse as DevtoolsHttpResponse}; -use headers::{ContentType, Cookie, HeaderMapExt}; +use headers::{ContentLength, ContentType, Cookie, HeaderMapExt}; use http::{HeaderMap, Method, header}; use net_traits::request::Destination as RequestDestination; use serde::Serialize; @@ -389,7 +389,7 @@ impl NetworkEventActor { self.response_headers = Some(Self::response_headers(&response)); self.response_cookies = Some(Self::response_cookies(&response)); self.response_start = Some(Self::response_start(&response)); - self.response_content = Some(Self::response_content(&response)); + self.response_content = Self::response_content(&response); self.response_body = response.body.clone(); self.response_headers_raw = response.headers.clone(); } @@ -441,7 +441,9 @@ impl NetworkEventActor { } } - pub fn response_content(response: &DevtoolsHttpResponse) -> ResponseContentMsg { + pub fn response_content(response: &DevtoolsHttpResponse) -> Option { + let _body = response.body.as_ref()?; + let mime_type = response .headers .as_ref() @@ -449,13 +451,20 @@ impl NetworkEventActor { .map(|ct| ct.to_string()) .unwrap_or_default(); - // TODO: Set correct values when response's body is sent to the devtools in http_loader. - ResponseContentMsg { + let transferred_size = response + .headers + .as_ref() + .and_then(|hdrs| hdrs.typed_get::()) + .map(|cl| cl.0); + + let content_size = response.body.as_ref().map(|body| body.len() as u64); + + Some(ResponseContentMsg { mime_type, - content_size: 0, - transferred_size: 0, + content_size: content_size.unwrap_or(0) as u32, + transferred_size: transferred_size.unwrap_or(0) as u32, discard_response_body: true, - } + }) } pub fn response_cookies(response: &DevtoolsHttpResponse) -> ResponseCookiesMsg { diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 69e2a0d87c7..0f270695f1e 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -662,7 +662,7 @@ pub async fn main_fetch( let mut response_loaded = false; let mut response = if !response.is_network_error() && !request.integrity_metadata.is_empty() { // Step 19.1. - wait_for_response(request, &mut response, target, done_chan).await; + wait_for_response(request, &mut response, target, done_chan, context).await; response_loaded = true; // Step 19.2. @@ -686,7 +686,7 @@ pub async fn main_fetch( // by sync fetch, but we overload it here for simplicity target.process_response(request, &response); if !response_loaded { - wait_for_response(request, &mut response, target, done_chan).await; + wait_for_response(request, &mut response, target, done_chan, context).await; } // overloaded similarly to process_response target.process_response_eof(request, &response); @@ -706,11 +706,11 @@ pub async fn main_fetch( // Step 22. target.process_response(request, &response); // Send Response to Devtools - send_response_to_devtools(request, context, &response); + send_response_to_devtools(request, context, &response, None); // Step 23. if !response_loaded { - wait_for_response(request, &mut response, target, done_chan).await; + wait_for_response(request, &mut response, target, done_chan, context).await; } // Step 24. @@ -718,7 +718,7 @@ pub async fn main_fetch( // Send Response to Devtools // This is done after process_response_eof to ensure that the body is fully // processed before sending the response to Devtools. - send_response_to_devtools(request, context, &response); + send_response_to_devtools(request, context, &response, None); if let Ok(http_cache) = context.state.http_cache.write() { http_cache.update_awaiting_consumers(request, &response); @@ -734,14 +734,20 @@ async fn wait_for_response( response: &mut Response, target: Target<'_>, done_chan: &mut DoneChannel, + context: &FetchContext, ) { if let Some(ref mut ch) = *done_chan { + let mut devtools_body = context.devtools_chan.as_ref().map(|_| Vec::new()); loop { match ch.1.recv().await { Some(Data::Payload(vec)) => { + if let Some(body) = devtools_body.as_mut() { + body.extend(&vec); + } target.process_response_chunk(request, vec); }, Some(Data::Done) => { + send_response_to_devtools(request, context, response, devtools_body); break; }, Some(Data::Cancelled) => { @@ -760,6 +766,11 @@ async fn wait_for_response( // obtained synchronously via scheme_fetch for data/file/about/etc // We should still send the body across as a chunk target.process_response_chunk(request, vec.clone()); + if context.devtools_chan.is_some() { + // Now that we've replayed the entire cached body, + // notify the DevTools server with the full Response. + send_response_to_devtools(request, context, response, Some(vec.clone())); + } } else { assert_eq!(*body, ResponseBody::Empty) } diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 8022cd994aa..56d709e07ca 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -433,27 +433,48 @@ pub fn send_request_to_devtools( .unwrap(); } -pub fn send_response_to_devtools(request: &Request, context: &FetchContext, response: &Response) { +pub fn send_response_to_devtools( + request: &Request, + context: &FetchContext, + response: &Response, + body_data: Option>, +) { + let meta = match response.metadata() { + Ok(FetchMetadata::Unfiltered(m)) => m, + Ok(FetchMetadata::Filtered { unsafe_, .. }) => unsafe_, + Err(_) => { + log::warn!("No metadata available, skipping devtools response."); + return; + }, + }; + send_response_values_to_devtools( + meta.headers.map(Serde::into_inner), + meta.status, + body_data, + request, + context.devtools_chan.clone(), + ); +} + +#[allow(clippy::too_many_arguments)] +pub fn send_response_values_to_devtools( + headers: Option, + status: HttpStatus, + body: Option>, + request: &Request, + devtools_chan: Option>>>, +) { if let (Some(devtools_chan), Some(pipeline_id), Some(webview_id)) = ( - context.devtools_chan.as_ref(), + devtools_chan, request.pipeline_id, request.target_webview_id, ) { let browsing_context_id = webview_id.0; - let meta = match response - .metadata() - .expect("Response metadata should exist at this stage") - { - FetchMetadata::Unfiltered(m) => m, - FetchMetadata::Filtered { unsafe_, .. } => unsafe_, - }; - let status = meta.status; - let headers = meta.headers.map(Serde::into_inner); let devtoolsresponse = DevtoolsHttpResponse { headers, status, - body: None, + body, pipeline_id, browsing_context_id, }; @@ -935,8 +956,6 @@ pub async fn http_fetch( .try_code() .is_some_and(is_redirect_status) { - // Notify devtools before handling redirect - send_response_to_devtools(request, context, &response); // Substep 1. if response.actual_response().status != StatusCode::SEE_OTHER { // TODO: send RST_STREAM frame @@ -2063,9 +2082,14 @@ async fn http_network_fetch( let done_sender3 = done_sender.clone(); let timing_ptr2 = context.timing.clone(); let timing_ptr3 = context.timing.clone(); - let url1 = request.url(); + let devtools_request = request.clone(); + let url1 = devtools_request.url(); let url2 = url1.clone(); + let status = response.status.clone(); + let headers = response.headers.clone(); + let devtools_chan = context.devtools_chan.clone(); + HANDLE.spawn( res.into_body() .map_err(|e| { @@ -2091,7 +2115,15 @@ async fn http_network_fetch( ResponseBody::Receiving(ref mut body) => std::mem::take(body), _ => vec![], }; + let devtools_response_body = completed_body.clone(); *body = ResponseBody::Done(completed_body); + send_response_values_to_devtools( + Some(headers), + status, + Some(devtools_response_body), + &devtools_request, + devtools_chan, + ); timing_ptr2 .lock() .unwrap() diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index df74855f3b5..2bf7768deea 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -47,7 +47,7 @@ use servo_arc::Arc as ServoArc; use servo_url::ServoUrl; use uuid::Uuid; -use crate::http_loader::{expect_devtools_http_request, expect_devtools_http_response}; +use crate::http_loader::{devtools_response_with_body, expect_devtools_http_request}; use crate::{ DEFAULT_USER_AGENT, create_embedder_proxy, create_embedder_proxy_and_receiver, create_http_state, fetch, fetch_with_context, fetch_with_cors_cache, make_body, make_server, @@ -1296,7 +1296,7 @@ fn test_fetch_with_devtools() { // notification received from devtools let devhttprequests = expect_devtools_http_request(&devtools_port); - let mut devhttpresponse = expect_devtools_http_response(&devtools_port); + let mut devhttpresponse = devtools_response_with_body(&devtools_port); //Creating default headers for request let mut headers = HeaderMap::new(); @@ -1356,7 +1356,7 @@ fn test_fetch_with_devtools() { let httpresponse = DevtoolsHttpResponse { headers: Some(response_headers), status: HttpStatus::default(), - body: None, + body: Some(content.as_bytes().to_vec()), pipeline_id: TEST_PIPELINE_ID, browsing_context_id: TEST_WEBVIEW_ID.0, }; diff --git a/components/net/tests/http_loader.rs b/components/net/tests/http_loader.rs index 0ac7f31e876..76ab3b7197c 100644 --- a/components/net/tests/http_loader.rs +++ b/components/net/tests/http_loader.rs @@ -159,6 +159,19 @@ pub fn expect_devtools_http_response( } } +pub fn devtools_response_with_body( + devtools_port: &Receiver, +) -> DevtoolsHttpResponse { + let devhttpresponses = vec![ + expect_devtools_http_response(devtools_port), + expect_devtools_http_response(devtools_port), + ]; + return devhttpresponses + .into_iter() + .find(|resp| resp.body.is_some()) + .expect("One of the responses should have a body"); +} + #[test] fn test_check_default_headers_loaded_in_every_request() { let expected_headers = Arc::new(Mutex::new(None)); @@ -337,7 +350,7 @@ fn test_request_and_response_data_with_network_messages() { // notification received from devtools let devhttprequests = expect_devtools_http_request(&devtools_port); - let devhttpresponse = expect_devtools_http_response(&devtools_port); + let devhttpresponse = devtools_response_with_body(&devtools_port); //Creating default headers for request let mut headers = HeaderMap::new(); @@ -409,7 +422,7 @@ fn test_request_and_response_data_with_network_messages() { let httpresponse = DevtoolsHttpResponse { headers: Some(response_headers), status: HttpStatus::default(), - body: None, + body: Some(content.as_bytes().to_vec()), pipeline_id: TEST_PIPELINE_ID, browsing_context_id: TEST_WEBVIEW_ID.0, };