diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index 02ca7af703e..0e2d4c4a027 100644 --- a/components/devtools/lib.rs +++ b/components/devtools/lib.rs @@ -493,6 +493,7 @@ impl DevtoolsInstance { ) { let browsing_context_id = match &network_event { NetworkEvent::HttpRequest(req) => req.browsing_context_id, + NetworkEvent::HttpRequestUpdate(req) => req.browsing_context_id, NetworkEvent::HttpResponse(resp) => resp.browsing_context_id, }; diff --git a/components/devtools/network_handler.rs b/components/devtools/network_handler.rs index 3aa535a07a6..fd2dace4d9d 100644 --- a/components/devtools/network_handler.rs +++ b/components/devtools/network_handler.rs @@ -55,6 +55,21 @@ pub(crate) fn handle_network_event( ); } }, + + NetworkEvent::HttpRequestUpdate(httprequest) => { + actor.add_request(httprequest); + let resource = actor.resource_updates(); + let watcher_actor = actors.find::(&watcher_name); + + for stream in &mut connections { + watcher_actor.resource_array( + resource.clone(), + "network-event".to_string(), + ResourceArrayType::Updated, + stream, + ); + } + }, NetworkEvent::HttpResponse(httpresponse) => { // Store the response information in the actor actor.add_response(httpresponse); diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index e99776be350..69e2a0d87c7 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -42,7 +42,10 @@ use super::fetch_params::FetchParams; use crate::fetch::cors_cache::CorsCache; use crate::fetch::headers::determine_nosniff; use crate::filemanager_thread::FileManager; -use crate::http_loader::{HttpState, determine_requests_referrer, http_fetch, set_default_accept}; +use crate::http_loader::{ + HttpState, determine_requests_referrer, http_fetch, send_early_httprequest_to_devtools, + send_response_to_devtools, set_default_accept, +}; use crate::protocols::{ProtocolRegistry, is_url_potentially_trustworthy}; use crate::request_interceptor::RequestInterceptor; use crate::subresource_integrity::is_response_integrity_valid; @@ -257,7 +260,8 @@ pub async fn main_fetch( ) -> Response { // Step 1: Let request be fetchParam's request. let request = &mut fetch_params.request; - + // send early HTTP request to DevTools + send_early_httprequest_to_devtools(request, context); // Step 2: Let response be null. let mut response = None; @@ -701,6 +705,8 @@ pub async fn main_fetch( // Step 22. target.process_response(request, &response); + // Send Response to Devtools + send_response_to_devtools(request, context, &response); // Step 23. if !response_loaded { @@ -709,6 +715,10 @@ pub async fn main_fetch( // Step 24. target.process_response_eof(request, &response); + // 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); if let Ok(http_cache) = context.state.http_cache.write() { http_cache.update_awaiting_consumers(request, &response); diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 906c338c9c9..baef79cacce 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -417,12 +417,12 @@ fn prepare_devtools_request( is_xhr, browsing_context_id, }; - let net_event = NetworkEvent::HttpRequest(request); + let net_event = NetworkEvent::HttpRequestUpdate(request); ChromeToDevtoolsControlMsg::NetworkEvent(request_id, net_event) } -fn send_request_to_devtools( +pub fn send_request_to_devtools( msg: ChromeToDevtoolsControlMsg, devtools_chan: &Sender, ) { @@ -431,25 +431,74 @@ fn send_request_to_devtools( .unwrap(); } -fn send_response_to_devtools( - devtools_chan: &Sender, - request_id: String, - headers: Option, - status: HttpStatus, - pipeline_id: PipelineId, - browsing_context_id: BrowsingContextId, +pub fn send_response_to_devtools( + request: &mut Request, + context: &FetchContext, + response: &Response, ) { - let response = DevtoolsHttpResponse { - headers, - status, - body: None, - pipeline_id, - browsing_context_id, - }; - let net_event_response = NetworkEvent::HttpResponse(response); + if let (Some(devtools_chan), Some(pipeline_id), Some(webview_id)) = ( + context.devtools_chan.as_ref(), + 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 msg = ChromeToDevtoolsControlMsg::NetworkEvent(request_id, net_event_response); - let _ = devtools_chan.send(DevtoolsControlMsg::FromChrome(msg)); + let devtoolsresponse = DevtoolsHttpResponse { + headers, + status, + body: None, + pipeline_id, + browsing_context_id, + }; + let net_event_response = NetworkEvent::HttpResponse(devtoolsresponse); + + let msg = + ChromeToDevtoolsControlMsg::NetworkEvent(request.id.0.to_string(), net_event_response); + + let _ = devtools_chan + .lock() + .unwrap() + .send(DevtoolsControlMsg::FromChrome(msg)); + } +} + +pub fn send_early_httprequest_to_devtools(request: &mut Request, context: &FetchContext) { + if let (Some(devtools_chan), Some(browsing_context_id), Some(pipeline_id)) = ( + context.devtools_chan.as_ref(), + request.target_webview_id.map(|id| id.0), + request.pipeline_id, + ) { + // Build the partial DevtoolsHttpRequest + let devtools_request = DevtoolsHttpRequest { + url: request.current_url().clone(), + method: request.method.clone(), + headers: request.headers.clone(), + body: None, + pipeline_id, + started_date_time: SystemTime::now(), + time_stamp: 0, + connect_time: Duration::from_millis(0), + send_time: Duration::from_millis(0), + is_xhr: false, + browsing_context_id, + }; + + let msg = ChromeToDevtoolsControlMsg::NetworkEvent( + request.id.0.to_string(), + NetworkEvent::HttpRequest(devtools_request), + ); + + send_request_to_devtools(msg, &devtools_chan.lock().unwrap()); + } } fn auth_from_cache( @@ -885,6 +934,8 @@ 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 @@ -1867,12 +1918,7 @@ async fn http_network_fetch( // Step 5 let url = request.current_url(); - - let request_id = context - .devtools_chan - .as_ref() - .map(|_| uuid::Uuid::new_v4().simple().to_string()); - + let request_id = request.id.0.to_string(); if log_enabled!(log::Level::Info) { info!("{:?} request for {}", request.method, url); for header in request.headers.iter() { @@ -1912,14 +1958,13 @@ async fn http_network_fetch( .map(|body| body.source_is_null()) .unwrap_or(false), &request.pipeline_id, - request_id.as_deref(), + Some(&request_id), is_xhr, context, fetch_terminated_sender, browsing_context_id, ); - let pipeline_id = request.pipeline_id; // This will only get the headers, the body is read later let (res, msg) = match response_future.await { Ok(wrapped_response) => wrapped_response, @@ -1995,17 +2040,8 @@ async fn http_network_fetch( // We're about to spawn a future to be waited on here let (done_sender, done_receiver) = unbounded_channel(); *done_chan = Some((done_sender.clone(), done_receiver)); - let meta = match response - .metadata() - .expect("Response metadata should exist at this stage") - { - FetchMetadata::Unfiltered(m) => m, - FetchMetadata::Filtered { unsafe_, .. } => unsafe_, - }; let devtools_sender = context.devtools_chan.clone(); - let meta_status = meta.status; - let meta_headers = meta.headers; let cancellation_listener = context.cancellation_listener.clone(); if cancellation_listener.cancelled() { return Response::network_error(NetworkError::Internal("Fetch aborted".into())); @@ -2019,19 +2055,6 @@ async fn http_network_fetch( if let Some(m) = msg { send_request_to_devtools(m, &sender); } - - // --- Tell devtools that we got a response - // Send an HttpResponse message to devtools with the corresponding request_id - if let (Some(pipeline_id), Some(browsing_context_id)) = (pipeline_id, browsing_context_id) { - send_response_to_devtools( - &sender, - request_id.unwrap(), - meta_headers.map(Serde::into_inner), - meta_status, - pipeline_id, - browsing_context_id, - ); - } } let done_sender2 = done_sender.clone(); diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index a17f1f9d35c..26df87d999b 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -1295,7 +1295,7 @@ fn test_fetch_with_devtools() { let _ = server.close(); // notification received from devtools - let devhttprequest = expect_devtools_http_request(&devtools_port); + let devhttprequests = expect_devtools_http_request(&devtools_port); let mut devhttpresponse = expect_devtools_http_response(&devtools_port); //Creating default headers for request @@ -1335,10 +1335,10 @@ fn test_fetch_with_devtools() { headers: headers, body: Some(vec![]), pipeline_id: TEST_PIPELINE_ID, - started_date_time: devhttprequest.started_date_time, - time_stamp: devhttprequest.time_stamp, - connect_time: devhttprequest.connect_time, - send_time: devhttprequest.send_time, + started_date_time: devhttprequests.1.started_date_time, + time_stamp: devhttprequests.1.time_stamp, + connect_time: devhttprequests.1.connect_time, + send_time: devhttprequests.1.send_time, is_xhr: true, browsing_context_id: TEST_WEBVIEW_ID.0, }; @@ -1360,7 +1360,7 @@ fn test_fetch_with_devtools() { browsing_context_id: TEST_WEBVIEW_ID.0, }; - assert_eq!(devhttprequest, httprequest); + assert_eq!(devhttprequests.1, httprequest); assert_eq!(devhttpresponse, httpresponse); } diff --git a/components/net/tests/http_loader.rs b/components/net/tests/http_loader.rs index 9c9abd05970..23896823524 100644 --- a/components/net/tests/http_loader.rs +++ b/components/net/tests/http_loader.rs @@ -67,21 +67,28 @@ fn assert_cookie_for_domain( assert_eq!(cookies.as_ref().map(|c| &**c), cookie); } -pub fn expect_devtools_http_request( - devtools_port: &Receiver, -) -> DevtoolsHttpRequest { +fn recv_http_request(devtools_port: &Receiver) -> DevtoolsHttpRequest { match devtools_port.recv().unwrap() { DevtoolsControlMsg::FromChrome(ChromeToDevtoolsControlMsg::NetworkEvent(_, net_event)) => { match net_event { - NetworkEvent::HttpRequest(httprequest) => httprequest, - - _ => panic!("No HttpRequest Received"), + NetworkEvent::HttpRequest(req) => req, + NetworkEvent::HttpRequestUpdate(req) => req, + other => panic!("Expected HttpRequest but got: {:?}", other), } }, - _ => panic!("No HttpRequest Received"), + other => panic!("Expected NetworkEvent but got: {:?}", other), } } +pub fn expect_devtools_http_request( + devtools_port: &Receiver, +) -> (DevtoolsHttpRequest, DevtoolsHttpRequest) { + ( + recv_http_request(devtools_port), + recv_http_request(devtools_port), + ) +} + pub fn expect_devtools_http_response( devtools_port: &Receiver, ) -> DevtoolsHttpResponse { @@ -92,9 +99,9 @@ pub fn expect_devtools_http_response( )) => match net_event_response { NetworkEvent::HttpResponse(httpresponse) => httpresponse, - _ => panic!("No HttpResponse Received"), + other => panic!("Expected HttpResponse but got: {:?}", other), }, - _ => panic!("No HttpResponse Received"), + other => panic!("Expected NetworkEvent but got: {:?}", other), } } @@ -275,7 +282,7 @@ fn test_request_and_response_data_with_network_messages() { let _ = server.close(); // notification received from devtools - let devhttprequest = expect_devtools_http_request(&devtools_port); + let devhttprequests = expect_devtools_http_request(&devtools_port); let devhttpresponse = expect_devtools_http_response(&devtools_port); //Creating default headers for request @@ -322,10 +329,10 @@ fn test_request_and_response_data_with_network_messages() { headers: headers, body: Some(vec![]), pipeline_id: TEST_PIPELINE_ID, - started_date_time: devhttprequest.started_date_time, - time_stamp: devhttprequest.time_stamp, - connect_time: devhttprequest.connect_time, - send_time: devhttprequest.send_time, + started_date_time: devhttprequests.1.started_date_time, + time_stamp: devhttprequests.1.time_stamp, + connect_time: devhttprequests.1.connect_time, + send_time: devhttprequests.1.send_time, is_xhr: false, browsing_context_id: TEST_WEBVIEW_ID.0, }; @@ -352,7 +359,7 @@ fn test_request_and_response_data_with_network_messages() { browsing_context_id: TEST_WEBVIEW_ID.0, }; - assert_eq!(devhttprequest, httprequest); + assert_eq!(devhttprequests.1, httprequest); assert_eq!(devhttpresponse, httpresponse); } @@ -421,21 +428,21 @@ fn test_redirected_request_to_devtools() { let _ = pre_server.close(); let _ = post_server.close(); - let devhttprequest = expect_devtools_http_request(&devtools_port); + let devhttprequests = expect_devtools_http_request(&devtools_port); let devhttpresponse = expect_devtools_http_response(&devtools_port); - assert_eq!(devhttprequest.method, Method::POST); - assert_eq!(devhttprequest.url, pre_url); + assert_eq!(devhttprequests.0.method, Method::POST); + assert_eq!(devhttprequests.0.url, pre_url); assert_eq!( devhttpresponse.status, HttpStatus::from(StatusCode::MOVED_PERMANENTLY) ); - let devhttprequest = expect_devtools_http_request(&devtools_port); + let devhttprequests = expect_devtools_http_request(&devtools_port); let devhttpresponse = expect_devtools_http_response(&devtools_port); - assert_eq!(devhttprequest.method, Method::GET); - assert_eq!(devhttprequest.url, post_url); + assert_eq!(devhttprequests.0.method, Method::GET); + assert_eq!(devhttprequests.0.url, post_url); assert_eq!(devhttpresponse.status, HttpStatus::default()); } diff --git a/components/shared/devtools/lib.rs b/components/shared/devtools/lib.rs index f4c468b1d5d..6d4ab0f8595 100644 --- a/components/shared/devtools/lib.rs +++ b/components/shared/devtools/lib.rs @@ -459,6 +459,7 @@ pub struct HttpResponse { #[derive(Debug)] pub enum NetworkEvent { HttpRequest(HttpRequest), + HttpRequestUpdate(HttpRequest), HttpResponse(HttpResponse), } diff --git a/components/shared/net/request.rs b/components/shared/net/request.rs index 7b39ac7b78d..12d960357b4 100644 --- a/components/shared/net/request.rs +++ b/components/shared/net/request.rs @@ -22,7 +22,7 @@ use crate::{ReferrerPolicy, ResourceTimingType}; #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)] /// An id to differeniate one network request from another. -pub struct RequestId(Uuid); +pub struct RequestId(pub Uuid); impl Default for RequestId { fn default() -> Self {