eliott/servo
1
0
Fork 0
mirror of https://github.com/servo/servo synced 2026-05-05 06:32:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

devtools: Remove unsupported serde annotations from shared/devtools.rs (#42219)

Browse Source 
There were some `#[serde(untagged)]` and `#[serde(skip*)]` annotations
in the shared devtools types. These are needed for correct JSON
serialization when sending messages to Firefox, but they fail in
multiprocess mode since we are sharing them through the IPC channel with
bincode, which doesn't support certain serde tags.

The workaround is to keep all shared types free from problematic
annotations, and defining wrapping types that are only used in the
devtools crate.

One instance are console messages and page errors. They have been moved
to `console.rs`, with `shared/devtools` only keeping the parts that are
needed for communication between threads.

The other instance are auto margins. Now the final message is built in
`page_style.rs`.

Apologies since both of them were introduced by me, I wasn't aware that
we were limited by bincode in multiprocess mode. A warning has been
added to `shared/devtools` listing the problematic annotations that
shouldn't be used in this file.

Testing: `mach test-devtools` and manual testing using `--multiprocess`.
Fixes: #42170

Signed-off-by: eri <eri@igalia.com>
This commit is contained in:
eri
2026-01-28 16:48:59 +01:00
committed by GitHub
parent 6b12ecf26b
commit 6e781aaf74
8 changed files with 265 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files

127
components/devtools/actors/console.rs
View File

@@ -9,7 +9,6 @@
use std::collections::HashMap;
use std::net::TcpStream;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::{SystemTime, UNIX_EPOCH};
use atomic_refcell::AtomicRefCell;
use base::generic_channel::{self, GenericSender};
@@ -17,7 +16,10 @@ use base::id::TEST_PIPELINE_ID;
use devtools_traits::EvaluateJSReply::{
ActorValue, BooleanValue, NullValue, NumberValue, StringValue, VoidValue,
};
use devtools_traits::{ConsoleResource, DevtoolScriptControlMsg};
use devtools_traits::{
ConsoleArgument, ConsoleMessage, ConsoleMessageFields, DevtoolScriptControlMsg, PageError,
StackFrame, get_time_stamp,
};
use serde::Serialize;
use serde_json::{self, Map, Number, Value};
use uuid::Uuid;
@@ -30,6 +32,105 @@ use crate::protocol::{ClientRequest, JsonPacketStream};
use crate::resource::{ResourceArrayType, ResourceAvailable};
use crate::{EmptyReplyMsg, StreamId, UniqueId};
#[derive(Clone, Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct DevtoolsConsoleMessage {
#[serde(flatten)]
fields: ConsoleMessageFields,
arguments: Vec<Value>,
#[serde(skip_serializing_if = "Option::is_none")]
stacktrace: Option<Vec<StackFrame>>,
// Not implemented in Servo
// inner_window_id
// source_id
}
impl From<ConsoleMessage> for DevtoolsConsoleMessage {
fn from(console_message: ConsoleMessage) -> Self {
Self {
fields: console_message.fields,
arguments: console_message
.arguments
.into_iter()
.map(console_argument_to_value)
.collect(),
stacktrace: console_message.stacktrace,
}
}
}
fn console_argument_to_value(argument: ConsoleArgument) -> Value {
match argument {
ConsoleArgument::String(value) => Value::String(value),
ConsoleArgument::Integer(value) => Value::Number(value.into()),
ConsoleArgument::Number(value) => {
Number::from_f64(value).map(Value::from).unwrap_or_default()
},
}
}
#[derive(Clone, Serialize)]
#[serde(rename_all = "camelCase")]
struct DevtoolsPageError {
#[serde(flatten)]
page_error: PageError,
category: String,
error: bool,
warning: bool,
info: bool,
private: bool,
#[serde(skip_serializing_if = "Option::is_none")]
stacktrace: Option<Vec<StackFrame>>,
// Not implemented in Servo
// inner_window_id
// source_id
// has_exception
// exception
}
impl From<PageError> for DevtoolsPageError {
fn from(page_error: PageError) -> Self {
Self {
page_error,
category: "script".to_string(),
error: true,
warning: false,
info: false,
private: false,
stacktrace: None,
}
}
}
#[derive(Clone, Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct PageErrorWrapper {
page_error: DevtoolsPageError,
}
impl From<PageError> for PageErrorWrapper {
fn from(page_error: PageError) -> Self {
Self {
page_error: page_error.into(),
}
}
}
#[derive(Clone, Serialize)]
#[serde(untagged)]
pub(crate) enum ConsoleResource {
ConsoleMessage(DevtoolsConsoleMessage),
PageError(PageErrorWrapper),
}
impl ConsoleResource {
pub fn resource_type(&self) -> String {
match self {
ConsoleResource::ConsoleMessage(_) => "console-message".into(),
ConsoleResource::PageError(_) => "error-message".into(),
}
}
}
#[derive(Serialize)]
pub struct ConsoleClearMessage {
pub level: String,
@@ -90,18 +191,27 @@ pub(crate) enum Root {
}
pub(crate) struct ConsoleActor {
pub name: String,
pub root: Root,
pub cached_events: AtomicRefCell<HashMap<UniqueId, Vec<ConsoleResource>>>,
name: String,
root: Root,
cached_events: AtomicRefCell<HashMap<UniqueId, Vec<ConsoleResource>>>,
/// Used to control whether to send resource array messages from
/// `handle_console_resource`. It starts being false, and it only gets
/// activated after the client requests `console-message` or `error-message`
/// resources for the first time. Otherwise we would be sending messages
/// before the client is ready to receive them.
pub client_ready_to_receive_messages: AtomicBool,
client_ready_to_receive_messages: AtomicBool,
}
impl ConsoleActor {
pub fn new(name: String, root: Root) -> Self {
Self {
name,
root,
cached_events: Default::default(),
client_ready_to_receive_messages: false.into(),
}
}
fn script_chan(&self, registry: &ActorRegistry) -> GenericSender<DevtoolScriptControlMsg> {
match &self.root {
Root::BrowsingContext(browsing_context) => registry
@@ -204,10 +314,7 @@ impl ConsoleActor {
from: self.name(),
input,
result,
timestamp: SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or_default()
.as_millis() as u64,
timestamp: get_time_stamp(),
exception: Value::Null,
exception_message: Value::Null,
helper_result: Value::Null,

31
components/devtools/actors/inspector/page_style.rs
View File

@@ -12,7 +12,7 @@ use std::iter::once;
use base::generic_channel::{self, GenericSender};
use base::id::PipelineId;
use devtools_traits::DevtoolScriptControlMsg::{GetLayout, GetSelectors};
use devtools_traits::{ComputedNodeLayout, DevtoolScriptControlMsg};
use devtools_traits::{AutoMargins, ComputedNodeLayout, DevtoolScriptControlMsg};
use serde::Serialize;
use serde_json::{self, Map, Value};
@@ -45,11 +45,37 @@ struct AppliedEntry {
inherited: Option<String>,
}
#[derive(Serialize)]
struct DevtoolsAutoMargins {
#[serde(skip_serializing_if = "Option::is_none")]
top: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
right: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
bottom: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
left: Option<String>,
}
impl From<AutoMargins> for DevtoolsAutoMargins {
fn from(auto_margins: AutoMargins) -> Self {
const AUTO: &str = "auto";
Self {
top: auto_margins.top.then_some(AUTO.into()),
right: auto_margins.right.then_some(AUTO.into()),
bottom: auto_margins.bottom.then_some(AUTO.into()),
left: auto_margins.left.then_some(AUTO.into()),
}
}
}
#[derive(Serialize)]
struct GetLayoutReply {
from: String,
#[serde(flatten)]
layout: ComputedNodeLayout,
#[serde(rename = "autoMargins")]
auto_margins: DevtoolsAutoMargins,
}
#[derive(Serialize)]
@@ -251,13 +277,14 @@ impl PageStyleActor {
tx,
))
.map_err(|_| ActorError::Internal)?;
let layout = rx
let (layout, auto_margins) = rx
.recv()
.map_err(|_| ActorError::Internal)?
.ok_or(ActorError::Internal)?;
request.reply_final(&GetLayoutReply {
from: self.name(),
layout,
auto_margins: auto_margins.into(),
})
}

15
components/devtools/actors/watcher.rs
View File

@@ -12,9 +12,9 @@
use std::collections::HashMap;
use std::net::TcpStream;
use std::time::{SystemTime, UNIX_EPOCH};
use base::id::BrowsingContextId;
use devtools_traits::get_time_stamp;
use log::warn;
use serde::Serialize;
use serde_json::{Map, Value};
@@ -196,7 +196,7 @@ pub(crate) struct WillNavigateMessage {
browsing_context_id: u32,
inner_window_id: u32,
name: String,
time: u128,
time: u64,
is_frame_switching: bool,
#[serde(rename = "newURI")]
new_uri: ServoUrl,
@@ -293,11 +293,7 @@ impl Actor for WatcherActor {
has_native_console_api: None,
name: name.into(),
new_uri: None,
time: SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or_default()
.as_millis()
as u64,
time: get_time_stamp(),
title: Some(target.title.borrow().clone()),
url: Some(target.url.borrow().clone()),
};
@@ -454,10 +450,7 @@ impl WatcherActor {
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(),
time: get_time_stamp(),
is_frame_switching: false, // TODO: Implement frame switching
new_uri: url,
};

35
components/devtools/lib.rs
View File

@@ -22,9 +22,9 @@ use base::generic_channel::{self, GenericSender};
use base::id::{BrowsingContextId, PipelineId, WebViewId};
use crossbeam_channel::{Receiver, Sender, unbounded};
use devtools_traits::{
ChromeToDevtoolsControlMsg, ConsoleLogLevel, ConsoleMessageBuilder, ConsoleResource,
ChromeToDevtoolsControlMsg, ConsoleLogLevel, ConsoleMessage, ConsoleMessageFields,
DevtoolScriptControlMsg, DevtoolsControlMsg, DevtoolsPageInfo, NavigationState, NetworkEvent,
ScriptToDevtoolsControlMsg, SourceInfo, WorkerId,
ScriptToDevtoolsControlMsg, SourceInfo, WorkerId, get_time_stamp,
};
use embedder_traits::{AllowOrDeny, EmbedderMsg, EmbedderProxy};
use log::{trace, warn};
@@ -35,7 +35,7 @@ use serde::Serialize;
use crate::actor::{Actor, ActorRegistry};
use crate::actors::browsing_context::BrowsingContextActor;
use crate::actors::console::{ConsoleActor, Root};
use crate::actors::console::{ConsoleActor, ConsoleResource, Root};
use crate::actors::framerate::FramerateActor;
use crate::actors::network_event::NetworkEventActor;
use crate::actors::root::RootActor;
@@ -240,7 +240,7 @@ impl DevtoolsInstance {
)) => self.handle_console_resource(
pipeline_id,
worker_id,
ConsoleResource::ConsoleMessage(console_message),
ConsoleResource::ConsoleMessage(console_message.into()),
),
DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ClearConsole(
pipeline_id,
@@ -266,18 +266,22 @@ impl DevtoolsInstance {
pipeline_id,
css_error,
)) => {
let mut console_message = ConsoleMessageBuilder::new(
ConsoleLogLevel::Warn,
css_error.filename,
css_error.line,
css_error.column,
);
console_message.add_argument(css_error.msg.into());
let console_message = ConsoleMessage {
fields: ConsoleMessageFields {
level: ConsoleLogLevel::Warn,
filename: css_error.filename,
line_number: css_error.line,
column_number: css_error.column,
time_stamp: get_time_stamp(),
},
arguments: vec![css_error.msg.into()],
stacktrace: None,
};
self.handle_console_resource(
pipeline_id,
None,
ConsoleResource::ConsoleMessage(console_message.finish()),
ConsoleResource::ConsoleMessage(console_message.into()),
)
},
DevtoolsControlMsg::FromChrome(ChromeToDevtoolsControlMsg::NetworkEvent(
@@ -405,12 +409,7 @@ impl DevtoolsInstance {
Root::BrowsingContext(name.clone())
};
let console = ConsoleActor {
name: console_name,
root: parent_actor,
cached_events: Default::default(),
client_ready_to_receive_messages: false.into(),
};
let console = ConsoleActor::new(console_name, parent_actor);
actors.register(console);
}