Send WillNavigate earlier during navigation startup (#37778)

The will-navigate message tells the devtools client to expect a
navigation for a browsing context. This makes the network monitor clear
any previous entries and show the requests for the new page that is
loaded. In order to support this correctly, we need to send the
navigation notification from the constellation instead of the script
thread, otherwise we silently ignore navigations triggered by the
browser URL bar.




Testing: Ran servo in devtools mode , now the requests appear for new
loaded page
Fixes: https://github.com/servo/servo/issues/37334

---------

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

components/devtools/actors/network_event.rs

@@ -41,6 +41,7 @@ pub struct NetworkEventActor {
     pub total_time: Duration,
     pub security_state: String,
     pub event_timing: Option<Timings>,
+    pub watcher_name: String,
 }
 
 #[derive(Clone, Serialize)]
@@ -342,7 +343,7 @@ impl Actor for NetworkEventActor {
 }
 
 impl NetworkEventActor {
-    pub fn new(name: String, resource_id: u64) -> NetworkEventActor {
+    pub fn new(name: String, resource_id: u64, watcher_name: String) -> NetworkEventActor {
         NetworkEventActor {
             name,
             resource_id,
@@ -367,6 +368,7 @@ impl NetworkEventActor {
             total_time: Duration::ZERO,
             security_state: "insecure".to_owned(),
             event_timing: None,
+            watcher_name,
         }
     }
 

components/devtools/actors/watcher.rs

@@ -14,9 +14,11 @@ use std::collections::HashMap;
 use std::net::TcpStream;
 use std::time::{SystemTime, UNIX_EPOCH};
 
+use base::id::BrowsingContextId;
 use log::warn;
 use serde::Serialize;
 use serde_json::{Map, Value};
+use servo_url::ServoUrl;
 
 use self::network_parent::{NetworkParentActor, NetworkParentActorMsg};
 use super::breakpoint::BreakpointListActor;
@@ -33,7 +35,7 @@ use crate::actors::watcher::thread_configuration::{
 };
 use crate::protocol::JsonPacketStream;
 use crate::resource::{ResourceArrayType, ResourceAvailable};
-use crate::{EmptyReplyMsg, StreamId, WorkerActor};
+use crate::{EmptyReplyMsg, IdMap, StreamId, WorkerActor};
 
 pub mod network_parent;
 pub mod target_configuration;
@@ -79,7 +81,7 @@ impl SessionContext {
                 ("local-storage", false),
                 ("session-storage", false),
                 ("platform-message", false),
-                ("network-event", false),
+                ("network-event", true),
                 ("network-event-stacktrace", false),
                 ("reflow", false),
                 ("stylesheet", false),
@@ -192,6 +194,19 @@ pub struct WatcherActor {
     session_context: SessionContext,
 }
 
+#[derive(Clone, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct WillNavigateMessage {
+    #[serde(rename = "browsingContextID")]
+    browsing_context_id: u32,
+    inner_window_id: u32,
+    name: String,
+    time: u128,
+    is_frame_switching: bool,
+    #[serde(rename = "newURI")]
+    new_uri: ServoUrl,
+}
+
 impl Actor for WatcherActor {
     fn name(&self) -> String {
         self.name.clone()
@@ -322,6 +337,7 @@ impl Actor for WatcherActor {
                             }
                         },
                         "console-message" | "error-message" => {},
+                        "network-event" => {},
                         _ => warn!("resource {} not handled yet", resource),
                     }
 
@@ -385,6 +401,12 @@ impl Actor for WatcherActor {
     }
 }
 
+impl ResourceAvailable for WatcherActor {
+    fn actor_name(&self) -> String {
+        self.name.clone()
+    }
+}
+
 impl WatcherActor {
     pub fn new(
         actors: &mut ActorRegistry,
@@ -422,4 +444,33 @@ impl WatcherActor {
             },
         }
     }
+
+    pub fn emit_will_navigate(
+        &self,
+        browsing_context_id: BrowsingContextId,
+        url: ServoUrl,
+        connections: &mut Vec<TcpStream>,
+        id_map: &mut IdMap,
+    ) {
+        let msg = WillNavigateMessage {
+            browsing_context_id: id_map.browsing_context_id(browsing_context_id).value(),
+            inner_window_id: 0, // TODO: set this to the correct value
+            name: "will-navigate".to_string(),
+            time: SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .unwrap_or_default()
+                .as_millis(),
+            is_frame_switching: false, // TODO: Implement frame switching
+            new_uri: url,
+        };
+
+        for stream in connections {
+            self.resource_array(
+                msg.clone(),
+                "document-event".to_string(),
+                ResourceArrayType::Available,
+                stream,
+            );
+        }
+    }
 }

components/devtools/lib.rs

@@ -45,6 +45,7 @@ use crate::actors::process::ProcessActor;
 use crate::actors::root::RootActor;
 use crate::actors::source::SourceActor;
 use crate::actors::thread::ThreadActor;
+use crate::actors::watcher::WatcherActor;
 use crate::actors::worker::{WorkerActor, WorkerType};
 use crate::id::IdMap;
 use crate::network_handler::handle_network_event;
@@ -289,11 +290,7 @@ impl DevtoolsInstance {
                         connections.push(stream.try_clone().unwrap());
                     }
 
-                    let pipeline_id = match network_event {
-                        NetworkEvent::HttpResponse(ref response) => response.pipeline_id,
-                        NetworkEvent::HttpRequest(ref request) => request.pipeline_id,
-                    };
-                    self.handle_network_event(connections, pipeline_id, request_id, network_event);
+                    self.handle_network_event(connections, request_id, network_event);
                 },
                 DevtoolsControlMsg::FromChrome(ChromeToDevtoolsControlMsg::ServerExitMsg) => break,
             }
