Removed some sources of panic from script thread and devtools, using …

…Option values instead to indicate when a pipeline context is missing where appropriate. Additionally, removed erroneous method get_browsing_context.

components/devtools/actors/inspector.rs

@@ -290,7 +290,7 @@ impl Actor for WalkerActor {
             "documentElement" => {
                 let (tx, rx) = ipc::channel().unwrap();
                 self.script_chan.send(GetDocumentElement(self.pipeline, tx)).unwrap();
-                let doc_elem_info = rx.recv().unwrap();
+                let doc_elem_info = try!(rx.recv().unwrap().ok_or(()));
                 let node = doc_elem_info.encode(registry, true, self.script_chan.clone(), self.pipeline);
 
                 let msg = DocumentElementReply {
@@ -316,7 +316,7 @@ impl Actor for WalkerActor {
                                                   registry.actor_to_script(target.to_owned()),
                                                   tx))
                                 .unwrap();
-                let children = rx.recv().unwrap();
+                let children = try!(rx.recv().unwrap().ok_or(()));
 
                 let msg = ChildrenReply {
                     hasFirst: true,
@@ -490,7 +490,7 @@ impl Actor for PageStyleActor {
                     borderTopWidth, borderRightWidth, borderBottomWidth, borderLeftWidth,
                     paddingTop, paddingRight, paddingBottom, paddingLeft,
                     width, height,
-                } = rx.recv().unwrap();
+                } = try!(rx.recv().unwrap().ok_or(()));
 
                 let auto_margins = msg.get("autoMargins")
                     .and_then(&Value::as_boolean).unwrap_or(false);
@@ -564,7 +564,7 @@ impl Actor for InspectorActor {
 
                 let (tx, rx) = ipc::channel().unwrap();
                 self.script_chan.send(GetRootNode(self.pipeline, tx)).unwrap();
-                let root_info = rx.recv().unwrap();
+                let root_info = try!(rx.recv().unwrap().ok_or(()));
 
                 let node = root_info.encode(registry, false, self.script_chan.clone(), self.pipeline);
 

components/devtools_traits/lib.rs

@@ -194,13 +194,13 @@ 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<NodeInfo>),
+    GetRootNode(PipelineId, IpcSender<Option<NodeInfo>>),
     /// Retrieve the details of the document element for the given pipeline.
-    GetDocumentElement(PipelineId, IpcSender<NodeInfo>),
+    GetDocumentElement(PipelineId, IpcSender<Option<NodeInfo>>),
     /// Retrieve the details of the child nodes of the given node in the given pipeline.
-    GetChildren(PipelineId, String, IpcSender<Vec<NodeInfo>>),
+    GetChildren(PipelineId, String, IpcSender<Option<Vec<NodeInfo>>>),
     /// Retrieve the computed layout properties of the given node in the given pipeline.
-    GetLayout(PipelineId, String, IpcSender<ComputedNodeLayout>),
+    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.

components/script/devtools.rs

@@ -25,7 +25,6 @@ use ipc_channel::ipc::IpcSender;
 use js::jsapi::{JSAutoCompartment, ObjectClassName};
 use js::jsval::UndefinedValue;
 use msg::constellation_msg::PipelineId;
-use script_thread::get_browsing_context;
 use std::ffi::CStr;
 use std::str;
 use style::properties::longhands::{margin_top, margin_right, margin_bottom, margin_left};
@@ -68,58 +67,72 @@ pub fn handle_evaluate_js(global: &GlobalRef, eval: String, reply: IpcSender<Eva
     reply.send(result).unwrap();
 }
 
-pub fn handle_get_root_node(context: &BrowsingContext, pipeline: PipelineId, reply: IpcSender<NodeInfo>) {
-    let context = get_browsing_context(context, pipeline);
+pub fn handle_get_root_node(context: &BrowsingContext, pipeline: PipelineId, reply: IpcSender<Option<NodeInfo>>) {
+    let context = match context.find(pipeline) {
+        Some(found_context) => found_context,
+        None => return reply.send(None).unwrap()
+    };
+
     let document = context.active_document();
 
     let node = document.upcast::<Node>();
-    reply.send(node.summarize()).unwrap();
+    reply.send(Some(node.summarize())).unwrap();
 }
 
 pub fn handle_get_document_element(context: &BrowsingContext,
                                    pipeline: PipelineId,
-                                   reply: IpcSender<NodeInfo>) {
-    let context = get_browsing_context(context, pipeline);
+                                   reply: IpcSender<Option<NodeInfo>>) {
+    let context = match context.find(pipeline) {
+        Some(found_context) => found_context,
+        None => return reply.send(None).unwrap()
+    };
+
     let document = context.active_document();
     let document_element = document.GetDocumentElement().unwrap();
 
     let node = document_element.upcast::<Node>();
-    reply.send(node.summarize()).unwrap();
+    reply.send(Some(node.summarize())).unwrap();
 }
 
 fn find_node_by_unique_id(context: &BrowsingContext,
                           pipeline: PipelineId,
-                          node_id: String)
-                          -> Root<Node> {
-    let context = get_browsing_context(context, pipeline);
+                          node_id: &str)
+                          -> Option<Root<Node>> {
+    let context = match context.find(pipeline) {
+        Some(found_context) => found_context,
+        None => return None
+    };
+
     let document = context.active_document();
     let node = document.upcast::<Node>();
 
-    for candidate in node.traverse_preorder() {
-        if candidate.unique_id() == node_id {
-            return candidate;
-        }
-    }
-
-    panic!("couldn't find node with unique id {}", node_id)
+    node.traverse_preorder().find(|candidate| candidate.unique_id() == node_id)
 }
 
 pub fn handle_get_children(context: &BrowsingContext,
                            pipeline: PipelineId,
                            node_id: String,
-                           reply: IpcSender<Vec<NodeInfo>>) {
-    let parent = find_node_by_unique_id(context, pipeline, node_id);
-    let children = parent.children()
-                         .map(|child| child.summarize())
-                         .collect();
-    reply.send(children).unwrap();
+                           reply: IpcSender<Option<Vec<NodeInfo>>>) {
+    match find_node_by_unique_id(context, pipeline, &*node_id) {
+        None => return reply.send(None).unwrap(),
+        Some(parent) => {
+            let children = parent.children()
+                                 .map(|child| child.summarize())
+                                 .collect();
+
+            reply.send(Some(children)).unwrap();
+        }
+    };
 }
 
 pub fn handle_get_layout(context: &BrowsingContext,
                          pipeline: PipelineId,
                          node_id: String,
-                         reply: IpcSender<ComputedNodeLayout>) {
-    let node = find_node_by_unique_id(context, pipeline, node_id);
+                         reply: IpcSender<Option<ComputedNodeLayout>>) {
+    let node = match find_node_by_unique_id(context, pipeline, &*node_id) {
+        None => return reply.send(None).unwrap(),
+        Some(found_node) => found_node
+    };
 
     let elem = node.downcast::<Element>().expect("should be getting layout of element");
     let rect = elem.GetBoundingClientRect();
@@ -130,7 +143,7 @@ pub fn handle_get_layout(context: &BrowsingContext,
     let elem = node.downcast::<Element>().expect("should be getting layout of element");
     let computed_style = window.r().GetComputedStyle(elem, None);
 
-    reply.send(ComputedNodeLayout {
+    reply.send(Some(ComputedNodeLayout {
         display: String::from(computed_style.Display()),
         position: String::from(computed_style.Position()),
         zIndex: String::from(computed_style.ZIndex()),
@@ -150,7 +163,7 @@ pub fn handle_get_layout(context: &BrowsingContext,
         paddingLeft: String::from(computed_style.PaddingLeft()),
         width: width,
         height: height,
-    }).unwrap();
+    })).unwrap();
 }
 
 fn determine_auto_margins(window: &Window, node: &Node) -> AutoMargins {
@@ -209,7 +222,11 @@ pub fn handle_modify_attribute(context: &BrowsingContext,
                                pipeline: PipelineId,
                                node_id: String,
                                modifications: Vec<Modification>) {
-    let node = find_node_by_unique_id(context, pipeline, node_id);
+    let node = match find_node_by_unique_id(context, pipeline, &*node_id) {
+        None => return warn!("node id {} for pipeline id {} is not found", &node_id, &pipeline),
+        Some(found_node) => found_node
+    };
+
     let elem = node.downcast::<Element>().expect("should be getting layout of element");
 
     for modification in modifications {
@@ -243,7 +260,11 @@ pub fn handle_drop_timeline_markers(context: &BrowsingContext,
 pub fn handle_request_animation_frame(context: &BrowsingContext,
                                       id: PipelineId,
                                       actor_name: String) {
-    let context = context.find(id).expect("There is no such context");
+    let context = match context.find(id) {
+        None => return warn!("context for pipeline id {} is not found", id),
+        Some(found_node) => found_node
+    };
+
     let doc = context.active_document();
     let devtools_sender = context.active_window().devtools_chan().unwrap();
     doc.request_animation_frame(box move |time| {
@@ -254,7 +275,11 @@ pub fn handle_request_animation_frame(context: &BrowsingContext,
 
 pub fn handle_reload(context: &BrowsingContext,
                      id: PipelineId) {
-    let context = context.find(id).expect("There is no such context");
+    let context = match context.find(id) {
+        None => return warn!("context for pipeline id {} is not found", id),
+        Some(found_node) => found_node
+    };
+
     let win = context.active_window();
     let location = win.Location();
     location.Reload();

components/script/script_thread.rs

@@ -1108,7 +1108,10 @@ impl ScriptThread {
 
     fn handle_resize(&self, id: PipelineId, size: WindowSizeData, size_type: WindowSizeType) {
         if let Some(ref context) = self.find_child_context(id) {
-            let window = context.active_window();
+            let window = match context.find(id) {
+                Some(browsing_context) => browsing_context.active_window(),
+                None => return warn!("Message sent to closed pipeline {}.", id),
+            };
             window.set_resize_event(size, size_type);
             return;
         }
@@ -2204,15 +2207,6 @@ fn shut_down_layout(context_tree: &BrowsingContext) {
     }
 }
 
-// TODO: remove this function, as it's a source of panic.
-pub fn get_browsing_context(context: &BrowsingContext,
-                            pipeline_id: PipelineId)
-                            -> Root<BrowsingContext> {
-    context.find(pipeline_id).expect("ScriptThread: received an event \
-            message for a layout channel that is not associated with this script thread.\
-            This is a bug.")
-}
-
 fn dom_last_modified(tm: &Tm) -> String {
     tm.to_local().strftime("%m/%d/%Y %H:%M:%S").unwrap().to_string()
 }

components/script/webdriver_handlers.rs

@@ -34,7 +34,6 @@ use msg::constellation_msg::PipelineId;
 use net_traits::CookieSource::{HTTP, NonHTTP};
 use net_traits::CoreResourceMsg::{GetCookiesDataForUrl, SetCookiesForUrlWithData};
 use net_traits::IpcSend;
-use script_thread::get_browsing_context;
 use script_traits::webdriver_msg::WebDriverCookieError;
 use script_traits::webdriver_msg::{WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue};
 use url::Url;
@@ -43,7 +42,11 @@ fn find_node_by_unique_id(context: &BrowsingContext,
                           pipeline: PipelineId,
                           node_id: String)
                           -> Option<Root<Node>> {
-    let context = get_browsing_context(&context, pipeline);
+    let context = match context.find(pipeline) {
+        Some(context) => context,
+        None => return None
+    };
+
     let document = context.active_document();
     document.upcast::<Node>().traverse_preorder().find(|candidate| candidate.unique_id() == node_id)
 }
@@ -72,7 +75,11 @@ pub fn handle_execute_script(context: &BrowsingContext,
                              pipeline: PipelineId,
                              eval: String,
                              reply: IpcSender<WebDriverJSResult>) {
-    let context = get_browsing_context(&context, pipeline);
+    let context = match context.find(pipeline) {
+        Some(context) => context,
+        None => return reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap()
+    };
+
     let window = context.active_window();
     let result = unsafe {
         let cx = window.get_cx();
@@ -87,7 +94,11 @@ pub fn handle_execute_async_script(context: &BrowsingContext,
                                    pipeline: PipelineId,
                                    eval: String,
                                    reply: IpcSender<WebDriverJSResult>) {
-    let context = get_browsing_context(&context, pipeline);
+    let context = match context.find(pipeline) {
+       Some(context) => context,
+       None => return reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap()
+   };
+
     let window = context.active_window();
     let cx = window.get_cx();
     window.set_webdriver_script_chan(Some(reply));

components/script_traits/webdriver_msg.rs

@@ -53,7 +53,10 @@ pub enum WebDriverJSValue {
 #[derive(Deserialize, Serialize)]
 pub enum WebDriverJSError {
     Timeout,
-    UnknownType
+    UnknownType,
+    /// Occurs when handler received an event message for a layout channel that is not
+    /// associated with the current script thread
+    BrowsingContextNotFound
 }
 
 pub type WebDriverJSResult = Result<WebDriverJSValue, WebDriverJSError>;

components/webdriver_server/lib.rs

@@ -738,7 +738,9 @@ impl Handler {
             Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))),
             Err(WebDriverJSError::Timeout) => Err(WebDriverError::new(ErrorStatus::Timeout, "")),
             Err(WebDriverJSError::UnknownType) => Err(WebDriverError::new(
-                ErrorStatus::UnsupportedOperation, "Unsupported return type"))
+                ErrorStatus::UnsupportedOperation, "Unsupported return type")),
+            Err(WebDriverJSError::BrowsingContextNotFound) => Err(WebDriverError::new(
+                ErrorStatus::JavascriptError, "Pipeline id not found in browsing context"))
         }
     }