Cleanup CORSCache

Remove the CORSCache trait, CORSCacheSender, CORSCacheThreadMsg, and
CORSCacheThread. Rename BasicCORSCache to CORSCache and keep its old
implementation of CORSCache.

components/net/fetch/cors_cache.rs

@@ -12,7 +12,6 @@
 use hyper::method::Method;
 use net_traits::request::Origin;
 use std::ascii::AsciiExt;
-use std::sync::mpsc::{Sender, Receiver, channel};
 use time;
 use time::{now, Timespec};
 use url::Url;
@@ -74,51 +73,20 @@ pub struct CacheRequestDetails {
     pub credentials: bool
 }
 
-/// Trait for a generic CORS Cache
-pub trait CORSCache {
-    /// [Clear the cache](https://fetch.spec.whatwg.org/#concept-cache-clear)
-    fn clear (&mut self, request: CacheRequestDetails);
-
-    /// Remove old entries
-    fn cleanup(&mut self);
-
-    /// Returns true if an entry with a
-    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool;
-
-    /// Updates max age if an entry for a
-    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found.
-    ///
-    /// If not, it will insert an equivalent entry
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool;
-
-    /// Returns true if an entry with a
-    /// [matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool;
-
-    /// Updates max age if an entry for
-    /// [a matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found.
-    ///
-    /// If not, it will insert an equivalent entry
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool;
-    /// Insert an entry
-    fn insert(&mut self, entry: CORSCacheEntry);
-}
-
-/// A simple, vector-based CORS Cache
-#[derive(Clone)]
-pub struct BasicCORSCache(Vec<CORSCacheEntry>);
-
 fn match_headers(cors_cache: &CORSCacheEntry, cors_req: &CacheRequestDetails) -> bool {
     cors_cache.origin == cors_req.origin &&
         cors_cache.url == cors_req.destination &&
         (cors_cache.credentials || !cors_req.credentials)
 }
 
