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

@@ -14,7 +14,6 @@ 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};
@@ -159,10 +158,13 @@ impl ResourceChannelManager {
                 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();
@@ -306,24 +308,8 @@ impl CoreResourceManager {
         }
     }
 
-    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)

servo/components/net_traits/lib.rs

@@ -355,16 +355,16 @@ 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

servo/components/script/dom/document.rs

@@ -93,6 +93,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;
@@ -2960,11 +2962,15 @@ impl DocumentMethods for Document {
             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(())
     }
 

servo/components/script/dom/websocket.rs

@@ -22,6 +22,8 @@ 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;
@@ -496,13 +498,10 @@ impl Runnable for ConnectionEstablishedTask {
         };
 
         // 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.

servo/components/script/webdriver_handlers.rs

@@ -31,7 +31,7 @@ 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};
@@ -243,14 +243,14 @@ pub fn handle_add_cookie(documents: &Documents,
         (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(())
         },
         (_, _) => {