servo: Merge #14526 - Attempt at refactoring and simplifying 'set cookies' operations on resource thread (from frewsxcv:cookies); r=jdm
authorCorey Farwell <coreyf@rwell.org>
Thu, 15 Dec 2016 15:54:39 -0800
changeset 340358 974397ff81151f5cf14e34bb5f9b3a34e6c1dab1
parent 340357 df53d42238307b1447262b9de74b27c0c967f2da
child 340359 2aa2650d550452708dc570d9e51aa6269830fa48
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
servo: Merge #14526 - Attempt at refactoring and simplifying 'set cookies' operations on resource thread (from frewsxcv:cookies); r=jdm Source-Repo: https://github.com/servo/servo Source-Revision: bde8dce756902b1b3ad2cff4a5b5bed1f3325852
servo/components/net/resource_thread.rs
servo/components/net_traits/lib.rs
servo/components/script/dom/document.rs
servo/components/script/dom/websocket.rs
servo/components/script/webdriver_handlers.rs
--- a/servo/components/net/resource_thread.rs
+++ b/servo/components/net/resource_thread.rs
@@ -9,17 +9,16 @@ use cookie;
 use cookie_rs;
 use cookie_storage::CookieStorage;
 use devtools_traits::DevtoolsControlMsg;
 use fetch::methods::{FetchContext, fetch};
 use filemanager_thread::{FileManager, TFDProvider};
 use hsts::HstsList;
 use http_loader::HttpState;
 use hyper::client::pool::Pool;
-use hyper::header::{Header, SetCookie};
 use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcReceiver, IpcReceiverSet, IpcSender};
 use net_traits::{CookieSource, CoreResourceThread};
 use net_traits::{CoreResourceMsg, FetchResponseMsg};
 use net_traits::{CustomResponseMediator, ResourceId};
 use net_traits::{ResourceThreads, WebSocketCommunicate, WebSocketConnectData};
 use net_traits::request::{Request, RequestInit};
 use net_traits::storage_thread::StorageThreadMsg;
