Support syntax highlighting of arguments in the devtools console (#34810)

* Implement Builder struct for console messages

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Support integer arguments for console methods

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Support floating point arguments to console methods in devtools

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Fix warnings

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Tidy

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

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 {
+        let log_message: ConsoleLog = console_message.into();
-            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,
-        };
-
         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::<BrowsingContextActor>(bc)
-                    .resource_available(console_api, "console-message".into())
+                    .resource_available(log_message, "console-message".into())
             };
         }
     }

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,
+    ChromeToDevtoolsControlMsg, ConsoleMessage, ConsoleMessageBuilder, DevtoolScriptControlMsg,
-    DevtoolsPageInfo, LogLevel, NavigationState, NetworkEvent, PageError,
+    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 {
+                let mut console_message = ConsoleMessageBuilder::new(
-                    message: css_error.msg,
+                    LogLevel::Warn,
-                    log_level: LogLevel::Warn,
+                    css_error.filename,
-                    filename: css_error.filename,
+                    css_error.line,
-                    line_number: css_error.line as usize,
+                    css_error.column,
-                    column_number: css_error.column as usize,
+                );
-                    stacktrace: vec![],
+                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,

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) {
+    fn build_message(level: LogLevel) -> ConsoleMessageBuilder {
-        if let Some(chan) = global.devtools_chan() {
+        let cx = GlobalScope::get_cx();
-            let caller =
+        let caller = unsafe { describe_scripted_caller(*cx) }.unwrap_or_default();
-                unsafe { describe_scripted_caller(*GlobalScope::get_cx()) }.unwrap_or_default();
 
-            let console_message = ConsoleMessage {
+        ConsoleMessageBuilder::new(level, caller.filename, caller.line, caller.col)
-                message,
+    }
-                log_level: level,
+
-                filename: caller.filename,
+    /// Helper to send a message that only consists of a single string to the devtools
-                line_number: caller.line as usize,
+    fn send_string_message(global: &GlobalScope, level: LogLevel, message: String) {
-                column_number: caller.col as usize,
+        let mut builder = Self::build_message(level);
-                stacktrace: get_js_stack(*GlobalScope::get_cx()),
+        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<HandleValue>,
+        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::<WorkerGlobalScope>()
                 .map(|worker| worker.get_worker_id());
-            let devtools_message = ScriptToDevtoolsControlMsg::ConsoleAPI(
+            let devtools_message =
-                global.pipeline_id(),
+                ScriptToDevtoolsControlMsg::ConsoleAPI(global.pipeline_id(), message, worker_id);
-                console_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<HandleValue>) -> 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<HandleValue>, level: LogLevel) {
+fn console_messages(global: &GlobalScope, messages: Vec<HandleValue>) {
-    let message = stringify_handle_values(messages);
+    let message = stringify_handle_values(&messages);
-    console_message(global, message, level)
+    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<crate::DomTypeHolder> for Console {
     // https://developer.mozilla.org/en-US/docs/Web/API/Console/log
     fn Log(_cx: JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
-        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<HandleValue> = Vec::new();
+        let message = Console::build_message(LogLevel::Clear).finish();
-        console_messages(global, message, LogLevel::Clear)
+        Console::send_to_devtools(global, message);
     }
 
     // https://developer.mozilla.org/en-US/docs/Web/API/Console
     fn Debug(_cx: JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
-        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<HandleValue>) {
-        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<HandleValue>) {
-        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<HandleValue>) {
-        console_messages(global, messages, LogLevel::Error)
+        Console::method(global, LogLevel::Error, messages, IncludeStackTrace::No);
     }
 
     /// <https://console.spec.whatwg.org/#trace>
     fn Trace(_cx: JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
-        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<HandleValue>) {
         if !condition {
-            let message = DOMString::from(format!(
+            let message = format!("Assertion failed: {}", stringify_handle_values(&messages));
-                "Assertion failed: {}",
+
-                stringify_handle_values(messages)
+            Console::send_string_message(global, LogLevel::Log, message.clone());
-            ));
+            console_message(global, DOMString::from(message));
-            console_message(global, message, LogLevel::Error);
         }
     }
 
     // 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"));
+            let message = format!("{label}: timer started");
-            console_message(global, message, LogLevel::Log);
+            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<HandleValue>) {
         if let Ok(delta) = global.time_log(&label) {
-            let message = DOMString::from(format!(
+            let message = format!("{label}: {delta}ms {}", stringify_handle_values(&data));
-                "{label}: {delta}ms {}",
+
-                stringify_handle_values(data)
+            Console::send_string_message(global, LogLevel::Log, message.clone());
-            ));
+            console_message(global, DOMString::from(message));
-            console_message(global, message, LogLevel::Log);
         }
     }
 
     // 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"));
+            let message = format!("{label}: {delta}ms");
-            console_message(global, message, LogLevel::Log);
+
+            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<HandleValue>) {
-        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<HandleValue>) {
-        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<crate::DomTypeHolder> for Console {
     /// <https://console.spec.whatwg.org/#count>
     fn Count(global: &GlobalScope, label: DOMString) {
         let count = global.increment_console_count(&label);
-        let message = DOMString::from(format!("{label}: {count}"));
+        let message = format!("{label}: {count}");
-        console_message(global, message, LogLevel::Log);
+
+        Console::send_string_message(global, LogLevel::Log, message.clone());
+        console_message(global, DOMString::from(message));
     }
 
     /// <https://console.spec.whatwg.org/#countreset>

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<StackFrame>,
+    pub arguments: Vec<ConsoleMessageArgument>,
+    pub stacktrace: Option<Vec<StackFrame>>,
+}
+
+#[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<String>,
+    pub arguments: Vec<ConsoleArgument>,
-    pub stacktrace: Vec<StackFrame>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub stacktrace: Option<Vec<StackFrame>>,
+}
+
+impl From<ConsoleMessage> 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<String>,
     pub subproperties: Vec<String>,
 }
+
+#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(untagged)]
+pub enum ConsoleArgument {
+    String(String),
+    Integer(i32),
+    Number(f64),
+}
+
+impl From<ConsoleMessageArgument> 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<String> 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<ConsoleMessageArgument>,
+    stack_trace: Option<Vec<StackFrame>>,
+}
+
+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<StackFrame>) -> &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,
+        }
+    }
+}