Remove duplication between request/response properties in NetworkEventActor (#37651)

- Remove request/response fields in `NetworkEventActor` which now stores
minimal request metadata needed : `URL`, `method`, `timestamps`, `raw
headers`, `body`.
- Refactor add_request and add_response: `add_request` takes the
incoming DevtoolsHttpRequest and populates
`request_url`, `request_method`, `request_started`,
`request_time_stamp`, `request_body`, `request_headers_raw`
`request_cookies`, `request_headers`,`total_time` and `event_timing`
(via a new helper that computes Timings)
While `add_response` takes the incoming DevtoolsHttpResponse and
populates `response_headers_raw`, `response_body` ,`response_cookies`,
`response_headers`, `response_start`, `response_content`
- Add a call to `resources_updated` to push initial request info in
`handle_network_event`
Testing: Run and logged servo in devtools mode and now, the request
header info isavailable as soon as the request gets initiated
Fixes: https://github.com/servo/servo/issues/37566

---------

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

components/devtools/actors/network_event.rs

@@ -12,7 +12,6 @@ use chrono::{Local, LocalResult, TimeZone};
 use devtools_traits::{HttpRequest as DevtoolsHttpRequest, HttpResponse as DevtoolsHttpResponse};
 use headers::{ContentType, Cookie, HeaderMapExt};
 use http::{HeaderMap, Method, header};
-use net_traits::http_status::HttpStatus;
 use serde::Serialize;
 use serde_json::{Map, Value};
 
@@ -21,36 +20,23 @@ use crate::actor::{Actor, ActorMessageStatus, ActorRegistry};
 use crate::network_handler::Cause;
 use crate::protocol::JsonPacketStream;
 
-#[derive(Clone)]
-pub struct HttpRequest {
-    url: String,
-    method: Method,
-    headers: HeaderMap,
-    body: Option<Vec<u8>>,
-    started_date_time: SystemTime,
-    time_stamp: i64,
-    connect_time: Duration,
-    send_time: Duration,
-}
-
-#[derive(Clone)]
-pub struct HttpResponse {
-    headers: Option<HeaderMap>,
-    status: HttpStatus,
-    body: Option<Vec<u8>>,
-}
-
 pub struct NetworkEventActor {
     pub name: String,
-    pub request: HttpRequest,
-    pub response: HttpResponse,
     pub is_xhr: bool,
+    pub request_url: String,
+    pub request_method: Method,
+    pub request_started: SystemTime,
+    pub request_time_stamp: i64,
+    pub request_headers_raw: Option<HeaderMap>,
+    pub request_body: Option<Vec<u8>>,
+    pub request_cookies: Option<RequestCookiesMsg>,
+    pub request_headers: Option<RequestHeadersMsg>,
+    pub response_headers_raw: Option<HeaderMap>,
+    pub response_body: Option<Vec<u8>>,
     pub response_content: Option<ResponseContentMsg>,
     pub response_start: Option<ResponseStartMsg>,
     pub response_cookies: Option<ResponseCookiesMsg>,
     pub response_headers: Option<ResponseHeadersMsg>,
-    pub request_cookies: Option<RequestCookiesMsg>,
-    pub request_headers: Option<RequestHeadersMsg>,
     pub total_time: Duration,
     pub security_state: String,
     pub event_timing: Option<Timings>,
@@ -176,7 +162,7 @@ struct GetResponseCookiesReply {
     cookies: Vec<u8>,
 }
 
-#[derive(Serialize)]
+#[derive(Clone, Default, Serialize)]
 pub struct Timings {
     blocked: u32,
     dns: u32,
@@ -224,15 +210,19 @@ impl Actor for NetworkEventActor {
                 let mut headers = Vec::new();
                 let mut raw_headers_string = "".to_owned();
                 let mut headers_size = 0;
-                for (name, value) in self.request.headers.iter() {
+                if let Some(ref headers_map) = self.request_headers_raw {
-                    let value = &value.to_str().unwrap().to_string();
+                    for (name, value) in headers_map.iter() {
-                    raw_headers_string = raw_headers_string + name.as_str() + ":" + value + "\r\n";
+                        let value = &value.to_str().unwrap().to_string();
-                    headers_size += name.as_str().len() + value.len();
+                        raw_headers_string =
-                    headers.push(Header {
+                            raw_headers_string + name.as_str() + ":" + value + "\r\n";
-                        name: name.as_str().to_owned(),
+                        headers_size += name.as_str().len() + value.len();
-                        value: value.to_owned(),
+                        headers.push(Header {
-                    });
+                            name: name.as_str().to_owned(),
+                            value: value.to_owned(),
+                        });
+                    }
                 }
+
                 let msg = GetRequestHeadersReply {
                     from: self.name(),
                     headers,
@@ -244,13 +234,13 @@ impl Actor for NetworkEventActor {
             },
             "getRequestCookies" => {
                 let mut cookies = Vec::new();
-
+                if let Some(ref headers) = self.request_headers_raw {
-                for cookie in self.request.headers.get_all(header::COOKIE) {
+                    for cookie in headers.get_all(header::COOKIE) {
-                    if let Ok(cookie_value) = String::from_utf8(cookie.as_bytes().to_vec()) {
+                        if let Ok(cookie_value) = String::from_utf8(cookie.as_bytes().to_vec()) {
-                        cookies = cookie_value.into_bytes();
+                            cookies = cookie_value.into_bytes();
+                        }
                     }
                 }
-
                 let msg = GetRequestCookiesReply {
                     from: self.name(),
                     cookies,
@@ -261,14 +251,14 @@ impl Actor for NetworkEventActor {
             "getRequestPostData" => {
                 let msg = GetRequestPostDataReply {
                     from: self.name(),
-                    post_data: self.request.body.clone(),
+                    post_data: self.request_body.clone(),
-                    post_data_discarded: false,
+                    post_data_discarded: self.request_body.is_none(),
                 };
                 let _ = stream.write_json_packet(&msg);
                 ActorMessageStatus::Processed
             },
             "getResponseHeaders" => {
-                if let Some(ref response_headers) = self.response.headers {
+                if let Some(ref response_headers) = self.response_headers_raw {
                     let mut headers = vec![];
                     let mut raw_headers_string = "".to_owned();
                     let mut headers_size = 0;
@@ -296,12 +286,13 @@ impl Actor for NetworkEventActor {
             "getResponseCookies" => {
                 let mut cookies = Vec::new();
                 // TODO: This seems quite broken
-                for cookie in self.request.headers.get_all(header::SET_COOKIE) {
+                if let Some(ref headers) = self.response_headers_raw {
-                    if let Ok(cookie_value) = String::from_utf8(cookie.as_bytes().to_vec()) {
+                    for cookie in headers.get_all(header::SET_COOKIE) {
-                        cookies = cookie_value.into_bytes();
+                        if let Ok(cookie_value) = String::from_utf8(cookie.as_bytes().to_vec()) {
+                            cookies = cookie_value.into_bytes();
+                        }
                     }
                 }
-
                 let msg = GetResponseCookiesReply {
                     from: self.name(),
                     cookies,
@@ -312,22 +303,16 @@ impl Actor for NetworkEventActor {
             "getResponseContent" => {
                 let msg = GetResponseContentReply {
                     from: self.name(),
-                    content: self.response.body.clone(),
+                    content: self.response_body.clone(),
-                    content_discarded: self.response.body.is_none(),
+                    content_discarded: self.response_body.is_none(),
                 };
                 let _ = stream.write_json_packet(&msg);
                 ActorMessageStatus::Processed
             },
             "getEventTimings" => {
                 // TODO: This is a fake timings msg
-                let timings_obj = Timings {
+                let timings_obj = self.event_timing.clone().unwrap_or_default();
-                    blocked: 0,
+                // Might use the one on self
-                    dns: 0,
-                    connect: self.request.connect_time.as_millis() as u64,
-                    send: self.request.send_time.as_millis() as u64,
-                    wait: 0,
-                    receive: 0,
-                };
                 let total = timings_obj.connect + timings_obj.send;
                 // TODO: Send the correct values for all these fields.
                 let msg = GetEventTimingsReply {
@@ -356,67 +341,60 @@ impl Actor for NetworkEventActor {
 
 impl NetworkEventActor {
     pub fn new(name: String) -> NetworkEventActor {
-        let request = HttpRequest {
+        NetworkEventActor {
-            url: String::new(),
+            name,
-            method: Method::GET,
+            is_xhr: false,
-            headers: HeaderMap::new(),
+            request_url: String::new(),
-            body: None,
+            request_method: Method::GET,
-            started_date_time: SystemTime::now(),
+            request_started: SystemTime::now(),
-            time_stamp: SystemTime::now()
+            request_time_stamp: SystemTime::now()
                 .duration_since(UNIX_EPOCH)
                 .unwrap_or_default()
                 .as_secs() as i64,
-            send_time: Duration::ZERO,
+            request_headers_raw: None,
-            connect_time: Duration::ZERO,
+            request_body: None,
-        };
+            request_cookies: None,
-        let response = HttpResponse {
+            request_headers: None,
-            headers: None,
+            response_headers_raw: None,
-            status: HttpStatus::default(),
+            response_body: None,
-            body: None,
-        };
-
-        NetworkEventActor {
-            name,
-            request: request.clone(),
-            response: response.clone(),
-            is_xhr: false,
             response_content: None,
             response_start: None,
             response_cookies: None,
             response_headers: None,
-            request_cookies: None,
+            total_time: Duration::ZERO,
-            request_headers: None,
+            security_state: "insecure".to_owned(),
-            total_time: Self::total_time(&request),
-            security_state: "insecure".to_owned(), // Default security state
             event_timing: None,
         }
     }
 
     pub fn add_request(&mut self, request: DevtoolsHttpRequest) {
-        request.url.as_str().clone_into(&mut self.request.url);
-
-        self.request.method = request.method.clone();
-        self.request.headers = request.headers.clone();
-        self.request.body = request.body;
-        self.request.started_date_time = request.started_date_time;
-        self.request.time_stamp = request.time_stamp;
-        self.request.connect_time = request.connect_time;
-        self.request.send_time = request.send_time;
         self.is_xhr = request.is_xhr;
+        self.request_cookies = Some(Self::request_cookies(&request));
+        self.request_headers = Some(Self::request_headers(&request));
+        self.total_time = Self::total_time(&request);
+        self.event_timing = Some(Self::event_timing(&request));
+        self.request_url = request.url.to_string();
+        self.request_method = request.method;
+        self.request_started = request.started_date_time;
+        self.request_time_stamp = request.time_stamp;
+        self.request_body = request.body.clone();
+        self.request_headers_raw = Some(request.headers.clone());
     }
 
     pub fn add_response(&mut self, response: DevtoolsHttpResponse) {
-        self.response.headers.clone_from(&response.headers);
+        self.response_headers = Some(Self::response_headers(&response));
-        self.response.status = response.status;
+        self.response_cookies = Some(Self::response_cookies(&response));
-        self.response.body = response.body;
+        self.response_start = Some(Self::response_start(&response));
+        self.response_content = Some(Self::response_content(&response));
+        self.response_body = response.body.clone();
+        self.response_headers_raw = response.headers.clone();
     }
 
     pub fn event_actor(&self) -> EventActor {
         // TODO: Send the correct values for startedDateTime, isXHR, private
 
         let started_datetime_rfc3339 = match Local.timestamp_millis_opt(
-            self.request
+            self.request_started
-                .started_date_time
                 .duration_since(UNIX_EPOCH)
                 .unwrap_or_default()
                 .as_millis() as i64,
@@ -426,7 +404,7 @@ impl NetworkEventActor {
             LocalResult::Ambiguous(date_time, _) => date_time.to_rfc3339().to_string(),
         };
 
-        let cause_type = match self.request.url.as_str() {
+        let cause_type = match self.request_url.as_str() {
             // Adjust based on request data
             url if url.ends_with(".css") => "stylesheet",
             url if url.ends_with(".js") => "script",
@@ -436,10 +414,10 @@ impl NetworkEventActor {
 
         EventActor {
             actor: self.name(),
-            url: self.request.url.clone(),
+            url: self.request_url.clone(),
-            method: format!("{}", self.request.method),
+            method: format!("{}", self.request_method),
             started_date_time: started_datetime_rfc3339,
-            time_stamp: self.request.time_stamp,
+            time_stamp: self.request_time_stamp,
             is_xhr: self.is_xhr,
             private: false,
             cause: Cause {
@@ -449,8 +427,7 @@ impl NetworkEventActor {
         }
     }
 
-    #[allow(dead_code)]
+    pub fn response_start(response: &DevtoolsHttpResponse) -> ResponseStartMsg {
-    pub fn response_start(response: &HttpResponse) -> ResponseStartMsg {
         // TODO: Send the correct values for all these fields.
         let h_size = response.headers.as_ref().map(|h| h.len()).unwrap_or(0);
         let status = &response.status;
@@ -467,16 +444,14 @@ impl NetworkEventActor {
         }
     }
 
-    #[allow(dead_code)]
+    pub fn response_content(response: &DevtoolsHttpResponse) -> ResponseContentMsg {
-    pub fn response_content(response: &HttpResponse) -> ResponseContentMsg {
+        let mime_type = response
-        let mime_type = if let Some(ref headers) = response.headers {
+            .headers
-            match headers.typed_get::<ContentType>() {
+            .as_ref()
-                Some(ct) => ct.to_string(),
+            .and_then(|h| h.typed_get::<ContentType>())
-                None => "".to_string(),
+            .map(|ct| ct.to_string())
-            }
+            .unwrap_or_default();
-        } else {
+
-            "".to_string()
-        };
         // TODO: Set correct values when response's body is sent to the devtools in http_loader.
         ResponseContentMsg {
             mime_type,
@@ -486,38 +461,33 @@ impl NetworkEventActor {
         }
     }
 
-    #[allow(dead_code)]
+    pub fn response_cookies(response: &DevtoolsHttpResponse) -> ResponseCookiesMsg {
-    pub fn response_cookies(response: &HttpResponse) -> ResponseCookiesMsg {
+        let cookies_size = response
-        let mut cookies_size = 0;
+            .headers
-        if let Some(ref headers) = response.headers {
+            .as_ref()
-            cookies_size = match headers.typed_get::<Cookie>() {
+            .map(|headers| headers.get_all("set-cookie").iter().count())
-                Some(ref cookie) => cookie.len(),
+            .unwrap_or(0);
-                _ => 0,
-            };
-        }
         ResponseCookiesMsg {
             cookies: cookies_size,
         }
     }
 
-    #[allow(dead_code)]
+    pub fn response_headers(response: &DevtoolsHttpResponse) -> ResponseHeadersMsg {
-    pub fn response_headers(response: &HttpResponse) -> ResponseHeadersMsg {
+        let mut header_size = 0;
-        let mut headers_size = 0;
         let mut headers_byte_count = 0;
         if let Some(ref headers) = response.headers {
-            headers_size = headers.len();
             for (name, value) in headers.iter() {
+                header_size += 1;
                 headers_byte_count += name.as_str().len() + value.len();
             }
         }
         ResponseHeadersMsg {
-            headers: headers_size,
+            headers: header_size,
             headers_size: headers_byte_count,
         }
     }
 
-    #[allow(dead_code)]
+    pub fn request_headers(request: &DevtoolsHttpRequest) -> RequestHeadersMsg {
-    pub fn request_headers(request: &HttpRequest) -> RequestHeadersMsg {
         let size = request.headers.iter().fold(0, |acc, (name, value)| {
             acc + name.as_str().len() + value.len()
         });
@@ -527,21 +497,32 @@ impl NetworkEventActor {
         }
     }
 
-    #[allow(dead_code)]
+    pub fn request_cookies(request: &DevtoolsHttpRequest) -> RequestCookiesMsg {
-    pub fn request_cookies(request: &HttpRequest) -> RequestCookiesMsg {
+        let cookies_size = request
-        let cookies_size = match request.headers.typed_get::<Cookie>() {
+            .headers
-            Some(ref cookie) => cookie.len(),
+            .typed_get::<Cookie>()
-            _ => 0,
+            .map(|c| c.len())
-        };
+            .unwrap_or(0);
         RequestCookiesMsg {
             cookies: cookies_size,
         }
     }
 
-    pub fn total_time(request: &HttpRequest) -> Duration {
+    pub fn total_time(request: &DevtoolsHttpRequest) -> Duration {
         request.connect_time + request.send_time
     }
 
+    pub fn event_timing(request: &DevtoolsHttpRequest) -> Timings {
+        Timings {
+            blocked: 0,
+            dns: 0,
+            connect: request.connect_time.as_millis() as u64,
+            send: request.send_time.as_millis() as u64,
+            wait: 0,
+            receive: 0,
+        }
+    }
+
     fn insert_serialized_map<T: Serialize>(map: &mut Map<String, Value>, obj: &Option<T>) {
         if let Some(value) = obj {
             if let Ok(Value::Object(serialized)) = serde_json::to_value(value) {

components/devtools/network_handler.rs

@@ -31,22 +31,30 @@ pub(crate) fn handle_network_event(
     let mut actors = actors.lock().unwrap();
     match network_event {
         NetworkEvent::HttpRequest(httprequest) => {
-            // Scope mutable borrow
+            let (event_actor, resource_updates) = {
-            let event_actor = {
                 let actor = actors.find_mut::<NetworkEventActor>(&netevent_actor_name);
                 actor.add_request(httprequest);
-                actor.event_actor()
+                (actor.event_actor(), actor.resource_updates())
             };
 
             let browsing_context_actor =
                 actors.find::<BrowsingContextActor>(&browsing_context_actor_name);
             for stream in &mut connections {
+                // Notify that a new network event has started
                 browsing_context_actor.resource_array(
                     event_actor.clone(),
                     "network-event".to_string(),
                     ResourceArrayType::Available,
                     stream,
                 );
+
+                // Also push initial resource update (request headers, cookies)
+                browsing_context_actor.resource_array(
+                    resource_updates.clone(),
+                    "network-event".to_string(),
+                    ResourceArrayType::Updated,
+                    stream,
+                );
             }
         },
         NetworkEvent::HttpResponse(httpresponse) => {