From 56c660d4de9d9a077f3d28348e29bda0b3ac0ae6 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 15 Jul 2015 15:29:09 +0200 Subject: [PATCH 1/4] Split up WebSocketTaskHandler. There's no real reason to have internal dynamic dispatch inside a trait object. --- components/script/dom/websocket.rs | 54 ++++++++++-------------------- 1 file changed, 17 insertions(+), 37 deletions(-) diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 6b97352f65b..2632d7838c5 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -142,7 +142,9 @@ impl WebSocket { debug!("Failed to establish a WebSocket connection: {:?}", e); let global_root = ws.r().global.root(); let address = Trusted::new(global_root.r().get_cx(), ws.r(), global_root.r().script_chan().clone()); - let task = box WebSocketTaskHandler::new(address, WebSocketTask::Close); + let task = box CloseTask { + addr: address, + }; global_root.r().script_chan().send(ScriptMsg::RunnableMsg(task)).unwrap(); return Ok(ws); } @@ -155,7 +157,9 @@ impl WebSocket { let global_root = ws.r().global.root(); let addr: Trusted = Trusted::new(global_root.r().get_cx(), ws.r(), global_root.r().script_chan().clone()); - let open_task = box WebSocketTaskHandler::new(addr, WebSocketTask::ConnectionEstablished); + let open_task = box ConnectionEstablishedTask { + addr: addr, + }; global_root.r().script_chan().send(ScriptMsg::RunnableMsg(open_task)).unwrap(); //TODO: Spawn thread here for receive loop /*TODO: Add receive loop here and make new thread run this @@ -273,29 +277,13 @@ impl<'a> WebSocketMethods for &'a WebSocket { } -pub enum WebSocketTask { - /// Task queued when *the WebSocket connection is established*. - ConnectionEstablished, - Close, -} - -pub struct WebSocketTaskHandler { +/// Task queued when *the WebSocket connection is established*. +struct ConnectionEstablishedTask { addr: Trusted, - task: WebSocketTask, } -impl WebSocketTaskHandler { - pub fn new(addr: Trusted, task: WebSocketTask) -> WebSocketTaskHandler { - WebSocketTaskHandler { - addr: addr, - task: task, - } - } - - fn connection_established(&self) { - /*TODO: Items 1, 3, 4, & 5 under "WebSocket connection is established" as specified here: - https://html.spec.whatwg.org/multipage/#feedback-from-the-protocol - */ +impl Runnable for ConnectionEstablishedTask { + fn handler(self: Box) { let ws = self.addr.root(); // Step 1: Protocols. @@ -314,8 +302,14 @@ impl WebSocketTaskHandler { EventCancelable::NotCancelable); event.fire(EventTargetCast::from_ref(ws.r())); } +} - fn dispatch_close(&self) { +struct CloseTask { + addr: Trusted, +} + +impl Runnable for CloseTask { + fn handler(self: Box) { let ws = self.addr.root(); let ws = ws.r(); let global = ws.global.root(); @@ -350,17 +344,3 @@ impl WebSocketTaskHandler { event.fire(target); } } - -impl Runnable for WebSocketTaskHandler { - fn handler(self: Box) { - match self.task { - WebSocketTask::ConnectionEstablished => { - self.connection_established(); - } - WebSocketTask::Close => { - self.dispatch_close(); - } - } - } -} - From 78df6e8d3e59d14dfecdad81f1276f34de49cd61 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 15 Jul 2015 16:38:00 +0200 Subject: [PATCH 2/4] Remove the receiver field from WebSocket. The receiver will be used from another thread than the WebSocket object in the future. --- components/script/dom/websocket.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 2632d7838c5..aeb13ac4f95 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -46,7 +46,6 @@ enum WebSocketRequestState { } no_jsmanaged_fields!(Sender); -no_jsmanaged_fields!(Receiver); #[dom_struct] pub struct WebSocket { @@ -55,7 +54,6 @@ pub struct WebSocket { global: GlobalField, ready_state: Cell, sender: RefCell>>, - receiver: RefCell>>, failed: Cell, //Flag to tell if websocket was closed due to failure full: Cell, //Flag to tell if websocket queue is full clean_close: Cell, //Flag to tell if the websocket closed cleanly (not due to full or fail) @@ -86,7 +84,6 @@ impl WebSocket { ready_state: Cell::new(WebSocketRequestState::Connecting), failed: Cell::new(false), sender: RefCell::new(None), - receiver: RefCell::new(None), full: Cell::new(false), clean_close: Cell::new(true), code: Cell::new(0), @@ -136,7 +133,7 @@ impl WebSocket { WebSocketBinding::Wrap); let channel = establish_a_websocket_connection(url, global.get_url().serialize()); - let (temp_sender, temp_receiver) = match channel { + let (temp_sender, _temp_receiver) = match channel { Ok(channel) => channel, Err(e) => { debug!("Failed to establish a WebSocket connection: {:?}", e); @@ -151,7 +148,6 @@ impl WebSocket { }; *ws.r().sender.borrow_mut() = Some(temp_sender); - *ws.r().receiver.borrow_mut() = Some(temp_receiver); //Create everything necessary for starting the open asynchronous task, then begin the task. let global_root = ws.r().global.root(); From 91849cb603a43ecdc403d059788ae79fb28e1f4a Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 15 Jul 2015 15:47:13 +0200 Subject: [PATCH 3/4] Set the WebSocket's Sender from the ConnectionEstablishedTask. This needs to happen off a task because we won't be able to access the WebSocket object directly once this code moves to a background thread. There is no behaviour change, because we make sure that self.ready_state is not Connecting in Send(). --- components/script/dom/websocket.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index aeb13ac4f95..2e415e5d73d 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -147,14 +147,13 @@ impl WebSocket { } }; - *ws.r().sender.borrow_mut() = Some(temp_sender); - //Create everything necessary for starting the open asynchronous task, then begin the task. let global_root = ws.r().global.root(); let addr: Trusted = Trusted::new(global_root.r().get_cx(), ws.r(), global_root.r().script_chan().clone()); let open_task = box ConnectionEstablishedTask { addr: addr, + sender: temp_sender, }; global_root.r().script_chan().send(ScriptMsg::RunnableMsg(open_task)).unwrap(); //TODO: Spawn thread here for receive loop @@ -276,12 +275,15 @@ impl<'a> WebSocketMethods for &'a WebSocket { /// Task queued when *the WebSocket connection is established*. struct ConnectionEstablishedTask { addr: Trusted, + sender: Sender, } impl Runnable for ConnectionEstablishedTask { fn handler(self: Box) { let ws = self.addr.root(); + *ws.r().sender.borrow_mut() = Some(self.sender); + // Step 1: Protocols. // Step 2. From 32ddd356b8eca20bca0e73c001bce01a34b62bca Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 13 Jul 2015 14:53:21 +0200 Subject: [PATCH 4/4] Spawn a thread for WebSocket messages. --- components/script/dom/websocket.rs | 77 +++++++++++++----------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 2e415e5d73d..2ffe520827d 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -24,6 +24,7 @@ use script_task::ScriptMsg; use std::cell::{Cell, RefCell}; use std::borrow::ToOwned; use util::str::DOMString; +use util::task::spawn_named; use hyper::header::Host; use websocket::Message; @@ -101,6 +102,9 @@ impl WebSocket { let parsed_url = try!(Url::parse(&url).map_err(|_| Error::Syntax)); let url = try!(parse_url(&parsed_url).map_err(|_| Error::Syntax)); + // Step 2: Disallow https -> ws connections. + // Step 3: Potentially block access to some ports. + // Step 4. let protocols = protocols.as_slice(); @@ -121,54 +125,41 @@ impl WebSocket { } } - /*TODO: This constructor is only a prototype, it does not accomplish the specs - defined here: - http://html.spec.whatwg.org - The remaining 8 items must be satisfied. - TODO: This constructor should be responsible for spawning a thread for the - receive loop after ws.r().Open() - See comment - */ + // Step 6: Origin. + + // Step 7. let ws = reflect_dom_object(box WebSocket::new_inherited(global, parsed_url), global, WebSocketBinding::Wrap); + let address = Trusted::new(global.get_cx(), ws.r(), global.script_chan()); - let channel = establish_a_websocket_connection(url, global.get_url().serialize()); - let (temp_sender, _temp_receiver) = match channel { - Ok(channel) => channel, - Err(e) => { - debug!("Failed to establish a WebSocket connection: {:?}", e); - let global_root = ws.r().global.root(); - let address = Trusted::new(global_root.r().get_cx(), ws.r(), global_root.r().script_chan().clone()); - let task = box CloseTask { - addr: address, - }; - global_root.r().script_chan().send(ScriptMsg::RunnableMsg(task)).unwrap(); - return Ok(ws); - } - }; + let origin = global.get_url().serialize(); + let sender = global.script_chan(); + spawn_named(format!("WebSocket connection to {}", ws.Url()), move || { + // Step 8: Protocols. - //Create everything necessary for starting the open asynchronous task, then begin the task. - let global_root = ws.r().global.root(); - let addr: Trusted = - Trusted::new(global_root.r().get_cx(), ws.r(), global_root.r().script_chan().clone()); - let open_task = box ConnectionEstablishedTask { - addr: addr, - sender: temp_sender, - }; - global_root.r().script_chan().send(ScriptMsg::RunnableMsg(open_task)).unwrap(); - //TODO: Spawn thread here for receive loop - /*TODO: Add receive loop here and make new thread run this - Receive is an infinite loop "similiar" the one shown here: - https://github.com/cyderize/rust-websocket/blob/master/examples/client.rs#L64 - TODO: The receive loop however does need to follow the spec. These are outlined here - under "WebSocket message has been received" items 1-5: - https://github.com/cyderize/rust-websocket/blob/master/examples/client.rs#L64 - TODO: The receive loop also needs to dispatch an asynchronous event as stated here: - https://github.com/cyderize/rust-websocket/blob/master/examples/client.rs#L64 - TODO: When the receive loop receives a close message from the server, - it confirms the websocket is now closed. This requires the close event - to be fired (dispatch_close fires the close event - see implementation below) - */ + // Step 9. + let channel = establish_a_websocket_connection(url, origin); + let (temp_sender, _temp_receiver) = match channel { + Ok(channel) => channel, + Err(e) => { + debug!("Failed to establish a WebSocket connection: {:?}", e); + let task = box CloseTask { + addr: address, + }; + sender.send(ScriptMsg::RunnableMsg(task)).unwrap(); + return; + } + }; + + let open_task = box ConnectionEstablishedTask { + addr: address, + sender: temp_sender, + }; + sender.send(ScriptMsg::RunnableMsg(open_task)).unwrap(); + }); + + // Step 7. Ok(ws) }