Refactor resource loader protocol to send metadata first

src/components/net/file_loader.rs

@@ -2,7 +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 resource_task::{Done, LoaderTask, Payload};
+use resource_task::{Metadata, Payload, Done, LoaderTask, start_sending};
 
 use std::io::{ReaderUtil, file_reader};
 use std::task;
@@ -10,10 +10,10 @@ use std::task;
 static READ_SIZE: uint = 1024;
 
 pub fn factory() -> LoaderTask {
-    let f: LoaderTask = |url, progress_chan| {
+    let f: LoaderTask = |url, start_chan| {
         assert!("file" == url.scheme);
+        let progress_chan = start_sending(start_chan, Metadata::default(url.clone()));
         do task::spawn {
-            // FIXME: Resolve bug prevents us from moving the path out of the URL.
             match file_reader(&Path(url.path)) {
                 Ok(reader) => {
                     while !reader.eof() {

src/components/net/http_loader.rs

@@ -2,7 +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 resource_task::{ProgressMsg, Payload, Done, UrlChange, LoaderTask};
+use resource_task::{Metadata, Payload, Done, LoadResponse, LoaderTask, start_sending};
 
 use std::cell::Cell;
 use std::vec;
@@ -13,15 +13,15 @@ use http::headers::HeaderEnum;
 use std::rt::io::Reader;
 
 pub fn factory() -> LoaderTask {
-    let f: LoaderTask = |url, progress_chan| {
+    let f: LoaderTask = |url, start_chan| {
         let url = Cell::new(url);
-        let progress_chan = Cell::new(progress_chan);
+        let start_chan = Cell::new(start_chan);
-        spawn(|| load(url.take(), progress_chan.take()))
+        spawn(|| load(url.take(), start_chan.take()))
     };
     f
 }
 
-fn load(url: Url, progress_chan: Chan<ProgressMsg>) {
+fn load(url: Url, start_chan: Chan<LoadResponse>) {
     assert!("http" == url.scheme);
 
     info!("requesting %s", url.to_str());
@@ -30,7 +30,7 @@ fn load(url: Url, progress_chan: Chan<ProgressMsg>) {
     let mut response = match request.read_response() {
         Ok(r) => r,
         Err(_) => {
-            progress_chan.send(Done(Err(())));
+            start_sending(start_chan, Metadata::default(url)).send(Done(Err(())));
             return;
         }
     };
@@ -52,12 +52,15 @@ fn load(url: Url, progress_chan: Chan<ProgressMsg>) {
     match redirect {
         Some(url) => {
             info!("redirecting to %s", url.to_str());
-            progress_chan.send(UrlChange(url.clone()));
+            return load(url, start_chan);
-            return load(url, progress_chan);
         }
         None => ()
     }
 
+    let mut metadata = Metadata::default(url);
+    // We will set other fields here.
+
+    let progress_chan = start_sending(start_chan, metadata);
     loop {
         let mut buf = vec::with_capacity(1024);
 

src/components/net/image_cache_task.rs

@@ -442,9 +442,9 @@ fn load_image_data(url: Url, resource_task: ResourceTask) -> Result<~[u8], ()> {
 
     let mut image_data = ~[];
 
+    let progress_port = response_port.recv().progress_port;
     loop {
-        match response_port.recv() {
+        match progress_port.recv() {
-            resource_task::UrlChange(*) => (),  // don't care that URL changed
             resource_task::Payload(data) => {
                 image_data.push_all(data);
             }

src/components/net/resource_task.rs

@@ -9,31 +9,69 @@ use http_loader;
 
 use std::cell::Cell;
 use std::comm::{Chan, Port, SharedChan};
+use std::comm;
 use extra::url::Url;
 use util::spawn_listener;
 
 pub enum ControlMsg {
     /// Request the data associated with a particular URL
-    Load(Url, Chan<ProgressMsg>),
+    Load(Url, Chan<LoadResponse>),
     Exit
 }
 
+/// Metadata about a loaded resource, such as is obtained from HTTP headers.
+pub struct Metadata {
+    /// Final URL after redirects.
+    final_url: Url,
+
+    // Other fields (e.g. content type, charset) will go here.
+}
+
+impl Metadata {
+    /// Metadata with defaults for everything optional.
+    pub fn default(url: Url) -> Metadata {
+        Metadata {
+            final_url: url,
+        }
+    }
+}
+
+/// Message sent in response to `Load`.  Contains metadata, and a port
+/// for receiving the data.
+///
+/// Even if loading fails immediately, we send one of these and the
+/// progress_port will provide the error.
+pub struct LoadResponse {
+    /// Metadata, such as from HTTP headers.
+    metadata: Metadata,
+    /// Port for reading data.
+    progress_port: Port<ProgressMsg>,
+}
+
 /// Messages sent in response to a `Load` message
 #[deriving(Eq)]
 pub enum ProgressMsg {
-    /// URL changed due to a redirect.  There can be zero or more of these,
-    /// but they are guaranteed to arrive before messages of any other type.
-    UrlChange(Url),
     /// Binary data - there may be multiple of these
     Payload(~[u8]),
     /// Indicates loading is complete, either successfully or not
     Done(Result<(), ()>)
 }
 
+/// For use by loaders in responding to a Load message.
+pub fn start_sending(start_chan: Chan<LoadResponse>,
+                     metadata:   Metadata) -> Chan<ProgressMsg> {
+    let (progress_port, progress_chan) = comm::stream();
+    start_chan.send(LoadResponse {
+        metadata:      metadata,
+        progress_port: progress_port,
+    });
+    progress_chan
+}
+
 /// Handle to a resource task
 pub type ResourceTask = SharedChan<ControlMsg>;
 
-pub type LoaderTask = ~fn(url: Url, Chan<ProgressMsg>);
+pub type LoaderTask = ~fn(url: Url, Chan<LoadResponse>);
 
 /**
 Creates a task to load a specific resource
@@ -81,8 +119,8 @@ impl ResourceManager {
     fn start(&self) {
         loop {
             match self.from_client.recv() {
-              Load(url, progress_chan) => {
+              Load(url, start_chan) => {
-                self.load(url.clone(), progress_chan)
+                self.load(url.clone(), start_chan)
               }
               Exit => {
                 break
@@ -91,16 +129,15 @@ impl ResourceManager {
         }
     }
 
-    fn load(&self, url: Url, progress_chan: Chan<ProgressMsg>) {
+    fn load(&self, url: Url, start_chan: Chan<LoadResponse>) {
-
         match self.get_loader_factory(&url) {
             Some(loader_factory) => {
                 debug!("resource_task: loading url: %s", url.to_str());
-                loader_factory(url, progress_chan);
+                loader_factory(url, start_chan);
             }
             None => {
                 debug!("resource_task: no loader for scheme %s", url.scheme);
-                progress_chan.send(Done(Err(())));
+                start_sending(start_chan, Metadata::default(url)).send(Done(Err(())));
             }
         }
     }
@@ -140,7 +177,8 @@ fn test_bad_scheme() {
 #[test]
 fn should_delegate_to_scheme_loader() {
     let payload = ~[1, 2, 3];
-    let loader_factory = |_url: Url, progress_chan: Chan<ProgressMsg>| {
+    let loader_factory = |url: Url, start_chan: Chan<LoadResponse>| {
+        let progress_chan = start_sending(start_chan, Metadata::default(url));
         progress_chan.send(Payload(payload.clone()));
         progress_chan.send(Done(Ok(())));
     };

src/components/script/html/cssparse.rs

@@ -10,7 +10,7 @@ use std::comm::Port;
 use std::task;
 use newcss::stylesheet::Stylesheet;
 use newcss::util::DataStream;
-use servo_net::resource_task::{ResourceTask, ProgressMsg, Load, Payload, Done, UrlChange};
+use servo_net::resource_task::{Load, LoadResponse, Payload, Done, ResourceTask};
 use extra::url::Url;
 
 /// Where a style sheet comes from.
@@ -55,21 +55,13 @@ fn data_stream(provenance: StylesheetProvenance, resource_task: ResourceTask) ->
     }
 }
 
-fn resource_port_to_data_stream(input_port: Port<ProgressMsg>) -> DataStream {
+fn resource_port_to_data_stream(input_port: Port<LoadResponse>) -> DataStream {
+    let progress_port = input_port.recv().progress_port;
     return || {
-        // Can't just 'return' the value since we're inside a lambda
+        match progress_port.recv() {
-        let mut result = None;
+            Payload(data) => Some(data),
-        loop {
+            Done(*) => None
-            match input_port.recv() {
-                UrlChange(*) => (),  // don't care that URL changed
-                Payload(data) => {
-                    result = Some(data);
-                    break;
-                }
-                Done(*) => break
-            }
         }
-        result
     }
 }
 

src/components/script/html/hubbub_html_parser.rs

@@ -25,7 +25,7 @@ use std::from_str::FromStr;
 use hubbub::hubbub;
 use servo_msg::constellation_msg::{ConstellationChan, SubpageId};
 use servo_net::image_cache_task::ImageCacheTask;
-use servo_net::resource_task::{ProgressMsg, Done, Load, Payload, UrlChange, ResourceTask};
+use servo_net::resource_task::{Load, Payload, Done, ResourceTask};
 use servo_util::tree::TreeNodeRef;
 use servo_util::url::make_url;
 use extra::url::Url;
@@ -170,10 +170,10 @@ fn js_script_listener(to_parent: SharedChan<HtmlDiscoveryMessage>,
                     // TODO: change copy to move once we can move into closures
                     resource_task.send(Load(url.clone(), input_chan));
 
+                    let progress_port = input_port.recv().progress_port;
                     let mut buf = ~[];
                     loop {
-                        match input_port.recv() {
+                        match progress_port.recv() {
-                            UrlChange(*) => (),  // don't care that URL changed
                             Payload(data) => {
                                 buf.push_all(data);
                             }
@@ -331,25 +331,13 @@ pub fn parse_html(cx: *JSContext,
     }
     let js_chan = SharedChan::new(js_msg_chan);
 
-    // Process any UrlChange messages before we build the parser, because the
+    // Wait for the LoadResponse so that the parser knows the final URL.
-    // tree handler functions need to know the final URL.
-    let mut final_url = url.clone();
     let (input_port, input_chan) = comm::stream();
     resource_task.send(Load(url.clone(), input_chan));
-    let mut progress_msg: ProgressMsg;
+    let load_response = input_port.recv();
-    loop {
-        progress_msg = input_port.recv();
-        match progress_msg {
-            UrlChange(url) => {
-                debug!("page URL changed to %s", url.to_str());
-                final_url = url;
-            }
-            _ => break
-        }
-    }
 
-    let url2 = final_url.clone();
+    let url2 = load_response.metadata.final_url.clone();
-    let url3 = final_url.clone();
+    let url3 = url2.clone();
 
     // Build the root node.
     let root = @HTMLHtmlElement { htmlelement: HTMLElement::new(HTMLHtmlElementTypeId, ~"html", document) };
@@ -573,11 +561,7 @@ pub fn parse_html(cx: *JSContext,
 
     debug!("loaded page");
     loop {
-        // We already have a message from the earlier UrlChange processing.
+        match load_response.progress_port.recv() {
-        match progress_msg {
-            UrlChange(*) => {
-                fail!("got UrlChange message after others");
-            }
             Payload(data) => {
                 debug!("received data");
                 parser.parse_chunk(data);
@@ -589,7 +573,6 @@ pub fn parse_html(cx: *JSContext,
                 break;
             }
         }
-        progress_msg = input_port.recv();
     }
 
     css_chan.send(CSSTaskExit);
@@ -598,7 +581,7 @@ pub fn parse_html(cx: *JSContext,
     HtmlParserResult {
         root: root,
         discovery_port: discovery_port,
-        url: final_url,
+        url: load_response.metadata.final_url,
     }
 }