Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt at refactoring and simplifying 'set cookies' operations on resource thread. #14526

Merged
merged 1 commit into from Dec 16, 2016
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Refactor and simplify 'set cookies' operations on resource thread.

  • Loading branch information
frewsxcv committed Dec 15, 2016
commit 77d2f9de3603b75468bdebf2aa24cc94d176d418
@@ -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)
@@ -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),

This comment has been minimized.

@frewsxcv

frewsxcv Dec 9, 2016

Author Member

I think this Serde<...> abstraction can go away once we bump cookies-rs and use the serde-serialization feature

/// Retrieve the stored cookies for a given URL
GetCookiesForUrl(ServoUrl, IpcSender<Option<String>>, CookieSource),
/// Get a cookie by name for a given originating URL
@@ -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(())
}

@@ -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.
@@ -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(())
},
(_, _) => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.