@@ -154,20 +153,23 @@ impl ResourceChannelManager {
     fn process_msg(&mut self,
                    msg: CoreResourceMsg,
                    group: &ResourceGroup) -> bool {
         match msg {
             CoreResourceMsg::Fetch(init, sender) =>
                 self.resource_manager.fetch(init, sender, group),
             CoreResourceMsg::WebsocketConnect(connect, connect_data) =>
                 self.resource_manager.websocket_connect(connect, connect_data, group),
-            CoreResourceMsg::SetCookiesForUrl(request, cookie_list, source) =>
-                self.resource_manager.set_cookies_for_url(request, cookie_list, source, group),
-            CoreResourceMsg::SetCookiesForUrlWithData(request, cookie, source) =>
-                self.resource_manager.set_cookies_for_url_with_data(request, cookie, source, group),
+            CoreResourceMsg::SetCookieForUrl(request, cookie, source) =>
+                self.resource_manager.set_cookie_for_url(&request, cookie, source, group),
+            CoreResourceMsg::SetCookiesForUrl(request, cookies, source) => {
+                for cookie in cookies {
+                    self.resource_manager.set_cookie_for_url(&request, cookie.0, source, group);
+                }
+            }
             CoreResourceMsg::GetCookiesForUrl(url, consumer, source) => {
                 let mut cookie_jar = group.cookie_jar.write().unwrap();
                 consumer.send(cookie_jar.cookies_for_url(&url, source)).unwrap();
             }
             CoreResourceMsg::NetworkMediator(mediator_chan) => {
                 self.resource_manager.swmanager_chan = Some(mediator_chan)
             }
             CoreResourceMsg::GetCookiesDataForUrl(url, consumer, source) => {
@@ -301,34 +303,18 @@ impl CoreResourceManager {
             user_agent: user_agent,
             devtools_chan: devtools_channel,
             swmanager_chan: None,
             filemanager: FileManager::new(),
             cancel_load_map: HashMap::new(),
         }
     }
 
-    fn set_cookies_for_url(&mut self,
-                           request: ServoUrl,
-                           cookie_list: String,
-                           source: CookieSource,
-                           resource_group: &ResourceGroup) {
-        let header = Header::parse_header(&[cookie_list.into_bytes()]);
-        if let Ok(SetCookie(cookies)) = header {
-            for bare_cookie in cookies {
-                if let Some(cookie) = cookie::Cookie::new_wrapped(bare_cookie, &request, source) {
-                    let mut cookie_jar = resource_group.cookie_jar.write().unwrap();
-                    cookie_jar.push(cookie, source);
-                }
-            }
-        }
-    }
-
-    fn set_cookies_for_url_with_data(&mut self, request: ServoUrl, cookie: cookie_rs::Cookie, source: CookieSource,
-                                     resource_group: &ResourceGroup) {
+    fn set_cookie_for_url(&mut self, request: &ServoUrl, cookie: cookie_rs::Cookie, source: CookieSource,
+                          resource_group: &ResourceGroup) {
         if let Some(cookie) = cookie::Cookie::new_wrapped(cookie, &request, source) {
             let mut cookie_jar = resource_group.cookie_jar.write().unwrap();
             cookie_jar.push(cookie, source)
         }
     }
 
     fn fetch(&self,
              init: RequestInit,
--- a/servo/components/net_traits/lib.rs
+++ b/servo/components/net_traits/lib.rs
@@ -350,26 +350,26 @@ pub struct WebSocketConnectData {
     pub protocols: Vec<String>,
 }
 
 #[derive(Deserialize, Serialize)]
 pub enum CoreResourceMsg {
     Fetch(RequestInit, IpcSender<FetchResponseMsg>),
     /// Try to make a websocket connection to a URL.
     WebsocketConnect(WebSocketCommunicate, WebSocketConnectData),
-    /// Store a set of cookies for a given originating URL
-    SetCookiesForUrl(ServoUrl, String, CookieSource),
-    /// Store a set of cookies for a given originating URL
-    SetCookiesForUrlWithData(
+    /// Store a cookie for a given originating URL
+    SetCookieForUrl(
         ServoUrl,
         #[serde(deserialize_with = "::hyper_serde::deserialize",
                 serialize_with = "::hyper_serde::serialize")]
         Cookie,
         CookieSource
     ),
+    /// Store cookies for a given originating URL
+    SetCookiesForUrl(ServoUrl, Vec<Serde<Cookie>>, CookieSource),
     /// Retrieve the stored cookies for a given URL
     GetCookiesForUrl(ServoUrl, IpcSender<Option<String>>, CookieSource),
     /// Get a cookie by name for a given originating URL
     GetCookiesDataForUrl(ServoUrl, IpcSender<Vec<Serde<Cookie>>>, CookieSource),
     /// Cancel a network request corresponding to a given `ResourceId`
     Cancel(ResourceId),
     /// Synchronization message solely for knowing the state of the ResourceChannelManager loop
     Synchronize(IpcSender<()>),
--- a/servo/components/script/dom/document.rs
+++ b/servo/components/script/dom/document.rs
@@ -88,16 +88,18 @@ use dom::uievent::UIEvent;
 use dom::webglcontextevent::WebGLContextEvent;
 use dom::window::{ReflowReason, Window};
 use encoding::EncodingRef;
 use encoding::all::UTF_8;
 use euclid::point::Point2D;
 use gfx_traits::ScrollRootId;
 use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
 use html5ever_atoms::{LocalName, QualName};
+use hyper::header::{Header, SetCookie};
+use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcSender};
 use js::jsapi::{JSContext, JSObject, JSRuntime};
 use js::jsapi::JS_GetRuntime;
 use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
 use msg::constellation_msg::{FrameId, Key, KeyModifiers, KeyState};
 use net_traits::{FetchResponseMsg, IpcSend, ReferrerPolicy};
 use net_traits::CookieSource::NonHTTP;
 use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl};
@@ -2955,21 +2957,25 @@ impl DocumentMethods for Document {
         if self.is_cookie_averse() {
             return Ok(());
         }
 
         if !self.origin.is_scheme_host_port_tuple() {
             return Err(Error::Security);
         }
 
-        let url = self.url();
-        let _ = self.window
-                    .upcast::<GlobalScope>()
-                    .resource_threads()
-                    .send(SetCookiesForUrl(url, String::from(cookie), NonHTTP));
+        let header = Header::parse_header(&[cookie.into()]);
+        if let Ok(SetCookie(cookies)) = header {
+            let cookies = cookies.into_iter().map(Serde).collect();
+            let _ = self.window
+                        .upcast::<GlobalScope>()
+                        .resource_threads()
+                        .send(SetCookiesForUrl(self.url(), cookies, NonHTTP));
+        }
+
         Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-bgcolor
     fn BgColor(&self) -> DOMString {
         self.get_body_attribute(&local_name!("bgcolor"))
     }
 
--- a/servo/components/script/dom/websocket.rs
+++ b/servo/components/script/dom/websocket.rs
@@ -17,16 +17,18 @@ use dom::bindings::reflector::{DomObject
 use dom::bindings::str::{DOMString, USVString, is_token};
 use dom::blob::{Blob, BlobImpl};
 use dom::closeevent::CloseEvent;
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::EventTarget;
 use dom::globalscope::GlobalScope;
 use dom::messageevent::MessageEvent;
 use dom::urlhelper::UrlHelper;
+use hyper;
+use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 use js::jsapi::{JS_GetArrayBufferData, JS_NewArrayBuffer};
 use js::jsapi::JSAutoCompartment;
 use js::jsval::UndefinedValue;
 use libc::{uint32_t, uint8_t};
 use net_traits::{WebSocketCommunicate, WebSocketConnectData, WebSocketDomAction, WebSocketNetworkEvent};
 use net_traits::CookieSource::HTTP;
 use net_traits::CoreResourceMsg::{SetCookiesForUrl, WebsocketConnect};
@@ -491,23 +493,20 @@ impl Runnable for ConnectionEstablishedT
 
         // Step 4: Protocols.
         let protocol_in_use = unwrap_websocket_protocol(self.headers.get::<WebSocketProtocol>());
         if let Some(protocol_name) = protocol_in_use {
             *ws.protocol.borrow_mut() = protocol_name.to_owned();
         };
 
         // Step 5: Cookies.
-        if let Some(cookies) = self.headers.get_raw("set-cookie") {
-            for cookie in cookies.iter() {
-                if let Ok(cookie_value) = String::from_utf8(cookie.clone()) {
-                    let _ = ws.global().core_resource_thread().send(
-                        SetCookiesForUrl(ws.url.clone(), cookie_value, HTTP));
-                }
-            }
+        if let Some(cookies) = self.headers.get::<hyper::header::SetCookie>() {
+            let cookies = cookies.iter().map(|c| Serde(c.clone())).collect();
+            let _ = ws.global().core_resource_thread().send(
+                SetCookiesForUrl(ws.url.clone(), cookies, HTTP));
         }
 
         // Step 6.
         ws.upcast().fire_event(atom!("open"));
     }
 }
 
 struct BufferedAmountTask {
--- a/servo/components/script/webdriver_handlers.rs
+++ b/servo/components/script/webdriver_handlers.rs
@@ -26,17 +26,17 @@ use euclid::point::Point2D;
 use euclid::rect::Rect;
 use euclid::size::Size2D;
 use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcSender};
 use js::jsapi::{HandleValue, JSContext};
 use js::jsval::UndefinedValue;
 use msg::constellation_msg::PipelineId;
 use net_traits::CookieSource::{HTTP, NonHTTP};
-use net_traits::CoreResourceMsg::{GetCookiesDataForUrl, SetCookiesForUrlWithData};
+use net_traits::CoreResourceMsg::{GetCookiesDataForUrl, SetCookieForUrl};
 use net_traits::IpcSend;
 use script_thread::Documents;
 use script_traits::webdriver_msg::{WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue};
 use script_traits::webdriver_msg::WebDriverCookieError;
 use servo_url::ServoUrl;
 
 fn find_node_by_unique_id(documents: &Documents,
                           pipeline: PipelineId,
@@ -238,24 +238,24 @@ pub fn handle_add_cookie(documents: &Doc
         HTTP
     } else {
         NonHTTP
     };
     reply.send(match (document.is_cookie_averse(), cookie.domain.clone()) {
         (true, _) => Err(WebDriverCookieError::InvalidDomain),
         (false, Some(ref domain)) if url.host_str().map(|x| { x == &**domain }).unwrap_or(false) => {
             let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
-                SetCookiesForUrlWithData(url, cookie, method)
-                );
+                SetCookieForUrl(url, cookie, method)
+            );
             Ok(())
         },
         (false, None) => {
             let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
-                SetCookiesForUrlWithData(url, cookie, method)
-                );
+                SetCookieForUrl(url, cookie, method)
+            );
             Ok(())
         },
         (_, _) => {
             Err(WebDriverCookieError::UnableToSetCookie)
         },
     }).unwrap();
 }