Create easy common interface for off-thread network listeners, and remove the CORS-specific reimplementation of async networking.

components/script/cors.rs

@@ -9,8 +9,14 @@
 //! This library will eventually become the core of the Fetch crate
 //! with CORSRequest being expanded into FetchRequest (etc)
 
+use network_listener::{NetworkListener, PreInvoke};
+use script_task::ScriptChan;
+use net_traits::{AsyncResponseTarget, AsyncResponseListener, ResponseAction, Metadata};
+
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
+use std::cell::RefCell;
+use std::sync::{Arc, Mutex};
 use time;
 use time::{now, Timespec};
 
@@ -27,14 +33,12 @@ use hyper::status::StatusClass::Success;
 use url::{SchemeData, Url};
 use util::task::spawn_named;
 
+/// Interface for network listeners concerned with CORS checks. Proper network requests
+/// should be initiated from this method, based on the response provided.
 pub trait AsyncCORSResponseListener {
     fn response_available(&self, response: CORSResponse);
 }
 
-pub trait AsyncCORSResponseTarget {
-    fn invoke_with_listener(&self, response: CORSResponse);
-}
-
 #[derive(Clone)]
 pub struct CORSRequest {
     pub origin: Url,
@@ -98,13 +102,48 @@ impl CORSRequest {
         }
     }
 
-    pub fn http_fetch_async(&self, listener: Box<AsyncCORSResponseTarget + Send>) {
+    pub fn http_fetch_async(&self,
+                            listener: Box<AsyncCORSResponseListener+Send>,
+                            script_chan: Box<ScriptChan+Send>) {
+        struct CORSContext {
+            listener: Box<AsyncCORSResponseListener+Send>,
+            response: RefCell<Option<CORSResponse>>,
+        }
+
+        // This is shoe-horning the CORSReponse stuff into the rest of the async network
+        // framework right now. It would be worth redesigning http_fetch to do this properly.
+        impl AsyncResponseListener for CORSContext {
+            fn headers_available(&self, _metadata: Metadata) {
+            }
+
+            fn data_available(&self, _payload: Vec<u8>) {
+            }
+
+            fn response_complete(&self, _status: Result<(), String>) {
+                let response = self.response.borrow_mut().take().unwrap();
+                self.listener.response_available(response);
+            }
+        }
+        impl PreInvoke for CORSContext {}
+
+        let context = CORSContext {
+            listener: listener,
+            response: RefCell::new(None),
+        };
+        let listener = NetworkListener {
+            context: Arc::new(Mutex::new(context)),
+            script_chan: script_chan,
+        };
+
         // TODO: this exists only to make preflight check non-blocking
         // perhaps should be handled by the resource task?
         let req = self.clone();
         spawn_named("cors".to_owned(), move || {
             let response = req.http_fetch();
-            listener.invoke_with_listener(response);
+            let mut context = listener.context.lock();
+            let context = context.as_mut().unwrap();
+            *context.response.borrow_mut() = Some(response);
+            listener.invoke_with_listener(ResponseAction::ResponseComplete(Ok(())));
         });
     }
 

components/script/dom/xmlhttprequest.rs

@@ -26,6 +26,7 @@ use dom::urlsearchparams::URLSearchParamsHelpers;
 use dom::xmlhttprequesteventtarget::XMLHttpRequestEventTarget;
 use dom::xmlhttprequesteventtarget::XMLHttpRequestEventTargetTypeId;
 use dom::xmlhttprequestupload::XMLHttpRequestUpload;
+use network_listener::{NetworkListener, PreInvoke};
 use script_task::{ScriptChan, ScriptMsg, Runnable, ScriptPort};
 
 use encoding::all::UTF_8;
@@ -43,10 +44,10 @@ use js::jsapi::JS_ClearPendingException;
 use js::jsval::{JSVal, NullValue, UndefinedValue};
 
 use net_traits::ControlMsg::Load;
-use net_traits::{ResourceTask, ResourceCORSData, LoadData, LoadConsumer, AsyncResponseTarget};
+use net_traits::{ResourceTask, ResourceCORSData, LoadData, LoadConsumer};
-use net_traits::{AsyncResponseListener, ResponseAction, Metadata};
+use net_traits::{AsyncResponseListener, Metadata};
 use cors::{allow_cross_origin_request, CORSRequest, RequestMode, AsyncCORSResponseListener};
-use cors::{AsyncCORSResponseTarget, CORSResponse};
+use cors::CORSResponse;
 use util::str::DOMString;
 use util::task::spawn_named;
 
@@ -224,45 +225,15 @@ impl XMLHttpRequest {
             }
         }
 
-        struct CORSListener {
+        let cors_context = CORSContext {
-            context: Arc<Mutex<CORSContext>>,
-            script_chan: Box<ScriptChan+Send>,
-        }
-
-        impl AsyncCORSResponseTarget for CORSListener {
-            fn invoke_with_listener(&self, response: CORSResponse) {
-                self.script_chan.send(ScriptMsg::RunnableMsg(box CORSRunnable {
-                    context: self.context.clone(),
-                    response: response,
-                })).unwrap();
-            }
-        }
-
-        struct CORSRunnable {
-            context: Arc<Mutex<CORSContext>>,
-            response: CORSResponse,
-        }
-
-        impl Runnable for CORSRunnable {
-            fn handler(self: Box<CORSRunnable>) {
-                let this = *self;
-                let context = this.context.lock().unwrap();
-                context.response_available(this.response);
-            }
-        }
-
-        let cors_context = Arc::new(Mutex::new(CORSContext {
             xhr: context,
             load_data: RefCell::new(Some(load_data)),
             req: req.clone(),
             script_chan: script_chan.clone(),
             resource_task: resource_task,
-        }));
+        };
 