-impl BasicCORSCache {
+/// A simple, vector-based CORS Cache
+#[derive(Clone)]
+pub struct CORSCache(Vec<CORSCacheEntry>);
 
-    pub fn new() -> BasicCORSCache {
+impl CORSCache {
-        BasicCORSCache(vec![])
+
+    pub fn new() -> CORSCache {
+        CORSCache(vec![])
     }
 
     fn find_entry_by_header<'a>(&'a mut self, request: &CacheRequestDetails,
@@ -133,33 +101,37 @@ impl BasicCORSCache {
         self.cleanup();
         self.0.iter_mut().find(|e| match_headers(e, request) && e.header_or_method.match_method(&method))
     }
-}
 
-impl CORSCache for BasicCORSCache {
+    /// [Clear the cache](https://fetch.spec.whatwg.org/#concept-cache-clear)
-    /// https://fetch.spec.whatwg.org/#concept-cache-clear
+    pub fn clear (&mut self, request: CacheRequestDetails) {
-    #[allow(dead_code)]
+        let CORSCache(buf) = self.clone();
-    fn clear (&mut self, request: CacheRequestDetails) {
-        let BasicCORSCache(buf) = self.clone();
         let new_buf: Vec<CORSCacheEntry> =
             buf.into_iter().filter(|e| e.origin == request.origin && request.destination == e.url).collect();
-        *self = BasicCORSCache(new_buf);
+        *self = CORSCache(new_buf);
     }
 
-    // Remove old entries
+    /// Remove old entries
-    fn cleanup(&mut self) {
+    pub fn cleanup(&mut self) {
-        let BasicCORSCache(buf) = self.clone();
+        let CORSCache(buf) = self.clone();
         let now = time::now().to_timespec();
         let new_buf: Vec<CORSCacheEntry> = buf.into_iter()
                                               .filter(|e| now.sec < e.created.sec + e.max_age as i64)
                                               .collect();
-        *self = BasicCORSCache(new_buf);
+        *self = CORSCache(new_buf);
     }
 
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
+    /// Returns true if an entry with a
+    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found
+    pub fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
         self.find_entry_by_header(&request, header_name).is_some()
     }
 
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool {
+    /// Updates max age if an entry for a
+    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found.
+    ///
+    /// If not, it will insert an equivalent entry
+    pub fn match_header_and_update(&mut self, request: CacheRequestDetails,
+                                   header_name: &str, new_max_age: u32) -> bool {
         match self.find_entry_by_header(&request, header_name).map(|e| e.max_age = new_max_age) {
             Some(_) => true,
             None => {
@@ -171,11 +143,17 @@ impl CORSCache for BasicCORSCache {
         }
     }
 
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
+    /// Returns true if an entry with a
+    /// [matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found
+    pub fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
         self.find_entry_by_method(&request, method).is_some()
     }
 
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
+    /// Updates max age if an entry for
+    /// [a matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found.
+    ///
+    /// If not, it will insert an equivalent entry
+    pub fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
         match self.find_entry_by_method(&request, method.clone()).map(|e| e.max_age = new_max_age) {
             Some(_) => true,
             None => {
@@ -186,138 +164,9 @@ impl CORSCache for BasicCORSCache {
         }
     }
 
-    fn insert(&mut self, entry: CORSCacheEntry) {
+    /// Insert an entry
+    pub fn insert(&mut self, entry: CORSCacheEntry) {
         self.cleanup();
         self.0.push(entry);
     }
 }
-
-/// Various messages that can be sent to a CORSCacheThread
-pub enum CORSCacheThreadMsg {
-    Clear(CacheRequestDetails, Sender<()>),
-    Cleanup(Sender<()>),
-    MatchHeader(CacheRequestDetails, String, Sender<bool>),
-    MatchHeaderUpdate(CacheRequestDetails, String, u32, Sender<bool>),
-    MatchMethod(CacheRequestDetails, Method, Sender<bool>),
-    MatchMethodUpdate(CacheRequestDetails, Method, u32, Sender<bool>),
-    Insert(CORSCacheEntry, Sender<()>),
-    ExitMsg
-}
-
-/// A Sender to a CORSCacheThread
-///
-/// This can be used as a CORS Cache.
-/// The methods on this type block until they can run, and it behaves similar to a mutex
-pub type CORSCacheSender = Sender<CORSCacheThreadMsg>;
-
-impl CORSCache for CORSCacheSender {
-    fn clear (&mut self, request: CacheRequestDetails) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Clear(request, tx));
-        let _ = rx.recv();
-    }
-
-    fn cleanup(&mut self) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Cleanup(tx));
-        let _ = rx.recv();
-    }
-
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchHeader(request, header_name.to_owned(), tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchHeaderUpdate(request, header_name.to_owned(), new_max_age, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchMethod(request, method, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchMethodUpdate(request, method, new_max_age, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn insert(&mut self, entry: CORSCacheEntry) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Insert(entry, tx));
-        let _ = rx.recv();
-    }
-}
-
-/// A simple thread-based CORS Cache that can be sent messages
-///
-/// #Example
-/// ```ignore
-/// let thread = CORSCacheThread::new();
-/// let builder = ThreadBuilder::new().named("XHRThread");
-/// let mut sender = thread.sender();
-/// builder.spawn(move || { thread.run() });
-/// sender.insert(CORSCacheEntry::new(/* parameters here */));
-/// ```
-pub struct CORSCacheThread {
-    receiver: Receiver<CORSCacheThreadMsg>,
-    cache: BasicCORSCache,
-    sender: CORSCacheSender
-}
-
-impl CORSCacheThread {
-    pub fn new() -> CORSCacheThread {
-        let (tx, rx) = channel();
-        CORSCacheThread {
-            receiver: rx,
-            cache: BasicCORSCache(vec![]),
-            sender: tx
-        }
-    }
-
-    /// Provides a sender to the cache thread
-    pub fn sender(&self) -> CORSCacheSender {
-        self.sender.clone()
-    }
-
-    /// Runs the cache thread
-    /// This blocks the current thread, so it is advised
-    /// to spawn a new thread for this
-    /// Send ExitMsg to the associated Sender to exit
-    pub fn run(&mut self) {
-        loop {
-            match self.receiver.recv().unwrap() {
-                CORSCacheThreadMsg::Clear(request, tx) => {
-                    self.cache.clear(request);
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::Cleanup(tx) => {
-                    self.cache.cleanup();
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::MatchHeader(request, header, tx) => {
-                    let _ = tx.send(self.cache.match_header(request, &header));
-                },
-                CORSCacheThreadMsg::MatchHeaderUpdate(request, header, new_max_age, tx) => {
-                    let _ = tx.send(self.cache.match_header_and_update(request, &header, new_max_age));
-                },
-                CORSCacheThreadMsg::MatchMethod(request, method, tx) => {
-                    let _ = tx.send(self.cache.match_method(request, method));
-                },
-                CORSCacheThreadMsg::MatchMethodUpdate(request, method, new_max_age, tx) => {
-                    let _ = tx.send(self.cache.match_method_and_update(request, method, new_max_age));
-                },
-                CORSCacheThreadMsg::Insert(entry, tx) => {
-                    self.cache.insert(entry);
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::ExitMsg => break
-            }
-        }
-    }
-}

components/net/fetch/methods.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use data_loader::decode;
-use fetch::cors_cache::{BasicCORSCache, CORSCache, CacheRequestDetails};
+use fetch::cors_cache::{CORSCache, CacheRequestDetails};
 use http_loader::{NetworkHttpRequestFactory, create_http_connector, obtain_response};
 use hyper::header::{Accept, AcceptLanguage, Authorization, AccessControlAllowCredentials};
 use hyper::header::{AccessControlAllowOrigin, AccessControlAllowHeaders, AccessControlAllowMethods};
@@ -40,10 +40,10 @@ pub fn fetch_async(request: Request, listener: Box<AsyncFetchListener + Send>) {
 
 /// [Fetch](https://fetch.spec.whatwg.org#concept-fetch)
 pub fn fetch(request: Rc<Request>) -> Response {
-    fetch_with_cors_cache(request, &mut BasicCORSCache::new())
+    fetch_with_cors_cache(request, &mut CORSCache::new())
 }
 
-pub fn fetch_with_cors_cache(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
+pub fn fetch_with_cors_cache(request: Rc<Request>, cache: &mut CORSCache) -> Response {
 
     // Step 1
     if request.window.get() == Window::Client {
@@ -110,7 +110,7 @@ pub fn fetch_with_cors_cache(request: Rc<Request>, cache: &mut BasicCORSCache) -
 }
 
 /// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch)
-fn main_fetch(request: Rc<Request>, cache: &mut BasicCORSCache, cors_flag: bool, recursive_flag: bool) -> Response {
+fn main_fetch(request: Rc<Request>, cache: &mut CORSCache, cors_flag: bool, recursive_flag: bool) -> Response {
     // TODO: Implement main fetch spec
 
     // Step 1
@@ -284,7 +284,7 @@ fn main_fetch(request: Rc<Request>, cache: &mut BasicCORSCache, cors_flag: bool,
 }
 
 /// [Basic fetch](https://fetch.spec.whatwg.org#basic-fetch)
-fn basic_fetch(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
+fn basic_fetch(request: Rc<Request>, cache: &mut CORSCache) -> Response {
 
     let url = request.current_url();
 
@@ -328,7 +328,7 @@ fn basic_fetch(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
 
 /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch)
 fn http_fetch(request: Rc<Request>,
-              cache: &mut BasicCORSCache,
+              cache: &mut CORSCache,
               cors_flag: bool,
               cors_preflight_flag: bool,
               authentication_fetch_flag: bool) -> Response {
@@ -507,7 +507,7 @@ fn http_fetch(request: Rc<Request>,
 
 /// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch)
 fn http_redirect_fetch(request: Rc<Request>,
-                       cache: &mut BasicCORSCache,
+                       cache: &mut CORSCache,
                        response: Rc<Response>,
                        cors_flag: bool) -> Response {
 
@@ -922,7 +922,7 @@ fn http_network_fetch(request: Rc<Request>,
 }
 
 /// [CORS preflight fetch](https://fetch.spec.whatwg.org#cors-preflight-fetch)
-fn cors_preflight_fetch(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
+fn cors_preflight_fetch(request: Rc<Request>, cache: &mut CORSCache) -> Response {
     // Step 1
     let mut preflight = Request::new(request.current_url(), Some(request.origin.borrow().clone()), false);
     *preflight.method.borrow_mut() = Method::Options;

tests/unit/net/fetch.rs

@@ -13,7 +13,7 @@ use hyper::server::{Handler, Listening, Server};
 use hyper::server::{Request as HyperRequest, Response as HyperResponse};
 use hyper::status::StatusCode;
 use hyper::uri::RequestUri;
-use net::fetch::cors_cache::{BasicCORSCache, CacheRequestDetails, CORSCache};
+use net::fetch::cors_cache::{CacheRequestDetails, CORSCache};
 use net::fetch::methods::{fetch, fetch_async, fetch_with_cors_cache};
 use net_traits::AsyncFetchListener;
 use net_traits::request::{Origin, RedirectMode, Referer, Request, RequestMode};
@@ -181,7 +181,7 @@ fn test_cors_preflight_cache_fetch() {
     static ACK: &'static [u8] = b"ACK";
     let state = Arc::new(AtomicUsize::new(0));
     let counter = state.clone();
-    let mut cache = BasicCORSCache::new();
+    let mut cache = CORSCache::new();
     let handler = move |request: HyperRequest, mut response: HyperResponse| {
         if request.method == Method::Options && state.clone().fetch_add(1, Ordering::SeqCst) == 0 {
             assert!(request.headers.has::<AccessControlRequestMethod>());