From a6d83a38e56fa953be4cd7353ca948010a52a0e7 Mon Sep 17 00:00:00 2001 From: Alan Jeffrey Date: Wed, 12 Oct 2016 12:56:23 -0500 Subject: [PATCH] Setting a devtools timeline marker may fail, due to pipeline lookup failure. --- components/devtools/actors/timeline.rs | 6 +++--- components/devtools_traits/lib.rs | 3 ++- components/script/devtools.rs | 15 ++++++++++----- components/script/dom/window.rs | 6 +++--- components/script/script_thread.rs | 20 ++++++++++---------- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/components/devtools/actors/timeline.rs b/components/devtools/actors/timeline.rs index d52437edbafa..0d495faf7c2e 100644 --- a/components/devtools/actors/timeline.rs +++ b/components/devtools/actors/timeline.rs @@ -143,7 +143,7 @@ impl TimelineActor { } } - fn pull_timeline_data(&self, receiver: IpcReceiver, mut emitter: Emitter) { + fn pull_timeline_data(&self, receiver: IpcReceiver>, mut emitter: Emitter) { let is_recording = self.is_recording.clone(); if !*is_recording.lock().unwrap() { @@ -157,7 +157,7 @@ impl TimelineActor { } 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); @@ -182,7 +182,7 @@ impl Actor for TimelineActor { "start" => { **self.is_recording.lock().as_mut().unwrap() = true; - let (tx, rx) = ipc::channel::().unwrap(); + let (tx, rx) = ipc::channel::>().unwrap(); self.script_sender.send(SetTimelineMarkers(self.pipeline, self.marker_types.clone(), tx)).unwrap(); diff --git a/components/devtools_traits/lib.rs b/components/devtools_traits/lib.rs index b761c410eb04..4944fd703174 100644 --- a/components/devtools_traits/lib.rs +++ b/components/devtools_traits/lib.rs @@ -190,6 +190,7 @@ pub struct AutoMargins { } /// 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. @@ -209,7 +210,7 @@ pub enum DevtoolScriptControlMsg { /// 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, IpcSender), + SetTimelineMarkers(PipelineId, Vec, IpcSender>), /// Withdraw request for live timeline notifications for a given pipeline. DropTimelineMarkers(PipelineId, Vec), /// Request a callback directed at the given actor name from the next animation frame diff --git a/components/script/devtools.rs b/components/script/devtools.rs index f8517ae0dba6..4163ee426ed4 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -250,16 +250,21 @@ pub fn handle_wants_live_notifications(global: &GlobalScope, send_notifications: } pub fn handle_set_timeline_markers(context: &BrowsingContext, + pipeline: PipelineId, marker_types: Vec, - reply: IpcSender) { - let window = context.active_window(); - window.set_devtools_timeline_markers(marker_types, reply); + reply: IpcSender>) { + 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) { - 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, diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 9bb813a29463..9a836b6d8450 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -172,7 +172,7 @@ pub struct Window { /// no devtools server devtools_markers: DOMRefCell>, #[ignore_heap_size_of = "channels are hard"] - devtools_marker_sender: DOMRefCell>>, + devtools_marker_sender: DOMRefCell>>>, /// Pending resize event, if any. resize_event: Cell>, @@ -1435,12 +1435,12 @@ impl Window { 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, - reply: IpcSender) { + reply: IpcSender>) { *self.devtools_marker_sender.borrow_mut() = Some(reply); self.devtools_markers.borrow_mut().extend(markers.into_iter()); } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index dfd5b0300b31..bbb8a6c203fb 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1004,8 +1004,8 @@ impl ScriptThread { 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) => { @@ -1015,14 +1015,14 @@ impl ScriptThread { }; 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), } }