Auto merge of #8037 - eefriedman:net-send-sniff, r=jdm

Consistently use content-type sniffing with all protocols.



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

components/net/about_loader.rs

@@ -9,7 +9,7 @@ use hyper::mime::{Mime, SubLevel, TopLevel};
 use mime_classifier::MIMEClassifier;
 use net_traits::ProgressMsg::Done;
 use net_traits::{LoadConsumer, LoadData, Metadata};
-use resource_task::{send_error, start_sending};
+use resource_task::{send_error, start_sending_sniffed_opt};
 use std::fs::PathExt;
 use std::sync::Arc;
 use url::Url;
@@ -18,14 +18,16 @@ use util::resource_files::resources_dir_path;
 pub fn factory(mut load_data: LoadData, start_chan: LoadConsumer, classifier: Arc<MIMEClassifier>) {
     match load_data.url.non_relative_scheme_data().unwrap() {
         "blank" => {
-            let chan = start_sending(start_chan, Metadata {
+            let metadata = Metadata {
                 final_url: load_data.url,
                 content_type: Some(ContentType(Mime(TopLevel::Text, SubLevel::Html, vec![]))),
                 charset: Some("utf-8".to_owned()),
                 headers: None,
                 status: Some(RawStatus(200, "OK".into())),
-            });
+            };
-            chan.send(Done(Ok(()))).unwrap();
+            if let Ok(chan) = start_sending_sniffed_opt(start_chan, metadata, classifier, &[]) {
+                let _ = chan.send(Done(Ok(())));
+            }
             return
         }
         "crash" => panic!("Loading the about:crash URL."),

components/net/data_loader.rs

@@ -6,21 +6,21 @@ use hyper::mime::{Mime, TopLevel, SubLevel, Attr, Value};
 use mime_classifier::MIMEClassifier;
 use net_traits::ProgressMsg::{Done, Payload};
 use net_traits::{LoadConsumer, LoadData, Metadata};
-use resource_task::{send_error, start_sending};
+use resource_task::{send_error, start_sending_sniffed_opt};
 use rustc_serialize::base64::FromBase64;
 use std::sync::Arc;
 use url::SchemeData;
 use url::percent_encoding::percent_decode;
 
-pub fn factory(load_data: LoadData, senders: LoadConsumer, _classifier: Arc<MIMEClassifier>) {
+pub fn factory(load_data: LoadData, senders: LoadConsumer, classifier: Arc<MIMEClassifier>) {
     // NB: we don't spawn a new task.
     // Hypothesis: data URLs are too small for parallel base64 etc. to be worth it.
     // Should be tested at some point.
     // Left in separate function to allow easy moving to a task, if desired.
-    load(load_data, senders)
+    load(load_data, senders, classifier)
 }
 
-pub fn load(load_data: LoadData, start_chan: LoadConsumer) {
+pub fn load(load_data: LoadData, start_chan: LoadConsumer, classifier: Arc<MIMEClassifier>) {
     let url = load_data.url;
     assert!(&*url.scheme == "data");
 
@@ -62,27 +62,25 @@ pub fn load(load_data: LoadData, start_chan: LoadConsumer) {
         content_type = Some(Mime(TopLevel::Text, SubLevel::Plain,
                                  vec!((Attr::Charset, Value::Ext("US-ASCII".to_owned())))));
     }
-    let mut metadata = Metadata::default(url);
-    metadata.set_content_type(content_type.as_ref());
 
-    let progress_chan = start_sending(start_chan, metadata);
     let bytes = percent_decode(parts[1].as_bytes());
 
-    if is_base64 {
+    let bytes = if is_base64 {
         // FIXME(#2909): It’s unclear what to do with non-alphabet characters,
         // but Acid 3 apparently depends on spaces being ignored.
         let bytes = bytes.into_iter().filter(|&b| b != ' ' as u8).collect::<Vec<u8>>();
         match bytes.from_base64() {
-            Err(..) => {
+            Err(..) => return send_error(url, "non-base64 data uri".to_owned(), start_chan),
-                progress_chan.send(Done(Err("non-base64 data uri".to_owned()))).unwrap();
+            Ok(data) => data,
-            }
-            Ok(data) => {
-                progress_chan.send(Payload(data)).unwrap();
-                progress_chan.send(Done(Ok(()))).unwrap();
-            }
         }
     } else {
-        progress_chan.send(Payload(bytes)).unwrap();
+        bytes
-        progress_chan.send(Done(Ok(()))).unwrap();
+    };
+
+    let mut metadata = Metadata::default(url);
+    metadata.set_content_type(content_type.as_ref());
+    if let Ok(chan) = start_sending_sniffed_opt(start_chan, metadata, classifier, &bytes) {
+        let _ = chan.send(Payload(bytes));
+        let _ = chan.send(Done(Ok(())));
     }
 }

components/net/file_loader.rs

@@ -5,7 +5,7 @@
 use mime_classifier::MIMEClassifier;
 use net_traits::ProgressMsg::{Done, Payload};
 use net_traits::{LoadConsumer, LoadData, Metadata};
-use resource_task::{ProgressSender, send_error, start_sending, start_sending_sniffed};
+use resource_task::{ProgressSender, send_error, start_sending_sniffed, start_sending_sniffed_opt};
 use std::borrow::ToOwned;
 use std::error::Error;
 use std::fs::File;
@@ -52,19 +52,28 @@ pub fn factory(load_data: LoadData, senders: LoadConsumer, classifier: Arc<MIMEC
             Ok(file_path) => {
                 match File::open(&file_path) {
                     Ok(ref mut reader) => {
-                        let metadata = Metadata::default(url);
+                        match read_block(reader) {
-                        let res = read_block(reader);
-                        let (res, progress_chan) = match res {
                             Ok(ReadStatus::Partial(buf)) => {
+                                let metadata = Metadata::default(url);
                                 let progress_chan = start_sending_sniffed(senders, metadata,
                                                                           classifier, &buf);
                                 progress_chan.send(Payload(buf)).unwrap();
-                                (read_all(reader, &progress_chan), progress_chan)
+                                let res = read_all(reader, &progress_chan);
+                                let _ = progress_chan.send(Done(res));
+                            }
+                            Ok(ReadStatus::EOF) => {
+                                let metadata = Metadata::default(url);
+                                if let Ok(chan) = start_sending_sniffed_opt(senders,
+                                                                            metadata,
+                                                                            classifier,
+                                                                            &[]) {
+                                    let _ = chan.send(Done(Ok(())));
+                                }
+                            }
+                            Err(e) => {
+                                send_error(url, e, senders);
                             }
-                            Ok(ReadStatus::EOF) | Err(_) =>
-                                (res.map(|_| ()), start_sending(senders, metadata)),
                         };
-                        progress_chan.send(Done(res)).unwrap();
                     }
                     Err(e) => {
                         send_error(url, e.description().to_owned(), senders);

components/net/resource_task.rs

@@ -59,11 +59,6 @@ pub fn send_error(url: Url, err: String, start_chan: LoadConsumer) {
     }
 }
 
-/// For use by loaders in responding to a Load message.
-pub fn start_sending(start_chan: LoadConsumer, metadata: Metadata) -> ProgressSender {
-    start_sending_opt(start_chan, metadata).ok().unwrap()
-}
-
 /// For use by loaders in responding to a Load message that allows content sniffing.
 pub fn start_sending_sniffed(start_chan: LoadConsumer, metadata: Metadata,
                              classifier: Arc<MIMEClassifier>, partial_body: &[u8])
@@ -122,7 +117,7 @@ fn apache_bug_predicate(last_raw_content_type: &[u8]) -> ApacheBugFlag {
 }
 
 /// For use by loaders in responding to a Load message.
-pub fn start_sending_opt(start_chan: LoadConsumer, metadata: Metadata) -> Result<ProgressSender, ()> {
+fn start_sending_opt(start_chan: LoadConsumer, metadata: Metadata) -> Result<ProgressSender, ()> {
     match start_chan {
         LoadConsumer::Channel(start_chan) => {
             let (progress_chan, progress_port) = ipc::channel().unwrap();
@@ -253,8 +248,7 @@ impl ResourceManager {
             "about" => from_factory(about_loader::factory),
             _ => {
                 debug!("resource_task: no loader for scheme {}", load_data.url.scheme);
-                start_sending(consumer, Metadata::default(load_data.url))
+                send_error(load_data.url, "no loader for scheme".to_owned(), consumer);
-                    .send(ProgressMsg::Done(Err("no loader for scheme".to_owned()))).unwrap();
                 return
             }
         };

tests/unit/net/data_loader.rs

@@ -17,11 +17,14 @@ fn assert_parse(url:          &'static str,
                 charset:      Option<String>,
                 data:         Option<Vec<u8>>) {
     use net::data_loader::load;
+    use net::mime_classifier::MIMEClassifier;
+    use std::sync::Arc;
     use std::sync::mpsc::channel;
     use url::Url;
 
     let (start_chan, start_port) = ipc::channel().unwrap();
-    load(LoadData::new(Url::parse(url).unwrap(), None), Channel(start_chan));
+    let classifier = Arc::new(MIMEClassifier::new());
+    load(LoadData::new(Url::parse(url).unwrap(), None), Channel(start_chan), classifier);
 
     let response = start_port.recv().unwrap();
     assert_eq!(&response.metadata.content_type, &content_type);