-        req.http_fetch_async(box CORSListener {
+        req.http_fetch_async(box cors_context, script_chan);
-            context: cors_context,
-            script_chan: script_chan
-        });
     }
 
     fn initiate_async_xhr(context: Arc<Mutex<XHRContext>>,
@@ -293,40 +264,14 @@ impl XMLHttpRequest {
             }
         }
 
-        struct XHRRunnable {
+        impl PreInvoke for XHRContext {
-            context: Arc<Mutex<XHRContext>>,
+            fn should_invoke(&self) -> bool {
-            action: ResponseAction,
+                let xhr = self.xhr.to_temporary().root();
-        }
+                xhr.r().generation_id.get() == self.gen_id
-
-        impl Runnable for XHRRunnable {
-            fn handler(self: Box<XHRRunnable>) {
-                let this = *self;
-
-                let context = this.context.lock().unwrap();
-                let xhr = context.xhr.to_temporary().root();
-                if xhr.r().generation_id.get() != context.gen_id {
-                    return;
-                }
-
-                this.action.process(&*context);
             }
         }
 
-        struct XHRListener {
+        let listener = box NetworkListener {
-            context: Arc<Mutex<XHRContext>>,
-            script_chan: Box<ScriptChan+Send>,
-        }
-
-        impl AsyncResponseTarget for XHRListener {
-            fn invoke_with_listener(&self, action: ResponseAction) {
-                self.script_chan.send(ScriptMsg::RunnableMsg(box XHRRunnable {
-                    context: self.context.clone(),
-                    action: action
-                })).unwrap();
-            }
-        }
-
-        let listener = box XHRListener {
             context: context,
             script_chan: script_chan,
         };
@@ -635,7 +580,7 @@ impl<'a> XMLHttpRequestMethods for JSRef<'a, XMLHttpRequest> {
         debug!("request_headers = {:?}", *self.request_headers.borrow());
 
         self.fetch_time.set(time::now().to_timespec().sec);
-        let rv = self.fetch2(load_data, cors_request, global.r());
+        let rv = self.fetch(load_data, cors_request, global.r());
         if self.sync.get() {
             return rv;
         }
@@ -798,8 +743,8 @@ trait PrivateXMLHttpRequestHelpers {
     fn cancel_timeout(self);
     fn filter_response_headers(self) -> Headers;
     fn discard_subsequent_responses(self);
-    fn fetch2(self, load_data: LoadData, cors_request: Result<Option<CORSRequest>,()>,
+    fn fetch(self, load_data: LoadData, cors_request: Result<Option<CORSRequest>,()>,
-              global: GlobalRef) -> ErrorResult;
+             global: GlobalRef) -> ErrorResult;
 }
 
 impl<'a> PrivateXMLHttpRequestHelpers for JSRef<'a, XMLHttpRequest> {
@@ -1119,8 +1064,7 @@ impl<'a> PrivateXMLHttpRequestHelpers for JSRef<'a, XMLHttpRequest> {
         self.response_status.set(Err(()));
     }
 
-    #[allow(unsafe_code)]
+    fn fetch(self,
-    fn fetch2(self,
               load_data: LoadData,
               cors_request: Result<Option<CORSRequest>,()>,
               global: GlobalRef) -> ErrorResult {

components/script/lib.rs

@@ -61,6 +61,7 @@ pub mod dom;
 pub mod parse;
 
 pub mod layout_interface;
+mod network_listener;
 pub mod page;
 pub mod script_task;
 mod timers;

components/script/network_listener.rs

@@ -0,0 +1,48 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use script_task::{ScriptChan, ScriptMsg, Runnable};
+use net_traits::{AsyncResponseTarget, AsyncResponseListener, ResponseAction};
+use std::sync::{Arc, Mutex};
+
+/// An off-thread sink for async network event runnables. All such events are forwarded to
+/// a target thread, where they are invoked on the provided context object.
+pub struct NetworkListener<T: AsyncResponseListener + PreInvoke + Send + 'static> {
+    pub context: Arc<Mutex<T>>,
+    pub script_chan: Box<ScriptChan+Send>,
+}
+
+impl<T: AsyncResponseListener + PreInvoke + Send + 'static> AsyncResponseTarget for NetworkListener<T> {
+    fn invoke_with_listener(&self, action: ResponseAction) {
+        self.script_chan.send(ScriptMsg::RunnableMsg(box ListenerRunnable {
+            context: self.context.clone(),
+            action: action,
+        })).unwrap();
+    }
+}
+
+/// A gating mechanism that runs before invoking the runnable on the target thread.
+/// If the `should_invoke` method returns false, the runnable is discarded without
+/// being invoked.
+pub trait PreInvoke {
+    fn should_invoke(&self) -> bool {
+        true
+    }
+}
+
+/// A runnable for moving the async network events between threads.
+struct ListenerRunnable<T: AsyncResponseListener + PreInvoke + Send> {
+    context: Arc<Mutex<T>>,
+    action: ResponseAction,
+}
+
+impl<T: AsyncResponseListener + PreInvoke + Send> Runnable for ListenerRunnable<T> {
+    fn handler(self: Box<ListenerRunnable<T>>) {
+        let this = *self;
+        let context = this.context.lock().unwrap();
+        if context.should_invoke() {
+            this.action.process(&*context);
+        }
+    }
+}