servo: Merge
#13728 - Setting a devtools timeline marker may fail, due to pipeline lookup failure (from asajeffrey:devtools-set-timeline-marker-may-fail); r=fitzgen
<!-- Please describe your changes on the following line: -->
Allow setting a devtools timeline marker to fail, due to pipeline lookup failure. This is part of tidying up pipeline lookup.
cc @jdm
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because I'm not sure how to test devtools.
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo:
https://github.com/servo/servo
Source-Revision:
bb75e2e727eae64477c128ace7bd5cff35e0ea81
--- a/servo/components/devtools/actors/timeline.rs
+++ b/servo/components/devtools/actors/timeline.rs
@@ -138,31 +138,31 @@ impl TimelineActor {
is_recording: Arc::new(Mutex::new(false)),
stream: RefCell::new(None),
framerate_actor: RefCell::new(None),
memory_actor: RefCell::new(None),
}
}
- fn pull_timeline_data(&self, receiver: IpcReceiver<TimelineMarker>, mut emitter: Emitter) {
+ fn pull_timeline_data(&self, receiver: IpcReceiver<Option<TimelineMarker>>, mut emitter: Emitter) {
let is_recording = self.is_recording.clone();
if !*is_recording.lock().unwrap() {
return;
}
spawn_named("PullTimelineMarkers".to_owned(), move || {
loop {
if !*is_recording.lock().unwrap() {
break;
}
let mut markers = vec![];
- while let Ok(marker) = receiver.try_recv() {
+ while let Ok(Some(marker)) = receiver.try_recv() {
markers.push(emitter.marker(marker));
}
emitter.send(markers);
thread::sleep(Duration::from_millis(DEFAULT_TIMELINE_DATA_PULL_TIMEOUT));
}
});
}
@@ -177,17 +177,17 @@ impl Actor for TimelineActor {
registry: &ActorRegistry,
msg_type: &str,
msg: &BTreeMap<String, Value>,
stream: &mut TcpStream) -> Result<ActorMessageStatus, ()> {
Ok(match msg_type {
"start" => {
**self.is_recording.lock().as_mut().unwrap() = true;
- let (tx, rx) = ipc::channel::<TimelineMarker>().unwrap();
+ let (tx, rx) = ipc::channel::<Option<TimelineMarker>>().unwrap();
self.script_sender.send(SetTimelineMarkers(self.pipeline,
self.marker_types.clone(),
tx)).unwrap();
*self.stream.borrow_mut() = stream.try_clone().ok();
// init memory actor
if let Some(with_memory) = msg.get("withMemory") {
--- a/servo/components/devtools_traits/lib.rs
+++ b/servo/components/devtools_traits/lib.rs
@@ -185,16 +185,17 @@ pub struct ComputedNodeLayout {
pub struct AutoMargins {
pub top: bool,
pub right: bool,
pub bottom: bool,
pub left: bool,
}
/// Messages to process in a particular script thread, as instructed by a devtools client.
+/// TODO: better error handling, e.g. if pipeline id lookup fails?
#[derive(Deserialize, Serialize)]
pub enum DevtoolScriptControlMsg {
/// Evaluate a JS snippet in the context of the global for the given pipeline.
EvaluateJS(PipelineId, String, IpcSender<EvaluateJSReply>),
/// Retrieve the details of the root node (ie. the document) for the given pipeline.
GetRootNode(PipelineId, IpcSender<Option<NodeInfo>>),
/// Retrieve the details of the document element for the given pipeline.
GetDocumentElement(PipelineId, IpcSender<Option<NodeInfo>>),
@@ -204,17 +205,17 @@ pub enum DevtoolScriptControlMsg {
GetLayout(PipelineId, String, IpcSender<Option<ComputedNodeLayout>>),
/// Retrieve all stored console messages for the given pipeline.
GetCachedMessages(PipelineId, CachedConsoleMessageTypes, IpcSender<Vec<CachedConsoleMessage>>),
/// Update a given node's attributes with a list of modifications.
ModifyAttribute(PipelineId, String, Vec<Modification>),
/// Request live console messages for a given pipeline (true if desired, false otherwise).
WantsLiveNotifications(PipelineId, bool),
/// Request live notifications for a given set of timeline events for a given pipeline.
- SetTimelineMarkers(PipelineId, Vec<TimelineMarkerType>, IpcSender<TimelineMarker>),
+ SetTimelineMarkers(PipelineId, Vec<TimelineMarkerType>, IpcSender<Option<TimelineMarker>>),
/// Withdraw request for live timeline notifications for a given pipeline.
DropTimelineMarkers(PipelineId, Vec<TimelineMarkerType>),
/// Request a callback directed at the given actor name from the next animation frame
/// executed in the given pipeline.
RequestAnimationFrame(PipelineId, String),
/// Direct the given pipeline to reload the current page.
Reload(PipelineId),
}
--- a/servo/components/script/devtools.rs
+++ b/servo/components/script/devtools.rs
@@ -245,26 +245,31 @@ pub fn handle_modify_attribute(context:
}
}
pub fn handle_wants_live_notifications(global: &GlobalScope, send_notifications: bool) {
global.set_devtools_wants_updates(send_notifications);
}
pub fn handle_set_timeline_markers(context: &BrowsingContext,
+ pipeline: PipelineId,
marker_types: Vec<TimelineMarkerType>,
- reply: IpcSender<TimelineMarker>) {
- let window = context.active_window();
- window.set_devtools_timeline_markers(marker_types, reply);
+ reply: IpcSender<Option<TimelineMarker>>) {
+ match context.find(pipeline) {
+ None => reply.send(None).unwrap(),
+ Some(context) => context.active_window().set_devtools_timeline_markers(marker_types, reply),
+ }
}
pub fn handle_drop_timeline_markers(context: &BrowsingContext,
+ pipeline: PipelineId,
marker_types: Vec<TimelineMarkerType>) {
- let window = context.active_window();
- window.drop_devtools_timeline_markers(marker_types);
+ if let Some(context) = context.find(pipeline) {
+ context.active_window().drop_devtools_timeline_markers(marker_types);
+ }
}
pub fn handle_request_animation_frame(context: &BrowsingContext,
id: PipelineId,
actor_name: String) {
let context = match context.find(id) {
None => return warn!("context for pipeline id {} is not found", id),
Some(found_node) => found_node
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -167,17 +167,17 @@ pub struct Window {
session_storage: MutNullableHeap<JS<Storage>>,
local_storage: MutNullableHeap<JS<Storage>>,
status: DOMRefCell<DOMString>,
/// For sending timeline markers. Will be ignored if
/// no devtools server
devtools_markers: DOMRefCell<HashSet<TimelineMarkerType>>,
#[ignore_heap_size_of = "channels are hard"]
- devtools_marker_sender: DOMRefCell<Option<IpcSender<TimelineMarker>>>,
+ devtools_marker_sender: DOMRefCell<Option<IpcSender<Option<TimelineMarker>>>>,
/// Pending resize event, if any.
resize_event: Cell<Option<(WindowSizeData, WindowSizeType)>>,
/// Parent id associated with this page, if any.
parent_info: Option<(PipelineId, FrameType)>,
/// Global static data related to the DOM.
@@ -1430,22 +1430,22 @@ impl Window {
pub fn need_emit_timeline_marker(&self, timeline_type: TimelineMarkerType) -> bool {
let markers = self.devtools_markers.borrow();
markers.contains(&timeline_type)
}
pub fn emit_timeline_marker(&self, marker: TimelineMarker) {
let sender = self.devtools_marker_sender.borrow();
let sender = sender.as_ref().expect("There is no marker sender");
- sender.send(marker).unwrap();
+ sender.send(Some(marker)).unwrap();
}
pub fn set_devtools_timeline_markers(&self,
markers: Vec<TimelineMarkerType>,
- reply: IpcSender<TimelineMarker>) {
+ reply: IpcSender<Option<TimelineMarker>>) {
*self.devtools_marker_sender.borrow_mut() = Some(reply);
self.devtools_markers.borrow_mut().extend(markers.into_iter());
}
pub fn drop_devtools_timeline_markers(&self, markers: Vec<TimelineMarkerType>) {
let mut devtools_markers = self.devtools_markers.borrow_mut();
for marker in markers {
devtools_markers.remove(&marker);
--- a/servo/components/script/script_thread.rs
+++ b/servo/components/script/script_thread.rs
@@ -999,35 +999,35 @@ impl ScriptThread {
DevtoolScriptControlMsg::GetRootNode(id, reply) =>
devtools::handle_get_root_node(&context, id, reply),
DevtoolScriptControlMsg::GetDocumentElement(id, reply) =>
devtools::handle_get_document_element(&context, id, reply),
DevtoolScriptControlMsg::GetChildren(id, node_id, reply) =>
devtools::handle_get_children(&context, id, node_id, reply),
DevtoolScriptControlMsg::GetLayout(id, node_id, reply) =>
devtools::handle_get_layout(&context, id, node_id, reply),
- DevtoolScriptControlMsg::GetCachedMessages(pipeline_id, message_types, reply) =>
- devtools::handle_get_cached_messages(pipeline_id, message_types, reply),
+ DevtoolScriptControlMsg::GetCachedMessages(id, message_types, reply) =>
+ devtools::handle_get_cached_messages(id, message_types, reply),
DevtoolScriptControlMsg::ModifyAttribute(id, node_id, modifications) =>
devtools::handle_modify_attribute(&context, id, node_id, modifications),
DevtoolScriptControlMsg::WantsLiveNotifications(id, to_send) => {
let window = match context.find(id) {
Some(browsing_context) => browsing_context.active_window(),
None => return warn!("Message sent to closed pipeline {}.", id),
};
devtools::handle_wants_live_notifications(window.upcast(), to_send)
},
- DevtoolScriptControlMsg::SetTimelineMarkers(_pipeline_id, marker_types, reply) =>
- devtools::handle_set_timeline_markers(&context, marker_types, reply),
- DevtoolScriptControlMsg::DropTimelineMarkers(_pipeline_id, marker_types) =>
- devtools::handle_drop_timeline_markers(&context, marker_types),
- DevtoolScriptControlMsg::RequestAnimationFrame(pipeline_id, name) =>
- devtools::handle_request_animation_frame(&context, pipeline_id, name),
- DevtoolScriptControlMsg::Reload(pipeline_id) =>
- devtools::handle_reload(&context, pipeline_id),
+ DevtoolScriptControlMsg::SetTimelineMarkers(id, marker_types, reply) =>
+ devtools::handle_set_timeline_markers(&context, id, marker_types, reply),
+ DevtoolScriptControlMsg::DropTimelineMarkers(id, marker_types) =>
+ devtools::handle_drop_timeline_markers(&context, id, marker_types),
+ DevtoolScriptControlMsg::RequestAnimationFrame(id, name) =>
+ devtools::handle_request_animation_frame(&context, id, name),
+ DevtoolScriptControlMsg::Reload(id) =>
+ devtools::handle_reload(&context, id),
}
}
fn handle_msg_from_image_cache(&self, msg: ImageCacheResult) {
msg.responder.unwrap().respond(msg.image_response);
}
fn handle_webdriver_msg(&self, pipeline_id: PipelineId, msg: WebDriverScriptCommand) {