From b252f238d1c66470a2a0d759f3df3a40504fd0e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Thu, 2 Jan 2025 19:47:52 +0100 Subject: [PATCH] Support syntax highlighting of arguments in the devtools console (#34810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement Builder struct for console messages Signed-off-by: Simon Wülker * Support integer arguments for console methods Signed-off-by: Simon Wülker * Support floating point arguments to console methods in devtools Signed-off-by: Simon Wülker * Fix warnings Signed-off-by: Simon Wülker * Tidy Signed-off-by: Simon Wülker --------- Signed-off-by: Simon Wülker --- components/devtools/actors/console.rs | 31 +---- components/devtools/lib.rs | 22 ++-- components/script/dom/console.rs | 167 ++++++++++++++++++-------- components/shared/devtools/lib.rs | 117 +++++++++++++++++- 4 files changed, 242 insertions(+), 95 deletions(-) diff --git a/components/devtools/actors/console.rs b/components/devtools/actors/console.rs index bf949d9219c..dd2b7b57a87 100644 --- a/components/devtools/actors/console.rs +++ b/components/devtools/actors/console.rs @@ -17,7 +17,7 @@ use devtools_traits::EvaluateJSReply::{ }; use devtools_traits::{ CachedConsoleMessage, CachedConsoleMessageTypes, ConsoleLog, ConsoleMessage, - DevtoolScriptControlMsg, LogLevel, PageError, + DevtoolScriptControlMsg, PageError, }; use ipc_channel::ipc::{self, IpcSender}; use log::debug; @@ -272,40 +272,17 @@ impl ConsoleActor { id: UniqueId, registry: &ActorRegistry, ) { - let level = match console_message.log_level { - LogLevel::Debug => "debug", - LogLevel::Info => "info", - LogLevel::Warn => "warn", - LogLevel::Error => "error", - LogLevel::Clear => "clear", - LogLevel::Trace => "trace", - LogLevel::Log => "log", - } - .to_owned(); - - let console_api = ConsoleLog { - level, - filename: console_message.filename, - 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], - stacktrace: console_message.stacktrace, - }; - + let log_message: ConsoleLog = console_message.into(); self.cached_events .borrow_mut() .entry(id.clone()) .or_default() - .push(CachedConsoleMessage::ConsoleLog(console_api.clone())); + .push(CachedConsoleMessage::ConsoleLog(log_message.clone())); if id == self.current_unique_id(registry) { if let Root::BrowsingContext(bc) = &self.root { registry .find::(bc) - .resource_available(console_api, "console-message".into()) + .resource_available(log_message, "console-message".into()) }; } } diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index f6d5576a355..09881b64423 100644 --- a/components/devtools/lib.rs +++ b/components/devtools/lib.rs @@ -22,8 +22,8 @@ use std::thread; use base::id::{BrowsingContextId, PipelineId}; use crossbeam_channel::{unbounded, Receiver, Sender}; use devtools_traits::{ - ChromeToDevtoolsControlMsg, ConsoleMessage, DevtoolScriptControlMsg, DevtoolsControlMsg, - DevtoolsPageInfo, LogLevel, NavigationState, NetworkEvent, PageError, + ChromeToDevtoolsControlMsg, ConsoleMessage, ConsoleMessageBuilder, DevtoolScriptControlMsg, + DevtoolsControlMsg, DevtoolsPageInfo, LogLevel, NavigationState, NetworkEvent, PageError, ScriptToDevtoolsControlMsg, WorkerId, }; use embedder_traits::{EmbedderMsg, EmbedderProxy, PromptDefinition, PromptOrigin, PromptResult}; @@ -688,19 +688,19 @@ fn run_server( id, css_error, )) => { - let console_message = ConsoleMessage { - message: css_error.msg, - log_level: LogLevel::Warn, - filename: css_error.filename, - line_number: css_error.line as usize, - column_number: css_error.column as usize, - stacktrace: vec![], - }; + let mut console_message = ConsoleMessageBuilder::new( + LogLevel::Warn, + css_error.filename, + css_error.line, + css_error.column, + ); + console_message.add_argument(css_error.msg.into()); + handle_console_message( actors.clone(), id, None, - console_message, + console_message.finish(), &browsing_contexts, &actor_workers, &pipelines, diff --git a/components/script/dom/console.rs b/components/script/dom/console.rs index 9e1c9e56abe..b45d65d210d 100644 --- a/components/script/dom/console.rs +++ b/components/script/dom/console.rs @@ -5,7 +5,10 @@ use std::convert::TryFrom; use std::{io, ptr}; -use devtools_traits::{ConsoleMessage, LogLevel, ScriptToDevtoolsControlMsg, StackFrame}; +use devtools_traits::{ + ConsoleMessage, ConsoleMessageArgument, ConsoleMessageBuilder, LogLevel, + ScriptToDevtoolsControlMsg, StackFrame, +}; use js::jsapi::{self, ESClass, PropertyDescriptor}; use js::jsval::UndefinedValue; use js::rust::wrappers::{ @@ -32,34 +35,60 @@ pub struct Console; impl Console { #[allow(unsafe_code)] - fn send_to_devtools(global: &GlobalScope, level: LogLevel, message: String) { - if let Some(chan) = global.devtools_chan() { - let caller = - unsafe { describe_scripted_caller(*GlobalScope::get_cx()) }.unwrap_or_default(); + fn build_message(level: LogLevel) -> ConsoleMessageBuilder { + let cx = GlobalScope::get_cx(); + let caller = unsafe { describe_scripted_caller(*cx) }.unwrap_or_default(); - let console_message = ConsoleMessage { - message, - log_level: level, - filename: caller.filename, - line_number: caller.line as usize, - column_number: caller.col as usize, - stacktrace: get_js_stack(*GlobalScope::get_cx()), - }; + ConsoleMessageBuilder::new(level, caller.filename, caller.line, caller.col) + } + + /// Helper to send a message that only consists of a single string to the devtools + fn send_string_message(global: &GlobalScope, level: LogLevel, message: String) { + let mut builder = Self::build_message(level); + builder.add_argument(message.into()); + let log_message = builder.finish(); + + Self::send_to_devtools(global, log_message); + } + + fn method( + global: &GlobalScope, + level: LogLevel, + messages: Vec, + include_stacktrace: IncludeStackTrace, + ) { + let cx = GlobalScope::get_cx(); + + let mut log: ConsoleMessageBuilder = Console::build_message(level); + for message in &messages { + log.add_argument(console_argument_from_handle_value(cx, *message)); + } + + if include_stacktrace == IncludeStackTrace::Yes { + log.attach_stack_trace(get_js_stack(*GlobalScope::get_cx())); + } + + Console::send_to_devtools(global, log.finish()); + + // Also log messages to stdout + console_messages(global, messages) + } + + fn send_to_devtools(global: &GlobalScope, message: ConsoleMessage) { + if let Some(chan) = global.devtools_chan() { let worker_id = global .downcast::() .map(|worker| worker.get_worker_id()); - let devtools_message = ScriptToDevtoolsControlMsg::ConsoleAPI( - global.pipeline_id(), - console_message, - worker_id, - ); + let devtools_message = + ScriptToDevtoolsControlMsg::ConsoleAPI(global.pipeline_id(), message, worker_id); chan.send(devtools_message).unwrap(); } } // Directly logs a DOMString, without processing the message pub fn internal_warn(global: &GlobalScope, message: DOMString) { - console_message(global, message, LogLevel::Warn) + Console::send_string_message(global, LogLevel::Warn, String::from(message.clone())); + console_message(global, message); } } @@ -89,6 +118,32 @@ unsafe fn handle_value_to_string(cx: *mut jsapi::JSContext, value: HandleValue) } } +#[allow(unsafe_code)] +fn console_argument_from_handle_value( + cx: JSContext, + handle_value: HandleValue, +) -> ConsoleMessageArgument { + if handle_value.is_string() { + let js_string = ptr::NonNull::new(handle_value.to_string()).unwrap(); + let dom_string = unsafe { jsstring_to_str(*cx, js_string) }; + return ConsoleMessageArgument::String(dom_string.into()); + } + + if handle_value.is_int32() { + let integer = handle_value.to_int32(); + return ConsoleMessageArgument::Integer(integer); + } + + if handle_value.is_number() { + let number = handle_value.to_number(); + return ConsoleMessageArgument::Number(number); + } + + // FIXME: Handle more complex argument types here + let stringified_value = stringify_handle_value(handle_value); + ConsoleMessageArgument::String(stringified_value.into()) +} + #[allow(unsafe_code)] fn stringify_handle_value(message: HandleValue) -> DOMString { let cx = *GlobalScope::get_cx(); @@ -208,110 +263,116 @@ fn stringify_handle_value(message: HandleValue) -> DOMString { } } -fn stringify_handle_values(messages: Vec) -> DOMString { +fn stringify_handle_values(messages: &[HandleValue]) -> DOMString { DOMString::from(itertools::join( - messages.into_iter().map(stringify_handle_value), + messages.iter().copied().map(stringify_handle_value), " ", )) } -fn console_messages(global: &GlobalScope, messages: Vec, level: LogLevel) { - let message = stringify_handle_values(messages); - console_message(global, message, level) +fn console_messages(global: &GlobalScope, messages: Vec) { + let message = stringify_handle_values(&messages); + console_message(global, message) } -fn console_message(global: &GlobalScope, message: DOMString, level: LogLevel) { +fn console_message(global: &GlobalScope, message: DOMString) { with_stderr_lock(move || { let prefix = global.current_group_label().unwrap_or_default(); let message = format!("{}{}", prefix, message); println!("{}", message); - Console::send_to_devtools(global, level, message); }) } +#[derive(Debug, Eq, PartialEq)] +enum IncludeStackTrace { + Yes, + No, +} + impl consoleMethods for Console { // https://developer.mozilla.org/en-US/docs/Web/API/Console/log fn Log(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Log) + Console::method(global, LogLevel::Log, messages, IncludeStackTrace::No); } // https://developer.mozilla.org/en-US/docs/Web/API/Console/clear fn Clear(global: &GlobalScope) { - let message: Vec = Vec::new(); - console_messages(global, message, LogLevel::Clear) + let message = Console::build_message(LogLevel::Clear).finish(); + Console::send_to_devtools(global, message); } // https://developer.mozilla.org/en-US/docs/Web/API/Console fn Debug(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Debug) + Console::method(global, LogLevel::Debug, messages, IncludeStackTrace::No); } // https://developer.mozilla.org/en-US/docs/Web/API/Console/info fn Info(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Info) + Console::method(global, LogLevel::Info, messages, IncludeStackTrace::No); } // https://developer.mozilla.org/en-US/docs/Web/API/Console/warn fn Warn(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Warn) + Console::method(global, LogLevel::Warn, messages, IncludeStackTrace::No); } // https://developer.mozilla.org/en-US/docs/Web/API/Console/error fn Error(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Error) + Console::method(global, LogLevel::Error, messages, IncludeStackTrace::No); } /// fn Trace(_cx: JSContext, global: &GlobalScope, messages: Vec) { - console_messages(global, messages, LogLevel::Trace) + Console::method(global, LogLevel::Trace, messages, IncludeStackTrace::Yes); } // https://developer.mozilla.org/en-US/docs/Web/API/Console/assert fn Assert(_cx: JSContext, global: &GlobalScope, condition: bool, messages: Vec) { if !condition { - let message = DOMString::from(format!( - "Assertion failed: {}", - stringify_handle_values(messages) - )); - console_message(global, message, LogLevel::Error); + let message = format!("Assertion failed: {}", stringify_handle_values(&messages)); + + Console::send_string_message(global, LogLevel::Log, message.clone()); + console_message(global, DOMString::from(message)); } } // https://console.spec.whatwg.org/#time fn Time(global: &GlobalScope, label: DOMString) { if let Ok(()) = global.time(label.clone()) { - let message = DOMString::from(format!("{label}: timer started")); - console_message(global, message, LogLevel::Log); + let message = format!("{label}: timer started"); + Console::send_string_message(global, LogLevel::Log, message.clone()); + console_message(global, DOMString::from(message)); } } // https://console.spec.whatwg.org/#timelog fn TimeLog(_cx: JSContext, global: &GlobalScope, label: DOMString, data: Vec) { if let Ok(delta) = global.time_log(&label) { - let message = DOMString::from(format!( - "{label}: {delta}ms {}", - stringify_handle_values(data) - )); - console_message(global, message, LogLevel::Log); + let message = format!("{label}: {delta}ms {}", stringify_handle_values(&data)); + + Console::send_string_message(global, LogLevel::Log, message.clone()); + console_message(global, DOMString::from(message)); } } // https://console.spec.whatwg.org/#timeend fn TimeEnd(global: &GlobalScope, label: DOMString) { if let Ok(delta) = global.time_end(&label) { - let message = DOMString::from(format!("{label}: {delta}ms")); - console_message(global, message, LogLevel::Log); + let message = format!("{label}: {delta}ms"); + + Console::send_string_message(global, LogLevel::Log, message.clone()); + console_message(global, DOMString::from(message)); } } // https://console.spec.whatwg.org/#group fn Group(_cx: JSContext, global: &GlobalScope, messages: Vec) { - global.push_console_group(stringify_handle_values(messages)); + global.push_console_group(stringify_handle_values(&messages)); } // https://console.spec.whatwg.org/#groupcollapsed fn GroupCollapsed(_cx: JSContext, global: &GlobalScope, messages: Vec) { - global.push_console_group(stringify_handle_values(messages)); + global.push_console_group(stringify_handle_values(&messages)); } // https://console.spec.whatwg.org/#groupend @@ -322,8 +383,10 @@ impl consoleMethods for Console { /// fn Count(global: &GlobalScope, label: DOMString) { let count = global.increment_console_count(&label); - let message = DOMString::from(format!("{label}: {count}")); - console_message(global, message, LogLevel::Log); + let message = format!("{label}: {count}"); + + Console::send_string_message(global, LogLevel::Log, message.clone()); + console_message(global, DOMString::from(message)); } /// diff --git a/components/shared/devtools/lib.rs b/components/shared/devtools/lib.rs index 4f26ba7c236..6bfc275dc44 100644 --- a/components/shared/devtools/lib.rs +++ b/components/shared/devtools/lib.rs @@ -12,7 +12,7 @@ use std::collections::HashMap; use std::net::TcpStream; -use std::time::{Duration, SystemTime}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use base::cross_process_instant::CrossProcessInstant; use base::id::{BrowsingContextId, PipelineId}; @@ -283,15 +283,23 @@ pub enum LogLevel { Trace, } +/// A console message as it is sent from script to the constellation #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ConsoleMessage { - pub message: String, pub log_level: LogLevel, pub filename: String, pub line_number: usize, pub column_number: usize, - pub stacktrace: Vec, + pub arguments: Vec, + pub stacktrace: Option>, +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +pub enum ConsoleMessageArgument { + String(String), + Integer(i32), + Number(f64), } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -335,6 +343,7 @@ pub struct PageError { pub private: bool, } +/// Represents a console message as it is sent to the devtools #[derive(Clone, Debug, Deserialize, Serialize)] pub struct ConsoleLog { pub level: String, @@ -342,8 +351,39 @@ pub struct ConsoleLog { pub line_number: u32, pub column_number: u32, pub time_stamp: u64, - pub arguments: Vec, - pub stacktrace: Vec, + pub arguments: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub stacktrace: Option>, +} + +impl From for ConsoleLog { + fn from(value: ConsoleMessage) -> Self { + let level = match value.log_level { + LogLevel::Debug => "debug", + LogLevel::Info => "info", + LogLevel::Warn => "warn", + LogLevel::Error => "error", + LogLevel::Clear => "clear", + LogLevel::Trace => "trace", + LogLevel::Log => "log", + } + .to_owned(); + + let time_stamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64; + + Self { + level, + filename: value.filename, + line_number: value.line_number as u32, + column_number: value.column_number as u32, + time_stamp, + arguments: value.arguments.into_iter().map(|arg| arg.into()).collect(), + stacktrace: value.stacktrace, + } + } } #[derive(Debug, Deserialize, Serialize)] @@ -412,3 +452,70 @@ pub struct CssDatabaseProperty { pub supports: Vec, pub subproperties: Vec, } + +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(untagged)] +pub enum ConsoleArgument { + String(String), + Integer(i32), + Number(f64), +} + +impl From for ConsoleArgument { + fn from(value: ConsoleMessageArgument) -> Self { + match value { + ConsoleMessageArgument::String(string) => Self::String(string), + ConsoleMessageArgument::Integer(integer) => Self::Integer(integer), + ConsoleMessageArgument::Number(number) => Self::Number(number), + } + } +} + +impl From for ConsoleMessageArgument { + fn from(value: String) -> Self { + Self::String(value) + } +} + +pub struct ConsoleMessageBuilder { + level: LogLevel, + filename: String, + line_number: u32, + column_number: u32, + arguments: Vec, + stack_trace: Option>, +} + +impl ConsoleMessageBuilder { + pub fn new(level: LogLevel, filename: String, line_number: u32, column_number: u32) -> Self { + Self { + level, + filename, + line_number, + column_number, + arguments: vec![], + stack_trace: None, + } + } + + pub fn attach_stack_trace(&mut self, stack_trace: Vec) -> &mut Self { + self.stack_trace = Some(stack_trace); + self + } + + pub fn add_argument(&mut self, argument: ConsoleMessageArgument) -> &mut Self { + self.arguments.push(argument); + self + } + + pub fn finish(self) -> ConsoleMessage { + ConsoleMessage { + log_level: self.level, + filename: self.filename, + line_number: self.line_number as usize, + column_number: self.column_number as usize, + arguments: self.arguments, + stacktrace: self.stack_trace, + } + } +}