Auto merge of #8216 - akumar21NCSU:master, r=jdm

M1502: Improve HTTP monitoring devtool support

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8216)
<!-- Reviewable:end -->

components/devtools_traits/lib.rs

@@ -264,6 +264,7 @@ pub struct HttpRequest {
     pub method: Method,
     pub headers: Headers,
     pub body: Option<Vec<u8>>,
+    pub pipeline_id: PipelineId,
 }
 
 #[derive(Debug, PartialEq)]
@@ -271,6 +272,7 @@ pub struct HttpResponse {
     pub headers: Option<Headers>,
     pub status: Option<RawStatus>,
     pub body: Option<Vec<u8>>,
+    pub pipeline_id: PipelineId,
 }
 
 pub enum NetworkEvent {

components/net/Cargo.toml

@@ -26,6 +26,9 @@ path = "../plugins"
 version = "0.6"
 features = [ "serde-serialization" ]
 
+[dependencies.msg]
+path = "../msg"
+
 [dependencies.ipc-channel]
 git = "https://github.com/pcwalton/ipc-channel"
 

components/net/http_loader.rs

@@ -23,6 +23,7 @@ use hyper::net::{Fresh, HttpsConnector, Openssl};
 use hyper::status::{StatusClass, StatusCode};
 use log;
 use mime_classifier::MIMEClassifier;
+use msg::constellation_msg::{PipelineId};
 use net_traits::ProgressMsg::{Done, Payload};
 use net_traits::hosts::replace_hosts;
 use net_traits::{CookieSource, IncludeSubdomains, LoadConsumer, LoadData, Metadata};
@@ -446,10 +447,12 @@ fn send_request_to_devtools(devtools_chan: Option<Sender<DevtoolsControlMsg>>,
                             url: Url,
                             method: Method,
                             headers: Headers,
-                            body: Option<Vec<u8>>) {
+                            body: Option<Vec<u8>>,
+                            pipeline_id: PipelineId) {
 
     if let Some(ref chan) = devtools_chan {
-        let request = DevtoolsHttpRequest { url: url, method: method, headers: headers, body: body };
+        let request = DevtoolsHttpRequest {
+            url: url, method: method, headers: headers, body: body, pipeline_id: pipeline_id };
         let net_event = NetworkEvent::HttpRequest(request);
 
         let msg = ChromeToDevtoolsControlMsg::NetworkEvent(request_id, net_event);
@@ -460,9 +463,10 @@ fn send_request_to_devtools(devtools_chan: Option<Sender<DevtoolsControlMsg>>,
 fn send_response_to_devtools(devtools_chan: Option<Sender<DevtoolsControlMsg>>,
                              request_id: String,
                              headers: Option<Headers>,
-                             status: Option<RawStatus>) {
+                             status: Option<RawStatus>,
+                             pipeline_id: PipelineId) {
     if let Some(ref chan) = devtools_chan {
-        let response = DevtoolsHttpResponse { headers: headers, status: status, body: None };
+        let response = DevtoolsHttpResponse { headers: headers, status: status, body: None, pipeline_id: pipeline_id };
         let net_event_response = NetworkEvent::HttpResponse(response);
 
         let msg = ChromeToDevtoolsControlMsg::NetworkEvent(request_id, net_event_response);
@@ -600,35 +604,30 @@ pub fn load<A>(load_data: LoadData,
             //
             // https://tools.ietf.org/html/rfc7231#section-6.4
             let is_redirected_request = iters != 1;
+            let cloned_data;
             let maybe_response = match load_data.data {
                 Some(ref data) if !is_redirected_request => {
                     req.headers_mut().set(ContentLength(data.len() as u64));
-
+                    cloned_data = load_data.data.clone();
-                    // TODO: Do this only if load_data has some pipeline_id, and send the pipeline_id
-                    // in the message
-                    send_request_to_devtools(
-                        devtools_chan.clone(), request_id.clone(), url.clone(),
-                        method.clone(), load_data.headers.clone(),
-                        load_data.data.clone()
-                    );
-
                     req.send(&load_data.data)
                 }
                 _ => {
                     if load_data.method != Method::Get && load_data.method != Method::Head {
                         req.headers_mut().set(ContentLength(0))
                     }
-
+                    cloned_data = None;
-                    send_request_to_devtools(
-                        devtools_chan.clone(), request_id.clone(), url.clone(),
-                        method.clone(), load_data.headers.clone(),
-                        None
-                    );
-
                     req.send(&None)
                 }
             };
 
+            if let Some(pipeline_id) = load_data.pipeline_id {
+                send_request_to_devtools(
+                    devtools_chan.clone(), request_id.clone(), url.clone(),
+                    method.clone(), request_headers.clone(),
+                    cloned_data, pipeline_id
+                );
+}
+
             response = match maybe_response {
                 Ok(r) => r,
                 Err(LoadError::ConnectionAborted(reason)) => {
@@ -704,13 +703,13 @@ pub fn load<A>(load_data: LoadData,
 
         // --- Tell devtools that we got a response
         // Send an HttpResponse message to devtools with the corresponding request_id
-        // TODO: Send this message only if load_data has a pipeline_id that is not None
         // TODO: Send this message even when the load fails?
-        send_response_to_devtools(
+        if let Some(pipeline_id) = load_data.pipeline_id {
-            devtools_chan, request_id,
+                send_response_to_devtools(
-            metadata.headers.clone(), metadata.status.clone()
+                    devtools_chan, request_id,
-        );
+                    metadata.headers.clone(), metadata.status.clone(),
-
+                    pipeline_id);
+         }
         return StreamedResponse::from_http_response(response, metadata)
     }
 }

components/net/image_cache_task.rs

@@ -461,7 +461,7 @@ pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
         placeholder_url.push("rippy.jpg");
         let placeholder_image = match Url::from_file_path(&*placeholder_url) {
             Ok(url) => {
-                match load_whole_resource(&resource_task, url) {
+                match load_whole_resource(&resource_task, url, None) {
                     Err(..) => {
                         debug!("image_cache_task: failed loading the placeholder.");
                         None

components/net/lib.rs

@@ -20,6 +20,7 @@ extern crate flate2;
 extern crate brotli;
 extern crate hyper;
 extern crate ipc_channel;
+extern crate msg;
 extern crate net_traits;
 extern crate openssl;
 extern crate rustc_serialize;

components/net_traits/lib.rs

@@ -383,10 +383,10 @@ pub enum ProgressMsg {
 }
 
 /// Convenience function for synchronously loading a whole resource.
-pub fn load_whole_resource(resource_task: &ResourceTask, url: Url)
+pub fn load_whole_resource(resource_task: &ResourceTask, url: Url, pipeline_id: Option<PipelineId>)
         -> Result<(Metadata, Vec<u8>), String> {
     let (start_chan, start_port) = ipc::channel().unwrap();
-    resource_task.send(ControlMsg::Load(LoadData::new(url, None),
+    resource_task.send(ControlMsg::Load(LoadData::new(url, pipeline_id),
                        LoadConsumer::Channel(start_chan))).unwrap();
     let response = start_port.recv().unwrap();
 

components/script/document_loader.rs

@@ -78,6 +78,7 @@ impl DocumentLoader {
         pending.load_async(listener)
     }
 
+
     /// Mark an in-progress network request complete.
     pub fn finish_load(&mut self, load: LoadType) {
         let idx = self.blocking_loads.iter().position(|unfinished| *unfinished == load);

components/script/dom/dedicatedworkerglobalscope.rs

@@ -231,7 +231,7 @@ impl DedicatedWorkerGlobalScope {
             let roots = RootCollection::new();
             let _stack_roots_tls = StackRootTLS::new(&roots);
 
-            let (url, source) = match load_whole_resource(&init.resource_task, worker_url) {
+            let (url, source) = match load_whole_resource(&init.resource_task, worker_url, None) {
                 Err(_) => {
                     println!("error loading script {}", serialized_worker_url);
                     parent_sender.send(CommonScriptMsg::RunnableMsg(WorkerEvent,

components/script/dom/workerglobalscope.rs

@@ -193,7 +193,7 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope {
         }
 
         for url in urls {
-            let (url, source) = match load_whole_resource(&self.resource_task, url) {
+            let (url, source) = match load_whole_resource(&self.resource_task, url, None) {
                 Err(_) => return Err(Error::Network),
                 Ok((metadata, bytes)) => {
                     (metadata.final_url, String::from_utf8(bytes).unwrap())

components/servo/Cargo.lock

@@ -1180,6 +1180,7 @@ dependencies = [
  "hyper 0.6.15 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipc-channel 0.1.0 (git+https://github.com/pcwalton/ipc-channel)",
  "log 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "msg 0.0.1",
  "net_traits 0.0.1",
  "openssl 0.6.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "plugins 0.0.1",
@@ -1201,6 +1202,7 @@ dependencies = [
  "flate2 0.2.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "hyper 0.6.15 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipc-channel 0.1.0 (git+https://github.com/pcwalton/ipc-channel)",
+ "msg 0.0.1",
  "net 0.0.1",
  "net_traits 0.0.1",
  "time 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",

tests/unit/net/Cargo.toml

@@ -17,6 +17,9 @@ path = "../../../components/net_traits"
 [dependencies.util]
 path = "../../../components/util"
 
+[dependencies.msg]
+path = "../../../components/msg"
+
 [dependencies.devtools_traits]
 path = "../../../components/devtools_traits"
 

tests/unit/net/http_loader.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+
 use cookie_rs::Cookie as CookiePair;
 use devtools_traits::HttpRequest as DevtoolsHttpRequest;
 use devtools_traits::HttpResponse as DevtoolsHttpResponse;
@@ -15,6 +16,7 @@ use hyper::http::RawStatus;
 use hyper::method::Method;
 use hyper::mime::{Mime, SubLevel, TopLevel};
 use hyper::status::StatusCode;
+use msg::constellation_msg::PipelineId;
 use net::cookie::Cookie;
 use net::cookie_storage::CookieStorage;
 use net::hsts::{HSTSList};
@@ -382,7 +384,9 @@ fn test_request_and_response_data_with_network_messages() {
 
     let url = Url::parse("https://mozilla.com").unwrap();
     let (devtools_chan, devtools_port) = mpsc::channel::<DevtoolsControlMsg>();
-    let mut load_data = LoadData::new(url.clone(), None);
+    // This will probably have to be changed as it uses fake_root_pipeline_id which is marked for removal.
+    let pipeline_id = PipelineId::fake_root_pipeline_id();
+    let mut load_data = LoadData::new(url.clone(), Some(pipeline_id));
     let mut request_headers = Headers::new();
     request_headers.set(Host { hostname: "bar.foo".to_owned(), port: None });
     load_data.headers = request_headers.clone();
@@ -393,11 +397,29 @@ fn test_request_and_response_data_with_network_messages() {
     let devhttprequest = expect_devtools_http_request(&devtools_port);
     let devhttpresponse = expect_devtools_http_response(&devtools_port);
 
+    //Creating default headers for request
+    let mut headers = Headers::new();
+    headers.set(AcceptEncoding(vec![
+                                   qitem(Encoding::Gzip),
+                                   qitem(Encoding::Deflate),
+                                   qitem(Encoding::EncodingExt("br".to_owned()))
+                                   ]));
+    headers.set(Host { hostname: "mozilla.com".to_owned() , port: None });
+    let accept = Accept(vec![
+                            qitem(Mime(TopLevel::Text, SubLevel::Html, vec![])),
+                            qitem(Mime(TopLevel::Application, SubLevel::Ext("xhtml+xml".to_owned()), vec![])),
+                            QualityItem::new(Mime(TopLevel::Application, SubLevel::Xml, vec![]), Quality(900u16)),
+                            QualityItem::new(Mime(TopLevel::Star, SubLevel::Star, vec![]), Quality(800u16)),
+                            ]);
+    headers.set(accept);
+    headers.set(UserAgent(DEFAULT_USER_AGENT.to_string()));
+
     let httprequest = DevtoolsHttpRequest {
         url: url,
         method: Method::Get,
-        headers: request_headers,
+        headers: headers,
         body: None,
+        pipeline_id: pipeline_id,
     };
 
     let content = "Yay!";
@@ -408,13 +430,45 @@ fn test_request_and_response_data_with_network_messages() {
     let httpresponse = DevtoolsHttpResponse {
         headers: Some(response_headers),
         status: Some(RawStatus(200, Cow::Borrowed("Ok"))),
-        body: None
+        body: None,
+        pipeline_id: pipeline_id,
     };
 
     assert_eq!(devhttprequest, httprequest);
     assert_eq!(devhttpresponse, httpresponse);
 }
 
+#[test]
+fn test_request_and_response_message_from_devtool_without_pipeline_id() {
+    struct Factory;
+
+    impl HttpRequestFactory for Factory {
+        type R = MockRequest;
+
+        fn create(&self, _: Url, _: Method) -> Result<MockRequest, LoadError> {
+            let mut headers = Headers::new();
+            headers.set(Host { hostname: "foo.bar".to_owned(), port: None });
+            Ok(MockRequest::new(
+                   ResponseType::WithHeaders(<[_]>::to_vec("Yay!".as_bytes()), headers))
+            )
+        }
+    }
+
+    let hsts_list = Arc::new(RwLock::new(HSTSList::new()));
+    let cookie_jar = Arc::new(RwLock::new(CookieStorage::new()));
+
+    let url = Url::parse("https://mozilla.com").unwrap();
+    let (devtools_chan, devtools_port) = mpsc::channel::<DevtoolsControlMsg>();
+    let load_data = LoadData::new(url.clone(), None);
+    let _ = load::<MockRequest>(load_data, hsts_list, cookie_jar, Some(devtools_chan), &Factory,
+                                DEFAULT_USER_AGENT.to_string());
+
+    // notification received from devtools
+    assert!(devtools_port.try_recv().is_err());
+}
+
+
+
 #[test]
 fn test_load_when_redirecting_from_a_post_should_rewrite_next_request_as_get() {
     struct Factory;

tests/unit/net/lib.rs

@@ -7,6 +7,7 @@ extern crate devtools_traits;
 extern crate flate2;
 extern crate hyper;
 extern crate ipc_channel;
+extern crate msg;
 extern crate net;
 extern crate net_traits;
 extern crate time;