@@ -313,11 +310,23 @@ impl DevtoolsInstance {
 
     fn handle_navigate(&self, browsing_context_id: BrowsingContextId, state: NavigationState) {
         let actor_name = self.browsing_contexts.get(&browsing_context_id).unwrap();
-        self.actors
-            .lock()
-            .unwrap()
-            .find::<BrowsingContextActor>(actor_name)
-            .navigate(state, &mut self.id_map.lock().expect("Mutex poisoned"));
+        let actors = self.actors.lock().unwrap();
+        let actor = actors.find::<BrowsingContextActor>(actor_name);
+        let mut id_map = self.id_map.lock().expect("Mutex poisoned");
+        if let NavigationState::Start(url) = &state {
+            let mut connections = Vec::<TcpStream>::new();
+            for stream in self.connections.values() {
+                connections.push(stream.try_clone().unwrap());
+            }
+            let watcher_actor = actors.find::<WatcherActor>(&actor.watcher);
+            watcher_actor.emit_will_navigate(
+                browsing_context_id,
+                url.clone(),
+                &mut connections,
+                &mut id_map,
+            );
+        };
+        actor.navigate(state, &mut id_map);
     }
 
     // We need separate actor representations for each script global that exists;
@@ -479,31 +488,39 @@ impl DevtoolsInstance {
     fn handle_network_event(
         &mut self,
         connections: Vec<TcpStream>,
-        pipeline_id: PipelineId,
         request_id: String,
         network_event: NetworkEvent,
     ) {
-        let netevent_actor_name = self.find_network_event_actor(request_id);
+        let browsing_context_id = match &network_event {
+            NetworkEvent::HttpRequest(req) => req.browsing_context_id,
+            NetworkEvent::HttpResponse(resp) => resp.browsing_context_id,
+        };
 
-        let Some(id) = self.pipelines.get(&pipeline_id) else {
-            return;
-        };
-        let Some(browsing_context_actor_name) = self.browsing_contexts.get(id) else {
+        let Some(browsing_context_actor_name) = self.browsing_contexts.get(&browsing_context_id)
+        else {
             return;
         };
+        let watcher_name = self
+            .actors
+            .lock()
+            .unwrap()
+            .find::<BrowsingContextActor>(browsing_context_actor_name)
+            .watcher
+            .clone();
+
+        let netevent_actor_name = self.find_network_event_actor(request_id, watcher_name);
 
         handle_network_event(
             Arc::clone(&self.actors),
             netevent_actor_name,
             connections,
             network_event,
-            browsing_context_actor_name.to_string(),
         )
     }
 
     // Find the name of NetworkEventActor corresponding to request_id
     // Create a new one if it does not exist, add it to the actor_requests hashmap
-    fn find_network_event_actor(&mut self, request_id: String) -> String {
+    fn find_network_event_actor(&mut self, request_id: String, watcher_name: String) -> String {
         let mut actors = self.actors.lock().unwrap();
         match self.actor_requests.entry(request_id) {
             Occupied(name) => {
@@ -514,7 +531,7 @@ impl DevtoolsInstance {
                 let resource_id = self.next_resource_id;
                 self.next_resource_id += 1;
                 let actor_name = actors.new_name("netevent");
-                let actor = NetworkEventActor::new(actor_name.clone(), resource_id);
+                let actor = NetworkEventActor::new(actor_name.clone(), resource_id, watcher_name);
                 entry.insert(actor_name.clone());
                 actors.register(Box::new(actor));
                 actor_name

components/devtools/network_handler.rs

@@ -9,8 +9,8 @@ use devtools_traits::NetworkEvent;
 use serde::Serialize;
 
 use crate::actor::ActorRegistry;
-use crate::actors::browsing_context::BrowsingContextActor;
 use crate::actors::network_event::NetworkEventActor;
+use crate::actors::watcher::WatcherActor;
 use crate::resource::{ResourceArrayType, ResourceAvailable};
 
 #[derive(Clone, Serialize)]
@@ -26,22 +26,20 @@ pub(crate) fn handle_network_event(
     netevent_actor_name: String,
     mut connections: Vec<TcpStream>,
     network_event: NetworkEvent,
-    browsing_context_actor_name: String,
 ) {
     let mut actors = actors.lock().unwrap();
+    let actor = actors.find_mut::<NetworkEventActor>(&netevent_actor_name);
+    let watcher_name = actor.watcher_name.clone();
     match network_event {
         NetworkEvent::HttpRequest(httprequest) => {
-            let (event_actor, resource_updates) = {
-                let actor = actors.find_mut::<NetworkEventActor>(&netevent_actor_name);
-                actor.add_request(httprequest);
-                (actor.event_actor(), actor.resource_updates())
-            };
+            actor.add_request(httprequest);
+
+            let event_actor = actor.event_actor();
+            let resource_updates = actor.resource_updates();
+            let watcher_actor = actors.find::<WatcherActor>(&watcher_name);
 
-            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(
+                watcher_actor.resource_array(
                     event_actor.clone(),
                     "network-event".to_string(),
                     ResourceArrayType::Available,
@@ -49,7 +47,7 @@ pub(crate) fn handle_network_event(
                 );
 
                 // Also push initial resource update (request headers, cookies)
-                browsing_context_actor.resource_array(
+                watcher_actor.resource_array(
                     resource_updates.clone(),
                     "network-event".to_string(),
                     ResourceArrayType::Updated,
@@ -58,18 +56,13 @@ pub(crate) fn handle_network_event(
             }
         },
         NetworkEvent::HttpResponse(httpresponse) => {
-            // Scope mutable borrow
-            let resource = {
-                let actor = actors.find_mut::<NetworkEventActor>(&netevent_actor_name);
-                // Store the response information in the actor
-                actor.add_response(httpresponse);
-                actor.resource_updates()
-            };
+            // Store the response information in the actor
+            actor.add_response(httpresponse);
+            let resource = actor.resource_updates();
+            let watcher_actor = actors.find::<WatcherActor>(&watcher_name);
 
-            let browsing_context_actor =
-                actors.find::<BrowsingContextActor>(&browsing_context_actor_name);
             for stream in &mut connections {
-                browsing_context_actor.resource_array(
+                watcher_actor.resource_array(
                     resource.clone(),
                     "network-event".to_string(),
                     ResourceArrayType::Updated,