Skip to content

Commit

Permalink
Replacing unwrap calls in devtools, returning/sending Option values i…
Browse files Browse the repository at this point in the history
…nstead
  • Loading branch information
craftytrickster committed Jul 19, 2016
1 parent d438c75 commit 3a6d2f9
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 41 deletions.
8 changes: 4 additions & 4 deletions components/devtools/actors/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 4 additions & 4 deletions components/devtools_traits/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
79 changes: 56 additions & 23 deletions components/script/devtools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,53 +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 = context.find(pipeline).unwrap();
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 = context.find(pipeline).unwrap();
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 = context.find(pipeline).unwrap();
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>();

node.traverse_preorder().find(|candidate| candidate.unique_id() == node_id)
.expect(&format!("couldn't find node with 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();
Expand All @@ -124,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()),
Expand All @@ -144,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 {
Expand Down Expand Up @@ -203,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 {
Expand Down Expand Up @@ -237,7 +260,12 @@ 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| {
Expand All @@ -248,7 +276,12 @@ 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();
Expand Down
10 changes: 2 additions & 8 deletions components/script/webdriver_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,7 @@ pub fn handle_execute_script(context: &BrowsingContext,
reply: IpcSender<WebDriverJSResult>) {
let context = match context.find(pipeline) {
Some(context) => context,
None => {
reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap();
return;
}
None => return reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap()
};

let window = context.active_window();
Expand All @@ -99,10 +96,7 @@ pub fn handle_execute_async_script(context: &BrowsingContext,
reply: IpcSender<WebDriverJSResult>) {
let context = match context.find(pipeline) {
Some(context) => context,
None => {
reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap();
return;
}
None => return reply.send(Err(WebDriverJSError::BrowsingContextNotFound)).unwrap()
};

let window = context.active_window();
Expand Down
3 changes: 1 addition & 2 deletions components/webdriver_server/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,8 +740,7 @@ impl Handler {
Err(WebDriverJSError::UnknownType) => Err(WebDriverError::new(
ErrorStatus::UnsupportedOperation, "Unsupported return type")),
Err(WebDriverJSError::BrowsingContextNotFound) => Err(WebDriverError::new(
ErrorStatus::JavascriptError, "Pipeline id not found in browsing context"
))
ErrorStatus::JavascriptError, "Pipeline id not found in browsing context"))
}
}

Expand Down

0 comments on commit 3a6d2f9

Please sign in to comment.