Auto merge of #16288 - nox:net, r=jdm

Kill ResourceGroup

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16288)
<!-- Reviewable:end -->

components/net/connector.rs

@@ -12,7 +12,6 @@ use openssl::ssl::{SslConnectorBuilder, SslMethod};
 use std::io;
 use std::net::TcpStream;
 use std::path::PathBuf;
-use std::sync::Arc;
 
 pub struct HttpsConnector {
     ssl: OpensslClient,
@@ -63,9 +62,9 @@ pub fn create_ssl_client(ca_file: &PathBuf) -> OpensslClient {
     OpensslClient::from(ssl_connector)
 }
 
-pub fn create_http_connector(ssl_client: OpensslClient) -> Arc<Pool<Connector>> {
+pub fn create_http_connector(ssl_client: OpensslClient) -> Pool<Connector> {
     let https_connector = HttpsConnector::new(ssl_client);
-    Arc::new(Pool::with_connector(Default::default(), https_connector))
+    Pool::with_connector(Default::default(), https_connector)
 }
 
 // The basic logic here is to prefer ciphers with ECDSA certificates, Forward

components/net/fetch/methods.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use blob_loader::load_blob_sync;
-use connector::Connector;
 use data_loader::decode;
 use devtools_traits::DevtoolsControlMsg;
 use fetch::cors_cache::CorsCache;
@@ -11,7 +10,6 @@ use filemanager_thread::FileManager;
 use http_loader::{HttpState, determine_request_referrer, http_fetch};
 use http_loader::{set_default_accept, set_default_accept_language};
 use hyper::{Error, Result as HyperResult};
-use hyper::client::Pool;
 use hyper::header::{Accept, AcceptLanguage, ContentLanguage, ContentType};
 use hyper::header::{Header, HeaderFormat, HeaderView, Headers, Referer as RefererHeader};
 use hyper::method::Method;
@@ -45,7 +43,6 @@ pub struct FetchContext {
     pub user_agent: Cow<'static, str>,
     pub devtools_chan: Option<Sender<DevtoolsControlMsg>>,
     pub filemanager: FileManager,
-    pub connector: Arc<Pool<Connector>>,
 }
 
 pub type DoneChannel = Option<(Sender<Data>, Receiver<Data>)>;

components/net/http_loader.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use brotli::Decompressor;
-use connector::Connector;
+use connector::{Connector, create_http_connector};
 use cookie;
 use cookie_storage::CookieStorage;
 use devtools_traits::{ChromeToDevtoolsControlMsg, DevtoolsControlMsg, HttpRequest as DevtoolsHttpRequest};
@@ -27,6 +27,7 @@ use hyper::header::{Pragma, Quality, QualityItem, Referer, SetCookie};
 use hyper::header::{UserAgent, q, qitem};
 use hyper::method::Method;
 use hyper::status::StatusCode;
+use hyper_openssl::OpensslClient;
 use hyper_serde::Serde;
 use log;
 use msg::constellation_msg::PipelineId;
@@ -43,7 +44,7 @@ use std::io::{self, Read, Write};
 use std::iter::FromIterator;
 use std::mem;
 use std::ops::Deref;
-use std::sync::{Arc, RwLock};
+use std::sync::RwLock;
 use std::sync::mpsc::{channel, Sender};
 use std::thread;
 use time;
@@ -68,14 +69,18 @@ pub struct HttpState {
     pub hsts_list: RwLock<HstsList>,
     pub cookie_jar: RwLock<CookieStorage>,
     pub auth_cache: RwLock<AuthCache>,
+    pub ssl_client: OpensslClient,
+    pub connector: Pool<Connector>,
 }
 
 impl HttpState {
-    pub fn new() -> HttpState {
+    pub fn new(ssl_client: OpensslClient) -> HttpState {
         HttpState {
             hsts_list: RwLock::new(HstsList::new()),
             cookie_jar: RwLock::new(CookieStorage::new(150)),
             auth_cache: RwLock::new(AuthCache::new()),
+            ssl_client: ssl_client.clone(),
+            connector: create_http_connector(ssl_client),
         }
     }
 }
@@ -389,7 +394,7 @@ fn auth_from_cache(auth_cache: &RwLock<AuthCache>, origin: &ImmutableOrigin) ->
     }
 }
 
-fn obtain_response(connector: Arc<Pool<Connector>>,
+fn obtain_response(connector: &Pool<Connector>,
                    url: &ServoUrl,
                    method: &Method,
                    request_headers: &Headers,
@@ -819,14 +824,11 @@ fn http_network_or_cache_fetch(request: &mut Request,
     };
 
     // Step 11
-    if !http_request.omit_origin_header {
+    if cors_flag || (http_request.method != Method::Get && http_request.method != Method::Head) {
-        let method = &http_request.method;
+        debug_assert!(http_request.origin != Origin::Client);
-        if cors_flag || (*method != Method::Get && *method != Method::Head) {
+        if let Origin::Origin(ref url_origin) = http_request.origin {
-            debug_assert!(http_request.origin != Origin::Client);
+            if let Some(hyper_origin) = try_immutable_origin_to_hyper_origin(url_origin) {
-            if let Origin::Origin(ref url_origin) = http_request.origin {
+                http_request.headers.set(hyper_origin)
-                if let Some(hyper_origin) = try_immutable_origin_to_hyper_origin(url_origin) {
-                    http_request.headers.set(hyper_origin)
-                }
             }
         }
     }
@@ -1079,7 +1081,9 @@ fn http_network_fetch(request: &Request,
     // do not. Once we support other kinds of fetches we'll need to be more fine grained here
     // since things like image fetches are classified differently by devtools
     let is_xhr = request.destination == Destination::None;
-    let wrapped_response = obtain_response(context.connector.clone(), &url, &request.method,
+    let wrapped_response = obtain_response(&context.state.connector,
+                                           &url,
+                                           &request.method,
                                            &request.headers,
                                            &request.body, &request.method,
                                            &request.pipeline_id, request.redirect_count + 1,

components/net/resource_thread.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! A thread that takes a URL and streams back the binary data.
-use connector::{Connector, create_http_connector, create_ssl_client};
+use connector::{create_http_connector, create_ssl_client};
 use cookie;
 use cookie_rs;
 use cookie_storage::CookieStorage;
@@ -12,8 +12,6 @@ use fetch::methods::{FetchContext, fetch};
 use filemanager_thread::{FileManager, TFDProvider};
 use hsts::HstsList;
 use http_loader::HttpState;
-use hyper::client::pool::Pool;
-use hyper_openssl::OpensslClient;
 use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcReceiver, IpcReceiverSet, IpcSender};
 use net_traits::{CookieSource, CoreResourceThread};
@@ -43,13 +41,6 @@ use websocket_loader;
 
 const TFD_PROVIDER: &'static TFDProvider = &TFDProvider;
 
-#[derive(Clone)]
-pub struct ResourceGroup {
-    http_state: Arc<HttpState>,
-    ssl_client: OpensslClient,
-    connector: Arc<Pool<Connector>>,
-}
-
 /// Returns a tuple of (public, private) senders to the new threads.
 pub fn new_resource_threads(user_agent: Cow<'static, str>,
                             devtools_chan: Option<Sender<DevtoolsControlMsg>>,
@@ -95,8 +86,7 @@ struct ResourceChannelManager {
     config_dir: Option<PathBuf>,
 }
 
-fn create_resource_groups(config_dir: Option<&Path>)
+fn create_http_states(config_dir: Option<&Path>) -> (Arc<HttpState>, Arc<HttpState>) {
-                          -> (ResourceGroup, ResourceGroup) {
     let mut hsts_list = HstsList::from_servo_preload();
     let mut auth_cache = AuthCache::new();
     let mut cookie_jar = CookieStorage::new(150);
@@ -105,11 +95,6 @@ fn create_resource_groups(config_dir: Option<&Path>)
         read_json_from_file(&mut hsts_list, config_dir, "hsts_list.json");
         read_json_from_file(&mut cookie_jar, config_dir, "cookie_jar.json");
     }
-    let http_state = HttpState {
-        cookie_jar: RwLock::new(cookie_jar),
-        auth_cache: RwLock::new(auth_cache),
-        hsts_list: RwLock::new(hsts_list),
-    };
 
     let ca_file = match opts::get().certificate_path {
         Some(ref path) => PathBuf::from(path),
@@ -117,20 +102,20 @@ fn create_resource_groups(config_dir: Option<&Path>)
             .expect("Need certificate file to make network requests")
             .join("certs"),
     };
-    let ssl_client = create_ssl_client(&ca_file);
 
-    let resource_group = ResourceGroup {
+    let ssl_client = create_ssl_client(&ca_file);
-        http_state: Arc::new(http_state),
+    let http_state = HttpState {
+        cookie_jar: RwLock::new(cookie_jar),
+        auth_cache: RwLock::new(auth_cache),
+        hsts_list: RwLock::new(hsts_list),
         ssl_client: ssl_client.clone(),
-        connector: create_http_connector(ssl_client.clone()),
+        connector: create_http_connector(ssl_client),
     };
+
     let private_ssl_client = create_ssl_client(&ca_file);
-    let private_resource_group = ResourceGroup {
+    let private_http_state = HttpState::new(private_ssl_client);
-        http_state: Arc::new(HttpState::new()),
+
-        ssl_client: private_ssl_client.clone(),
+    (Arc::new(http_state), Arc::new(private_http_state))
-        connector: create_http_connector(private_ssl_client),
-    };
-    (resource_group, private_resource_group)
 }
 
 impl ResourceChannelManager {
@@ -138,8 +123,8 @@ impl ResourceChannelManager {
     fn start(&mut self,
              public_receiver: IpcReceiver<CoreResourceMsg>,
              private_receiver: IpcReceiver<CoreResourceMsg>) {
-        let (public_resource_group, private_resource_group) =
+        let (public_http_state, private_http_state) =
-            create_resource_groups(self.config_dir.as_ref().map(Deref::deref));
+            create_http_states(self.config_dir.as_ref().map(Deref::deref));
 
         let mut rx_set = IpcReceiverSet::new().unwrap();
         let private_id = rx_set.add(private_receiver).unwrap();
@@ -148,10 +133,10 @@ impl ResourceChannelManager {
         loop {
             for (id, data) in rx_set.select().unwrap().into_iter().map(|m| m.unwrap()) {
                 let group = if id == private_id {
-                    &private_resource_group
+                    &private_http_state
                 } else {
                     assert_eq!(id, public_id);
-                    &public_resource_group
+                    &public_http_state
                 };
                 if let Ok(msg) = data.to() {
                     if !self.process_msg(msg, group) {
@@ -166,28 +151,28 @@ impl ResourceChannelManager {
     /// Returns false if the thread should exit.
     fn process_msg(&mut self,
                    msg: CoreResourceMsg,
-                   group: &ResourceGroup) -> bool {
+                   http_state: &Arc<HttpState>) -> bool {
         match msg {
             CoreResourceMsg::Fetch(init, sender) =>
-                self.resource_manager.fetch(init, sender, group),
+                self.resource_manager.fetch(init, sender, http_state),
             CoreResourceMsg::WebsocketConnect(connect, connect_data) =>
-                self.resource_manager.websocket_connect(connect, connect_data, group),
+                self.resource_manager.websocket_connect(connect, connect_data, http_state),
             CoreResourceMsg::SetCookieForUrl(request, cookie, source) =>
-                self.resource_manager.set_cookie_for_url(&request, cookie.into_inner(), source, group),
+                self.resource_manager.set_cookie_for_url(&request, cookie.into_inner(), source, http_state),
             CoreResourceMsg::SetCookiesForUrl(request, cookies, source) => {
                 for cookie in cookies {
-                    self.resource_manager.set_cookie_for_url(&request, cookie.into_inner(), source, group);
+                    self.resource_manager.set_cookie_for_url(&request, cookie.into_inner(), source, http_state);
                 }
             }
             CoreResourceMsg::GetCookiesForUrl(url, consumer, source) => {
-                let mut cookie_jar = group.http_state.cookie_jar.write().unwrap();
+                let mut cookie_jar = http_state.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) => {
-                let mut cookie_jar = group.http_state.cookie_jar.write().unwrap();
+                let mut cookie_jar = http_state.cookie_jar.write().unwrap();
                 let cookies = cookie_jar.cookies_data_for_url(&url, source).map(Serde).collect();
                 consumer.send(cookies).unwrap();
             }
@@ -203,15 +188,15 @@ impl ResourceChannelManager {
             CoreResourceMsg::ToFileManager(msg) => self.resource_manager.filemanager.handle(msg, TFD_PROVIDER),
             CoreResourceMsg::Exit(sender) => {
                 if let Some(ref config_dir) = self.config_dir {
-                    match group.http_state.auth_cache.read() {
+                    match http_state.auth_cache.read() {
                         Ok(auth_cache) => write_json_to_file(&*auth_cache, config_dir, "auth_cache.json"),
                         Err(_) => warn!("Error writing auth cache to disk"),
                     }
-                    match group.http_state.cookie_jar.read() {
+                    match http_state.cookie_jar.read() {
                         Ok(jar) => write_json_to_file(&*jar, config_dir, "cookie_jar.json"),
                         Err(_) => warn!("Error writing cookie jar to disk"),
                     }
-                    match group.http_state.hsts_list.read() {
+                    match http_state.hsts_list.read() {
                         Ok(hsts) => write_json_to_file(&*hsts, config_dir, "hsts_list.json"),
                         Err(_) => warn!("Error writing hsts list to disk"),
                     }
@@ -325,9 +310,9 @@ impl CoreResourceManager {
     fn set_cookie_for_url(&mut self, request: &ServoUrl,
                           cookie: cookie_rs::Cookie<'static>,
                           source: CookieSource,
-                          resource_group: &ResourceGroup) {
+                          http_state: &Arc<HttpState>) {
         if let Some(cookie) = cookie::Cookie::new_wrapped(cookie, request, source) {
-            let mut cookie_jar = resource_group.http_state.cookie_jar.write().unwrap();
+            let mut cookie_jar = http_state.cookie_jar.write().unwrap();
             cookie_jar.push(cookie, request, source)
         }
     }
@@ -335,12 +320,11 @@ impl CoreResourceManager {
     fn fetch(&self,
              init: RequestInit,
              mut sender: IpcSender<FetchResponseMsg>,
-             group: &ResourceGroup) {
+             http_state: &Arc<HttpState>) {
-        let http_state = group.http_state.clone();
+        let http_state = http_state.clone();
         let ua = self.user_agent.clone();
         let dc = self.devtools_chan.clone();
         let filemanager = self.filemanager.clone();
-        let connector = group.connector.clone();
 
         thread::Builder::new().name(format!("fetch thread for {}", init.url)).spawn(move || {
             let mut request = Request::from_init(init);
@@ -353,7 +337,6 @@ impl CoreResourceManager {
                 user_agent: ua,
                 devtools_chan: dc,
                 filemanager: filemanager,
-                connector: connector,
             };
             fetch(&mut request, &mut sender, &context);
         }).expect("Thread spawning failed");
@@ -362,9 +345,7 @@ impl CoreResourceManager {
     fn websocket_connect(&self,
                          connect: WebSocketCommunicate,
                          connect_data: WebSocketConnectData,
-                         resource_grp: &ResourceGroup) {
+                         http_state: &Arc<HttpState>) {
-        websocket_loader::init(connect,
+        websocket_loader::init(connect, connect_data, http_state.clone());
-                               connect_data,
-                               resource_grp.http_state.clone());
     }
 }

components/net_traits/request.rs

@@ -220,7 +220,6 @@ pub struct Request {
     // TODO: priority object
     /// https://fetch.spec.whatwg.org/#concept-request-origin
     pub origin: Origin,
-    pub omit_origin_header: bool,
     /// https://fetch.spec.whatwg.org/#concept-request-referrer
     pub referrer: Referrer,
     /// https://fetch.spec.whatwg.org/#concept-request-referrer-policy
@@ -273,7 +272,6 @@ impl Request {
             type_: Type::None,
             destination: Destination::None,
             origin: origin.unwrap_or(Origin::Client),
-            omit_origin_header: false,
             referrer: Referrer::Client,
             referrer_policy: None,
             pipeline_id: pipeline_id,

components/script/dom/request.rs

@@ -161,7 +161,6 @@ impl Request {
         request.window = window;
         // TODO: `entry settings object` is not implemented in Servo yet.
         request.origin = Origin::Client;
-        request.omit_origin_header = temporary_request.omit_origin_header;
         request.referrer = temporary_request.referrer;
         request.referrer_policy = temporary_request.referrer_policy;
         request.mode = temporary_request.mode;
@@ -187,10 +186,8 @@ impl Request {
                     request.mode = NetTraitsRequestMode::SameOrigin;
                 }
                 // Step 13.2
-                request.omit_origin_header = false;
-                // Step 13.3
                 request.referrer = NetTraitsRequestReferrer::Client;
-                // Step 13.4
+                // Step 13.3
                 request.referrer_policy = None;
             }
 

tests/unit/net/fetch.rs

@@ -23,7 +23,7 @@ use hyper::status::StatusCode;
 use hyper::uri::RequestUri;
 use hyper_openssl;
 use msg::constellation_msg::TEST_PIPELINE_ID;
-use net::connector::{create_http_connector, create_ssl_client};
+use net::connector::create_ssl_client;
 use net::fetch::cors_cache::CorsCache;
 use net::fetch::methods::FetchContext;
 use net::filemanager_thread::FileManager;
@@ -532,14 +532,12 @@ fn test_fetch_with_hsts() {
 
     let ca_file = resources_dir_path().unwrap().join("self_signed_certificate_for_testing.crt");
     let ssl_client = create_ssl_client(&ca_file);
-    let connector = create_http_connector(ssl_client);
 
     let context =  FetchContext {
-        state: Arc::new(HttpState::new()),
+        state: Arc::new(HttpState::new(ssl_client)),
         user_agent: DEFAULT_USER_AGENT.into(),
         devtools_chan: None,
         filemanager: FileManager::new(),
-        connector: connector,
     };
 
     {

tests/unit/net/lib.rs

@@ -56,13 +56,11 @@ struct FetchResponseCollector {
 fn new_fetch_context(dc: Option<Sender<DevtoolsControlMsg>>) -> FetchContext {
     let ca_file = resources_dir_path().unwrap().join("certs");
     let ssl_client = create_ssl_client(&ca_file);
-    let connector = create_http_connector(ssl_client);
     FetchContext {
-        state: Arc::new(HttpState::new()),
+        state: Arc::new(HttpState::new(ssl_client)),
         user_agent: DEFAULT_USER_AGENT.into(),
         devtools_chan: dc,
         filemanager: FileManager::new(),
-        connector: connector,
     }
 }
 impl FetchTaskTarget for FetchResponseCollector {