Send early DevToolsHttpRequest and relocate response reporting to main_fetch (#37906)

This change refactors how we notify DevTools about network activity so
that all fetches (even those served from cache) appear correctly in the
Network panel, and so that DevTools sees request metadata as soon as
possible rather than waiting until the end of a full HTTP cycle.
- Before, we only send DevTools events inside http_network_fetch, so
cached responses (which skip that path) never show up. By emitting a
minimal HttpRequest event at the very start of main_fetch (with URL,
method, pipeline and browsing IDs), we guarantee every fetch shows up
immediately.
- Then, by moving HttpResponse notifications out of http_network_fetch
into main_fetch (right after process_response and process_response_eof),
we ensure DevTools gets status, header, and completion events for both
network and cache hits. Leveraging nullable fields in NetworkEventActor
lets us incrementally fill in timing, header, and body data later,
improving DevTools’ visibility.
Testing: Ran servo with `--devtools=6080` flag, cached responses now
appear in the network panel
Fixes: https://github.com/servo/servo/issues/37869

---------

Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>

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,
         };
 

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::<WatcherActor>(&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);

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);

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<DevtoolsControlMsg>,
 ) {
@@ -431,25 +431,74 @@ fn send_request_to_devtools(
         .unwrap();
 }
 
-fn send_response_to_devtools(
-    devtools_chan: &Sender<DevtoolsControlMsg>,
-    request_id: String,
-    headers: Option<HeaderMap>,
-    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 {
+    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 devtoolsresponse = DevtoolsHttpResponse {
             headers,
             status,
             body: None,
             pipeline_id,
             browsing_context_id,
         };
-    let net_event_response = NetworkEvent::HttpResponse(response);
+        let net_event_response = NetworkEvent::HttpResponse(devtoolsresponse);
 
-    let msg = ChromeToDevtoolsControlMsg::NetworkEvent(request_id, net_event_response);
-    let _ = devtools_chan.send(DevtoolsControlMsg::FromChrome(msg));
+        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();

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);
 }
 

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<DevtoolsControlMsg>,
-) -> DevtoolsHttpRequest {
+fn recv_http_request(devtools_port: &Receiver<DevtoolsControlMsg>) -> 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<DevtoolsControlMsg>,
+) -> (DevtoolsHttpRequest, DevtoolsHttpRequest) {
+    (
+        recv_http_request(devtools_port),
+        recv_http_request(devtools_port),
+    )
+}
+
 pub fn expect_devtools_http_response(
     devtools_port: &Receiver<DevtoolsControlMsg>,
 ) -> 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());
 }
 

components/shared/devtools/lib.rs

@@ -459,6 +459,7 @@ pub struct HttpResponse {
 #[derive(Debug)]
 pub enum NetworkEvent {
     HttpRequest(HttpRequest),
+    HttpRequestUpdate(HttpRequest),
     HttpResponse(HttpResponse),
 }
 

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 {