From 33f3c34d28cdd970455f93dde4d7f3a9ad0bbb2e Mon Sep 17 00:00:00 2001 From: eri Date: Tue, 9 Jul 2024 20:27:47 +0200 Subject: [PATCH] DevTools: Display console messages and errors (#32727) * feat: add streams to browsing context * feat: console now works! * feat: order console messages * feat: add streams to new browsing contexts * fix: apply suggestions Co-authored-by: Martin Robinson --------- Co-authored-by: Martin Robinson --- .../devtools/actors/browsing_context.rs | 64 ++++++++- components/devtools/actors/console.rs | 123 +++++------------- components/devtools/actors/object.rs | 1 + components/devtools/lib.rs | 48 ++++--- components/script/dom/globalscope.rs | 12 +- components/shared/devtools/lib.rs | 12 +- 6 files changed, 141 insertions(+), 119 deletions(-) diff --git a/components/devtools/actors/browsing_context.rs b/components/devtools/actors/browsing_context.rs index d584a28621b..1d5dbfaf4c6 100644 --- a/components/devtools/actors/browsing_context.rs +++ b/components/devtools/actors/browsing_context.rs @@ -12,7 +12,7 @@ use std::net::TcpStream; use base::id::{BrowsingContextId, PipelineId}; use devtools_traits::DevtoolScriptControlMsg::{self, WantsLiveNotifications}; -use devtools_traits::{DevtoolsPageInfo, NavigationState}; +use devtools_traits::{ConsoleLog, DevtoolsPageInfo, NavigationState, PageError}; use ipc_channel::ipc::IpcSender; use serde::Serialize; use serde_json::{Map, Value}; @@ -70,6 +70,36 @@ struct ResourceAvailableMsg { url: Option, } +#[derive(Serialize)] +struct ConsoleMsg { + from: String, + #[serde(rename = "type")] + type_: String, + resources: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct ConsoleMessageResource { + message: ConsoleLog, + resource_type: String, +} + +#[derive(Serialize)] +struct PageErrorMsg { + from: String, + #[serde(rename = "type")] + type_: String, + resources: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct PageErrorResource { + page_error: PageError, + resource_type: String, +} + #[derive(Serialize)] struct TabNavigated { from: String, @@ -347,7 +377,7 @@ impl BrowsingContextActor { from: self.name(), type_: "resource-available-form".into(), resources: vec![ResourceAvailableMsg { - has_native_console_api: None, + has_native_console_api: Some(true), name: name.into(), new_uri: None, resource_type: "document-event".into(), @@ -358,4 +388,34 @@ impl BrowsingContextActor { }); } } + + pub(crate) fn console_message(&self, message: ConsoleLog) { + let msg = ConsoleMsg { + from: self.name(), + type_: "resource-available-form".into(), + resources: vec![ConsoleMessageResource { + message, + resource_type: "console-message".into(), + }], + }; + + for stream in self.streams.borrow_mut().values_mut() { + let _ = stream.write_json_packet(&msg); + } + } + + pub(crate) fn page_error(&self, page_error: PageError) { + let msg = PageErrorMsg { + from: self.name(), + type_: "resource-available-form".into(), + resources: vec![PageErrorResource { + page_error, + resource_type: "error-message".into(), + }], + }; + + for stream in self.streams.borrow_mut().values_mut() { + let _ = stream.write_json_packet(&msg); + } + } } diff --git a/components/devtools/actors/console.rs b/components/devtools/actors/console.rs index 56887b45882..0a773c270d4 100644 --- a/components/devtools/actors/console.rs +++ b/components/devtools/actors/console.rs @@ -16,7 +16,7 @@ use devtools_traits::EvaluateJSReply::{ ActorValue, BooleanValue, NullValue, NumberValue, StringValue, VoidValue, }; use devtools_traits::{ - CachedConsoleMessage, CachedConsoleMessageTypes, ConsoleAPI, ConsoleMessage, + CachedConsoleMessage, CachedConsoleMessageTypes, ConsoleLog, ConsoleMessage, DevtoolScriptControlMsg, LogLevel, PageError, }; use ipc_channel::ipc::{self, IpcSender}; @@ -40,7 +40,7 @@ impl EncodableConsoleMessage for CachedConsoleMessage { fn encode(&self) -> serde_json::Result { match *self { CachedConsoleMessage::PageError(ref a) => serde_json::to_string(a), - CachedConsoleMessage::ConsoleAPI(ref a) => serde_json::to_string(a), + CachedConsoleMessage::ConsoleLog(ref a) => serde_json::to_string(a), } } } @@ -141,23 +141,6 @@ impl ConsoleActor { } } - fn streams_mut(&self, registry: &ActorRegistry, cb: impl Fn(&mut TcpStream)) { - match &self.root { - Root::BrowsingContext(bc) => registry - .find::(bc) - .streams - .borrow_mut() - .values_mut() - .for_each(cb), - Root::DedicatedWorker(worker) => registry - .find::(worker) - .streams - .borrow_mut() - .values_mut() - .for_each(cb), - } - } - fn current_unique_id(&self, registry: &ActorRegistry) -> UniqueId { match &self.root { Root::BrowsingContext(bc) => UniqueId::Pipeline( @@ -177,7 +160,7 @@ impl ConsoleActor { ) -> Result { let input = msg.get("text").unwrap().as_str().unwrap().to_owned(); let (chan, port) = ipc::channel().unwrap(); - // FIXME: redesign messages so we don't have to fake pipeline ids when + // FIXME: Redesign messages so we don't have to fake pipeline ids when // communicating with workers. let pipeline = match self.current_unique_id(registry) { UniqueId::Pipeline(p) => p, @@ -191,7 +174,7 @@ impl ConsoleActor { )) .unwrap(); - //TODO: extract conversion into protocol module or some other useful place + // TODO: Extract conversion into protocol module or some other useful place let result = match port.recv().map_err(|_| ())? { VoidValue => { let mut m = Map::new(); @@ -227,7 +210,7 @@ impl ConsoleActor { }, StringValue(s) => Value::String(s), ActorValue { class, uuid } => { - //TODO: make initial ActorValue message include these properties? + // TODO: Make initial ActorValue message include these properties? let mut m = Map::new(); let actor = ObjectActor::register(registry, uuid); @@ -241,12 +224,15 @@ impl ConsoleActor { }, }; - //TODO: catch and return exception values from JS evaluation + // TODO: Catch and return exception values from JS evaluation let reply = EvaluateJSReply { from: self.name(), input, result, - timestamp: 0, + timestamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64, exception: Value::Null, exception_message: Value::Null, helper_result: Value::Null, @@ -266,14 +252,11 @@ impl ConsoleActor { .or_default() .push(CachedConsoleMessage::PageError(page_error.clone())); if id == self.current_unique_id(registry) { - let msg = PageErrorMsg { - from: self.name(), - type_: "pageError".to_owned(), - page_error, + if let Root::BrowsingContext(bc) = &self.root { + registry + .find::(bc) + .page_error(page_error) }; - self.streams_mut(registry, |stream| { - let _ = stream.write_json_packet(&msg); - }); } } @@ -292,42 +275,30 @@ impl ConsoleActor { _ => "log", } .to_owned(); + + let console_api = ConsoleLog { + level: level.clone(), + filename: console_message.filename.clone(), + line_number: console_message.line_number as u32, + column_number: console_message.column_number as u32, + time_stamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64, + arguments: vec![console_message.message.clone()], + }; + self.cached_events .borrow_mut() .entry(id.clone()) .or_default() - .push(CachedConsoleMessage::ConsoleAPI(ConsoleAPI { - type_: "ConsoleAPI".to_owned(), - level: level.clone(), - filename: console_message.filename.clone(), - line_number: console_message.line_number as u32, - function_name: "".to_string(), //TODO - time_stamp: SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() as u64, - private: false, - arguments: vec![console_message.message.clone()], - })); + .push(CachedConsoleMessage::ConsoleLog(console_api.clone())); if id == self.current_unique_id(registry) { - let msg = ConsoleAPICall { - from: self.name(), - type_: "consoleAPICall".to_owned(), - message: ConsoleMsg { - level, - timestamp: SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() as u64, - arguments: vec![console_message.message], - filename: console_message.filename, - line_number: console_message.line_number, - column_number: console_message.column_number, - }, + if let Root::BrowsingContext(bc) = &self.root { + registry + .find::(bc) + .console_message(console_api) }; - self.streams_mut(registry, |stream| { - let _ = stream.write_json_packet(&msg); - }); } } } @@ -385,7 +356,7 @@ impl Actor for ConsoleActor { { true }, - CachedConsoleMessage::ConsoleAPI(_) + CachedConsoleMessage::ConsoleLog(_) if message_types.contains(CachedConsoleMessageTypes::CONSOLE_API) => { true @@ -506,31 +477,3 @@ impl Actor for ConsoleActor { }) } } - -#[derive(Serialize)] -struct ConsoleAPICall { - from: String, - #[serde(rename = "type")] - type_: String, - message: ConsoleMsg, -} - -#[derive(Serialize)] -#[serde(rename_all = "camelCase")] -struct ConsoleMsg { - level: String, - timestamp: u64, - arguments: Vec, - filename: String, - line_number: usize, - column_number: usize, -} - -#[derive(Serialize)] -#[serde(rename_all = "camelCase")] -struct PageErrorMsg { - from: String, - #[serde(rename = "type")] - type_: String, - page_error: PageError, -} diff --git a/components/devtools/actors/object.rs b/components/devtools/actors/object.rs index 4091596f0b2..5a7f9c581bd 100644 --- a/components/devtools/actors/object.rs +++ b/components/devtools/actors/object.rs @@ -26,6 +26,7 @@ impl Actor for ObjectActor { _: &mut TcpStream, _: StreamId, ) -> Result { + // TODO: Handle enumSymbols for console object inspection Ok(ActorMessageStatus::Ignored) } } diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index b7c6fdc54a1..caa55bf425b 100644 --- a/components/devtools/lib.rs +++ b/components/devtools/lib.rs @@ -191,13 +191,11 @@ fn run_server( let actors = registry.create_shareable(); - let mut accepted_connections: Vec = Vec::new(); - - let mut browsing_contexts: HashMap = HashMap::new(); - let mut pipelines: HashMap = HashMap::new(); - let mut actor_requests: HashMap = HashMap::new(); - - let mut actor_workers: HashMap = HashMap::new(); + let mut accepted_connections = HashMap::new(); + let mut browsing_contexts: HashMap<_, String> = HashMap::new(); + let mut pipelines = HashMap::new(); + let mut actor_requests = HashMap::new(); + let mut actor_workers = HashMap::new(); /// Process the input from a single devtools client until EOF. fn handle_client(actors: Arc>, mut stream: TcpStream, id: StreamId) { @@ -289,6 +287,7 @@ fn run_server( pipelines: &mut HashMap, actor_workers: &mut HashMap, page_info: DevtoolsPageInfo, + connections: &HashMap, ) { let mut actors = actors.lock().unwrap(); @@ -324,10 +323,9 @@ fn run_server( Root::DedicatedWorker(worker_name) } else { pipelines.insert(pipeline, browsing_context); - Root::BrowsingContext( - if let Some(actor) = browsing_contexts.get(&browsing_context) { - actor.to_owned() - } else { + let name = browsing_contexts + .entry(browsing_context) + .or_insert_with(|| { let browsing_context_actor = BrowsingContextActor::new( console_name.clone(), browsing_context, @@ -337,11 +335,18 @@ fn run_server( &mut actors, ); let name = browsing_context_actor.name(); - browsing_contexts.insert(browsing_context, name.clone()); actors.register(Box::new(browsing_context_actor)); name - }, - ) + }); + + // Add existing streams to the new browsing context + let browsing_context = actors.find::(&name); + let mut streams = browsing_context.streams.borrow_mut(); + for (id, stream) in connections { + streams.insert(*id, stream.try_clone().unwrap()); + } + + Root::BrowsingContext(name.clone()) }; let console = ConsoleActor { @@ -609,7 +614,15 @@ fn run_server( let actors = actors.clone(); let id = next_id; next_id = StreamId(id.0 + 1); - accepted_connections.push(stream.try_clone().unwrap()); + accepted_connections.insert(id, stream.try_clone().unwrap()); + + // Inform every browsing context of the new stream + for name in browsing_contexts.values() { + let actors = actors.lock().unwrap(); + let browsing_context = actors.find::(name); + let mut streams = browsing_context.streams.borrow_mut(); + streams.insert(id, stream.try_clone().unwrap()); + } thread::Builder::new() .name("DevtoolsClientHandler".to_owned()) .spawn(move || handle_client(actors, stream.try_clone().unwrap(), id)) @@ -641,6 +654,7 @@ fn run_server( &mut pipelines, &mut actor_workers, pageinfo, + &accepted_connections, ), DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::Navigate( browsing_context, @@ -698,7 +712,7 @@ fn run_server( )) => { // copy the accepted_connections vector let mut connections = Vec::::new(); - for stream in &accepted_connections { + for stream in accepted_connections.values() { connections.push(stream.try_clone().unwrap()); } @@ -721,7 +735,7 @@ fn run_server( DevtoolsControlMsg::FromChrome(ChromeToDevtoolsControlMsg::ServerExitMsg) => break, } } - for connection in &mut accepted_connections { + for connection in accepted_connections.values_mut() { let _ = connection.shutdown(Shutdown::Both); } } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index bd1ab6fd178..0efa0af851e 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -11,7 +11,7 @@ use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::thread::JoinHandle; -use std::time::Instant; +use std::time::{Instant, SystemTime, UNIX_EPOCH}; use std::{mem, ptr}; use base::id::{ @@ -2305,7 +2305,10 @@ impl GlobalScope { line_number: 0, column_number: 0, category: "script".to_string(), - time_stamp: 0, //TODO + time_stamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64, error: false, warning: true, exception: true, @@ -2493,7 +2496,10 @@ impl GlobalScope { line_number: error_info.lineno, column_number: error_info.column, category: "script".to_string(), - time_stamp: 0, //TODO + time_stamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64, error: true, warning: false, exception: true, diff --git a/components/shared/devtools/lib.rs b/components/shared/devtools/lib.rs index 4e9d92acab4..05e68e35838 100644 --- a/components/shared/devtools/lib.rs +++ b/components/shared/devtools/lib.rs @@ -282,23 +282,21 @@ pub struct PageError { pub private: bool, } -#[derive(Debug, Deserialize, Serialize)] -pub struct ConsoleAPI { - #[serde(rename = "_type")] - pub type_: String, +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct ConsoleLog { pub level: String, pub filename: String, pub line_number: u32, - pub function_name: String, + pub column_number: u32, pub time_stamp: u64, - pub private: bool, pub arguments: Vec, + // pub stacktrace: Vec<...>, } #[derive(Debug, Deserialize, Serialize)] pub enum CachedConsoleMessage { PageError(PageError), - ConsoleAPI(ConsoleAPI), + ConsoleLog(ConsoleLog), } #[derive(Debug, PartialEq)]