devtools: improve ID Naming for better readability and context (#35942)

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

components/devtools/actor.rs

@@ -32,7 +32,7 @@ pub(crate) trait Actor: Any + ActorAsAny {
         msg_type: &str,
         msg: &Map<String, Value>,
         stream: &mut TcpStream,
-        id: StreamId,
+        stream_id: StreamId,
     ) -> Result<ActorMessageStatus, ()>;
     fn name(&self) -> String;
     fn cleanup(&self, _id: StreamId) {}
@@ -77,9 +77,9 @@ impl ActorRegistry {
         }
     }
 
-    pub(crate) fn cleanup(&self, id: StreamId) {
+    pub(crate) fn cleanup(&self, stream_id: StreamId) {
         for actor in self.actors.values() {
-            actor.cleanup(id);
+            actor.cleanup(stream_id);
         }
     }
 
@@ -170,7 +170,7 @@ impl ActorRegistry {
         &mut self,
         msg: &Map<String, Value>,
         stream: &mut TcpStream,
-        id: StreamId,
+        stream_id: StreamId,
     ) -> Result<(), ()> {
         let to = match msg.get("to") {
             Some(to) => to.as_str().unwrap(),
@@ -184,7 +184,7 @@ impl ActorRegistry {
             None => log::warn!("message received for unknown actor \"{}\"", to),
             Some(actor) => {
                 let msg_type = msg.get("type").unwrap().as_str().unwrap();
-                if actor.handle_message(self, msg_type, msg, stream, id)? !=
+                if actor.handle_message(self, msg_type, msg, stream, stream_id)? !=
                     ActorMessageStatus::Processed
                 {
                     log::warn!(

components/devtools/actors/browsing_context.rs

@@ -173,9 +173,9 @@ impl Actor for BrowsingContextActor {
 impl BrowsingContextActor {
     pub(crate) fn new(
         console: String,
-        id: BrowsingContextId,
+        browsing_context_id: BrowsingContextId,
         page_info: DevtoolsPageInfo,
-        pipeline: PipelineId,
+        pipeline_id: PipelineId,
         script_sender: IpcSender<DevtoolScriptControlMsg>,
         actors: &mut ActorRegistry,
     ) -> BrowsingContextActor {
@@ -224,8 +224,8 @@ impl BrowsingContextActor {
             script_chan: script_sender,
             title: RefCell::new(title),
             url: RefCell::new(url.into_string()),
-            active_pipeline: Cell::new(pipeline),
+            active_pipeline: Cell::new(pipeline_id),
-            browsing_context_id: id,
+            browsing_context_id,
             accessibility: accessibility.name(),
             console,
             css_properties: css_properties.name(),
@@ -308,8 +308,8 @@ impl BrowsingContextActor {
         }
     }
 
-    pub(crate) fn title_changed(&self, pipeline: PipelineId, title: String) {
+    pub(crate) fn title_changed(&self, pipeline_id: PipelineId, title: String) {
-        if pipeline != self.active_pipeline.get() {
+        if pipeline_id != self.active_pipeline.get() {
             return;
         }
         *self.title.borrow_mut() = title;

components/devtools/actors/console.rs

@@ -155,7 +155,7 @@ impl ConsoleActor {
                     .active_pipeline
                     .get(),
             ),
-            Root::DedicatedWorker(w) => UniqueId::Worker(registry.find::<WorkerActor>(w).id),
+            Root::DedicatedWorker(w) => UniqueId::Worker(registry.find::<WorkerActor>(w).worker_id),
         }
     }
 

components/devtools/actors/framerate.rs

@@ -16,7 +16,7 @@ use crate::actors::timeline::HighResolutionStamp;
 
 pub struct FramerateActor {
     name: String,
-    pipeline: PipelineId,
+    pipeline_id: PipelineId,
     script_sender: IpcSender<DevtoolScriptControlMsg>,
     is_recording: bool,
     ticks: Vec<HighResolutionStamp>,
@@ -49,7 +49,7 @@ impl FramerateActor {
         let actor_name = registry.new_name("framerate");
         let mut actor = FramerateActor {
             name: actor_name.clone(),
-            pipeline: pipeline_id,
+            pipeline_id,
             script_sender,
             is_recording: false,
             ticks: Vec::new(),
@@ -64,7 +64,7 @@ impl FramerateActor {
         self.ticks.push(HighResolutionStamp::wrap(tick));
 
         if self.is_recording {
-            let msg = DevtoolScriptControlMsg::RequestAnimationFrame(self.pipeline, self.name());
+            let msg = DevtoolScriptControlMsg::RequestAnimationFrame(self.pipeline_id, self.name());
             self.script_sender.send(msg).unwrap();
         }
     }
@@ -80,7 +80,7 @@ impl FramerateActor {
 
         self.is_recording = true;
 
-        let msg = DevtoolScriptControlMsg::RequestAnimationFrame(self.pipeline, self.name());
+        let msg = DevtoolScriptControlMsg::RequestAnimationFrame(self.pipeline_id, self.name());
         self.script_sender.send(msg).unwrap();
     }
 

components/devtools/actors/timeline.rs

@@ -30,7 +30,7 @@ pub struct TimelineActor {
     name: String,
     script_sender: IpcSender<DevtoolScriptControlMsg>,
     marker_types: Vec<TimelineMarkerType>,
-    pipeline: PipelineId,
+    pipeline_id: PipelineId,
     is_recording: Arc<Mutex<bool>>,
     stream: RefCell<Option<TcpStream>>,
 
@@ -132,14 +132,14 @@ static DEFAULT_TIMELINE_DATA_PULL_TIMEOUT: u64 = 200; //ms
 impl TimelineActor {
     pub fn new(
         name: String,
-        pipeline: PipelineId,
+        pipeline_id: PipelineId,
         script_sender: IpcSender<DevtoolScriptControlMsg>,
     ) -> TimelineActor {
         let marker_types = vec![TimelineMarkerType::Reflow, TimelineMarkerType::DOMEvent];
 
         TimelineActor {
             name,
-            pipeline,
+            pipeline_id,
             marker_types,
             script_sender,
             is_recording: Arc::new(Mutex::new(false)),
@@ -204,7 +204,7 @@ impl Actor for TimelineActor {
                 let (tx, rx) = ipc::channel::<Option<TimelineMarker>>().unwrap();
                 self.script_sender
                     .send(SetTimelineMarkers(
-                        self.pipeline,
+                        self.pipeline_id,
                         self.marker_types.clone(),
                         tx,
                     ))
@@ -225,7 +225,7 @@ impl Actor for TimelineActor {
                     if let Some(true) = with_ticks.as_bool() {
                         let framerate_actor = Some(FramerateActor::create(
                             registry,
-                            self.pipeline,
+                            self.pipeline_id,
                             self.script_sender.clone(),
                         ));
                         *self.framerate_actor.borrow_mut() = framerate_actor;
@@ -266,7 +266,7 @@ impl Actor for TimelineActor {
                 let _ = stream.write_json_packet(&msg);
                 self.script_sender
                     .send(DropTimelineMarkers(
-                        self.pipeline,
+                        self.pipeline_id,
                         self.marker_types.clone(),
                     ))
                     .unwrap();

components/devtools/actors/worker.rs

@@ -30,7 +30,7 @@ pub(crate) struct WorkerActor {
     pub name: String,
     pub console: String,
     pub thread: String,
-    pub id: WorkerId,
+    pub worker_id: WorkerId,
     pub url: ServoUrl,
     pub type_: WorkerType,
     pub script_chan: IpcSender<DevtoolScriptControlMsg>,
@@ -43,7 +43,7 @@ impl WorkerActor {
             actor: self.name.clone(),
             console_actor: self.console.clone(),
             thread_actor: self.thread.clone(),
-            id: self.id.0.to_string(),
+            worker_id: self.worker_id.0.to_string(),
             url: self.url.to_string(),
             traits: WorkerTraits {
                 is_parent_intercept_enabled: false,
@@ -63,7 +63,7 @@ impl Actor for WorkerActor {
         msg_type: &str,
         _msg: &Map<String, Value>,
         stream: &mut TcpStream,
-        id: StreamId,
+        stream_id: StreamId,
     ) -> Result<ActorMessageStatus, ()> {
         Ok(match msg_type {
             "attach" => {
@@ -77,7 +77,7 @@ impl Actor for WorkerActor {
                 }
                 self.streams
                     .borrow_mut()
-                    .insert(id, stream.try_clone().unwrap());
+                    .insert(stream_id, stream.try_clone().unwrap());
                 // FIXME: fix messages to not require forging a pipeline for worker messages
                 self.script_chan
                     .send(WantsLiveNotifications(TEST_PIPELINE_ID, true))
@@ -102,7 +102,7 @@ impl Actor for WorkerActor {
                     type_: "detached".to_string(),
                 };
                 let _ = stream.write_json_packet(&msg);
-                self.cleanup(id);
+                self.cleanup(stream_id);
                 ActorMessageStatus::Processed
             },
 
@@ -110,8 +110,8 @@ impl Actor for WorkerActor {
         })
     }
 
-    fn cleanup(&self, id: StreamId) {
+    fn cleanup(&self, stream_id: StreamId) {
-        self.streams.borrow_mut().remove(&id);
+        self.streams.borrow_mut().remove(&stream_id);
         if self.streams.borrow().is_empty() {
             self.script_chan
                 .send(WantsLiveNotifications(TEST_PIPELINE_ID, false))
@@ -157,7 +157,7 @@ pub(crate) struct WorkerMsg {
     actor: String,
     console_actor: String,
     thread_actor: String,
-    id: String,
+    worker_id: String,
     url: String,
     traits: WorkerTraits,
     #[serde(rename = "type")]

components/devtools/lib.rs

@@ -275,16 +275,16 @@ impl DevtoolsInstance {
                     state,
                 )) => self.handle_navigate(browsing_context, state),
                 DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ConsoleAPI(
-                    id,
+                    pipeline_id,
                     console_message,
                     worker_id,
-                )) => self.handle_console_message(id, worker_id, console_message),
+                )) => self.handle_console_message(pipeline_id, worker_id, console_message),
                 DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ReportPageError(
-                    id,
+                    pipeline_id,
                     page_error,
-                )) => self.handle_page_error(id, None, page_error),
+                )) => self.handle_page_error(pipeline_id, None, page_error),
                 DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ReportCSSError(
-                    id,
+                    pipeline_id,
                     css_error,
                 )) => {
                     let mut console_message = ConsoleMessageBuilder::new(
@@ -295,7 +295,7 @@ impl DevtoolsInstance {
                     );
                     console_message.add_argument(css_error.msg.into());
 
-                    self.handle_console_message(id, None, console_message.finish())
+                    self.handle_console_message(pipeline_id, None, console_message.finish())
                 },
                 DevtoolsControlMsg::FromChrome(ChromeToDevtoolsControlMsg::NetworkEvent(
                     request_id,
@@ -329,8 +329,8 @@ impl DevtoolsInstance {
         framerate_actor.add_tick(tick);
     }
 
-    fn handle_navigate(&self, browsing_context: BrowsingContextId, state: NavigationState) {
+    fn handle_navigate(&self, browsing_context_id: BrowsingContextId, state: NavigationState) {
-        let actor_name = self.browsing_contexts.get(&browsing_context).unwrap();
+        let actor_name = self.browsing_contexts.get(&browsing_context_id).unwrap();
         self.actors
             .lock()
             .unwrap()
@@ -349,13 +349,13 @@ impl DevtoolsInstance {
     ) {
         let mut actors = self.actors.lock().unwrap();
 
-        let (browsing_context, pipeline, worker_id) = ids;
+        let (browsing_context_id, pipeline_id, worker_id) = ids;
 
         let console_name = actors.new_name("console");
 
         let parent_actor = if let Some(id) = worker_id {
-            assert!(self.pipelines.contains_key(&pipeline));
+            assert!(self.pipelines.contains_key(&pipeline_id));
-            assert!(self.browsing_contexts.contains_key(&browsing_context));
+            assert!(self.browsing_contexts.contains_key(&browsing_context_id));
 
             let thread = ThreadActor::new(actors.new_name("context"));
             let thread_name = thread.name();
@@ -366,7 +366,7 @@ impl DevtoolsInstance {
                 name: worker_name.clone(),
                 console: console_name.clone(),
                 thread: thread_name,
-                id,
+                worker_id: id,
                 url: page_info.url.clone(),
                 type_: WorkerType::Dedicated,
                 script_chan: script_sender,
@@ -380,16 +380,16 @@ impl DevtoolsInstance {
 
             Root::DedicatedWorker(worker_name)
         } else {
-            self.pipelines.insert(pipeline, browsing_context);
+            self.pipelines.insert(pipeline_id, browsing_context_id);
             let name = self
                 .browsing_contexts
-                .entry(browsing_context)
+                .entry(browsing_context_id)
                 .or_insert_with(|| {
                     let browsing_context_actor = BrowsingContextActor::new(
                         console_name.clone(),
-                        browsing_context,
+                        browsing_context_id,
                         page_info,
-                        pipeline,
+                        pipeline_id,
                         script_sender,
                         &mut actors,
                     );
@@ -417,8 +417,8 @@ impl DevtoolsInstance {
         actors.register(Box::new(console));
     }
 
-    fn handle_title_changed(&self, pipeline: PipelineId, title: String) {
+    fn handle_title_changed(&self, pipeline_id: PipelineId, title: String) {
-        let bc = match self.pipelines.get(&pipeline) {
+        let bc = match self.pipelines.get(&pipeline_id) {
             Some(bc) => bc,
             None => return,
         };
@@ -428,44 +428,44 @@ impl DevtoolsInstance {
         };
         let actors = self.actors.lock().unwrap();
         let browsing_context = actors.find::<BrowsingContextActor>(name);
-        browsing_context.title_changed(pipeline, title);
+        browsing_context.title_changed(pipeline_id, title);
     }
 
     fn handle_page_error(
         &self,
-        id: PipelineId,
+        pipeline_id: PipelineId,
         worker_id: Option<WorkerId>,
         page_error: PageError,
     ) {
-        let console_actor_name = match self.find_console_actor(id, worker_id) {
+        let console_actor_name = match self.find_console_actor(pipeline_id, worker_id) {
             Some(name) => name,
             None => return,
         };
         let actors = self.actors.lock().unwrap();
         let console_actor = actors.find::<ConsoleActor>(&console_actor_name);
-        let id = worker_id.map_or(UniqueId::Pipeline(id), UniqueId::Worker);
+        let id = worker_id.map_or(UniqueId::Pipeline(pipeline_id), UniqueId::Worker);
         console_actor.handle_page_error(page_error, id, &actors);
     }
 
     fn handle_console_message(
         &self,
-        id: PipelineId,
+        pipeline_id: PipelineId,
         worker_id: Option<WorkerId>,
         console_message: ConsoleMessage,
     ) {
-        let console_actor_name = match self.find_console_actor(id, worker_id) {
+        let console_actor_name = match self.find_console_actor(pipeline_id, worker_id) {
             Some(name) => name,
             None => return,
         };
         let actors = self.actors.lock().unwrap();
         let console_actor = actors.find::<ConsoleActor>(&console_actor_name);
-        let id = worker_id.map_or(UniqueId::Pipeline(id), UniqueId::Worker);
+        let id = worker_id.map_or(UniqueId::Pipeline(pipeline_id), UniqueId::Worker);
         console_actor.handle_console_api(console_message, id, &actors);
     }
 
     fn find_console_actor(
         &self,
-        pipeline: PipelineId,
+        pipeline_id: PipelineId,
         worker_id: Option<WorkerId>,
     ) -> Option<String> {
         let actors = self.actors.lock().unwrap();
@@ -473,7 +473,7 @@ impl DevtoolsInstance {
             let actor_name = self.actor_workers.get(&worker_id)?;
             Some(actors.find::<WorkerActor>(actor_name).console.clone())
         } else {
-            let id = self.pipelines.get(&pipeline)?;
+            let id = self.pipelines.get(&pipeline_id)?;
             let actor_name = self.browsing_contexts.get(id)?;
             Some(
                 actors
@@ -649,7 +649,7 @@ fn allow_devtools_client(stream: &mut TcpStream, embedder: &EmbedderProxy, token
 }
 
 /// Process the input from a single devtools client until EOF.
-fn handle_client(actors: Arc<Mutex<ActorRegistry>>, mut stream: TcpStream, id: StreamId) {
+fn handle_client(actors: Arc<Mutex<ActorRegistry>>, mut stream: TcpStream, stream_id: StreamId) {
     log::info!("Connection established to {}", stream.peer_addr().unwrap());
     let msg = actors.lock().unwrap().find::<RootActor>("root").encodable();
     if let Err(e) = stream.write_json_packet(&msg) {
@@ -663,7 +663,7 @@ fn handle_client(actors: Arc<Mutex<ActorRegistry>>, mut stream: TcpStream, id: S
                 if let Err(()) = actors.lock().unwrap().handle_message(
                     json_packet.as_object().unwrap(),
                     &mut stream,
-                    id,
+                    stream_id,
                 ) {
                     log::error!("Devtools actor stopped responding");
                     let _ = stream.shutdown(Shutdown::Both);
@@ -681,5 +681,5 @@ fn handle_client(actors: Arc<Mutex<ActorRegistry>>, mut stream: TcpStream, id: S
         }
     }
 
-    actors.lock().unwrap().cleanup(id);
+    actors.lock().unwrap().cleanup(stream_id);
 }