From 6374d74d6edda7e5db5c2930432c8e3edec4d59d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 10:50:13 +0100 Subject: [PATCH 1/6] Simplify the content_changed call in ScriptTask::load. --- components/script/script_task.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 23ab04374c3..7972b2a378a 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -758,11 +758,7 @@ impl ScriptTask { // Kick off the initial reflow of the page. debug!("kicking off initial reflow of {}", url); - { - let document_js_ref = (&*document).clone(); - let document_as_node = NodeCast::from_ref(document_js_ref); - document.content_changed(document_as_node); - } + document.content_changed(NodeCast::from_ref(*document)); window.flush_layout(); { From 69e13f3267e52d20ab87eccdd45583ad35ed0150 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 11:04:04 +0100 Subject: [PATCH 2/6] Cleanup last_loaded_url/last_url handling in ScriptTask::load. --- components/script/script_task.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 7972b2a378a..5a8caadb3e6 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -693,20 +693,17 @@ impl ScriptTask { message for a layout channel that is not associated with this script task. This is a bug."); - let last_loaded_url = replace(&mut *page.mut_url(), None); - match last_loaded_url { - Some((ref loaded, needs_reflow)) if *loaded == url => { - *page.mut_url() = Some((loaded.clone(), false)); - if needs_reflow { + let last_url = match &mut *page.mut_url() { + &Some((ref mut loaded, ref mut needs_reflow)) if *loaded == url => { + if replace(needs_reflow, false) { self.force_reflow(&*page); } return; }, - _ => (), - } + url => replace(url, None).map(|(loaded, _)| loaded), + }; let is_javascript = url.scheme.as_slice() == "javascript"; - let last_url = last_loaded_url.map(|(ref loaded, _)| loaded.clone()); let cx = self.js_context.borrow(); let cx = cx.as_ref().unwrap(); From 5d0934d8ec7d3596d388549cc48daeffc7af73c3 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 11:39:45 +0100 Subject: [PATCH 3/6] Pass the load data to parse_html directly, rather than in an Option. --- components/script/parse/html.rs | 10 ++++------ components/script/script_task.rs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 70ffc76e6d9..b83743a450a 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -212,17 +212,15 @@ pub fn parse_html(page: &Page, document: JSRef, input: HTMLInput, resource_task: ResourceTask, - msg_load_data: Option) { + msg_load_data: MsgLoadData) { let (base_url, load_response) = match input { InputUrl(ref url) => { // Wait for the LoadResponse so that the parser knows the final URL. let (input_chan, input_port) = channel(); let mut load_data = LoadData::new(url.clone(), input_chan); - msg_load_data.map(|m| { - load_data.headers = m.headers; - load_data.method = m.method; - load_data.data = m.data; - }); + load_data.headers = msg_load_data.headers; + load_data.method = msg_load_data.method; + load_data.data = msg_load_data.data; resource_task.send(Load(load_data)); let load_response = input_port.recv(); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 5a8caadb3e6..7da46d628ce 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -748,7 +748,7 @@ impl ScriptTask { InputString(strval.unwrap_or("".to_string())) }; - parse_html(&*page, *document, parser_input, self.resource_task.clone(), Some(load_data)); + parse_html(&*page, *document, parser_input, self.resource_task.clone(), load_data); url = page.get_url().clone(); document.set_ready_state(DocumentReadyStateValues::Interactive); From de318ae8f13fcbe82ad545bb73b2c0a690e0ea15 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 11:47:34 +0100 Subject: [PATCH 4/6] Simplify the LoadData creation in parse_html. --- components/script/parse/html.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index b83743a450a..1bef0f21c91 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -217,11 +217,14 @@ pub fn parse_html(page: &Page, InputUrl(ref url) => { // Wait for the LoadResponse so that the parser knows the final URL. let (input_chan, input_port) = channel(); - let mut load_data = LoadData::new(url.clone(), input_chan); - load_data.headers = msg_load_data.headers; - load_data.method = msg_load_data.method; - load_data.data = msg_load_data.data; - resource_task.send(Load(load_data)); + resource_task.send(Load(LoadData { + url: url.clone(), + method: msg_load_data.method, + headers: msg_load_data.headers, + data: msg_load_data.data, + cors: None, + consumer: input_chan, + })); let load_response = input_port.recv(); From c7c7dc03ee944ca03748f236dd08601eab5ceb24 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 11:39:53 +0100 Subject: [PATCH 5/6] Move the networking code in parse_html into ScriptTask::load. This doesn't really have anything to do with parsing HTML, and fits in better with the code in ScriptTask::load. In particular, all changes to Page's url now go through ScriptTask methods. --- components/script/parse/html.rs | 96 ++------------------------------ components/script/script_task.rs | 88 ++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 94 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 1bef0f21c91..5f34c87d3da 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -16,72 +16,26 @@ use dom::node::{Node, NodeHelpers, TrustedNodeAddress}; use dom::servohtmlparser; use dom::servohtmlparser::ServoHTMLParser; use dom::text::Text; -use page::Page; use parse::Parser; use encoding::all::UTF_8; use encoding::types::{Encoding, DecodeReplace}; -use servo_net::resource_task::{Load, LoadData, Payload, Done, ResourceTask}; -use servo_msg::constellation_msg::LoadData as MsgLoadData; +use servo_net::resource_task::{Payload, Done, LoadResponse}; use servo_util::task_state; use servo_util::task_state::IN_HTML_PARSER; use std::ascii::AsciiExt; -use std::comm::channel; -use std::fmt::{mod, Show}; use std::str::MaybeOwned; use url::Url; -use time::{Tm, strptime}; use html5ever::Attribute; use html5ever::tree_builder::{TreeSink, QuirksMode, NodeOrText, AppendNode, AppendText}; use string_cache::QualName; -use hyper::header::{Header, HeaderFormat}; -use hyper::header::common::util as header_util; pub enum HTMLInput { InputString(String), InputUrl(Url), } -//FIXME(seanmonstar): uplift to Hyper -#[deriving(Clone)] -struct LastModified(pub Tm); - -impl Header for LastModified { - #[inline] - fn header_name(_: Option) -> &'static str { - "Last-Modified" - } - - // Parses an RFC 2616 compliant date/time string, - fn parse_header(raw: &[Vec]) -> Option { - header_util::from_one_raw_str(raw).and_then(|s: String| { - let s = s.as_slice(); - strptime(s, "%a, %d %b %Y %T %Z").or_else(|_| { - strptime(s, "%A, %d-%b-%y %T %Z") - }).or_else(|_| { - strptime(s, "%c") - }).ok().map(|tm| LastModified(tm)) - }) - } -} - -impl HeaderFormat for LastModified { - // a localized date/time string in a format suitable - // for document.lastModified. - fn fmt_header(&self, f: &mut fmt::Formatter) -> fmt::Result { - let LastModified(ref tm) = *self; - match tm.tm_gmtoff { - 0 => tm.rfc822().fmt(f), - _ => tm.to_utc().rfc822().fmt(f) - } - } -} - -fn dom_last_modified(tm: &Tm) -> String { - tm.to_local().strftime("%m/%d/%Y %H:%M:%S").unwrap() -} - trait SinkHelpers { fn get_or_create(&self, child: NodeOrText) -> Temporary; } @@ -207,52 +161,10 @@ impl<'a> TreeSink for servohtmlparser::Sink { } } -// The url from msg_load_data is ignored here -pub fn parse_html(page: &Page, - document: JSRef, +pub fn parse_html(document: JSRef, input: HTMLInput, - resource_task: ResourceTask, - msg_load_data: MsgLoadData) { - let (base_url, load_response) = match input { - InputUrl(ref url) => { - // Wait for the LoadResponse so that the parser knows the final URL. - let (input_chan, input_port) = channel(); - resource_task.send(Load(LoadData { - url: url.clone(), - method: msg_load_data.method, - headers: msg_load_data.headers, - data: msg_load_data.data, - cors: None, - consumer: input_chan, - })); - - let load_response = input_port.recv(); - - load_response.metadata.headers.as_ref().map(|headers| { - headers.get().map(|&LastModified(ref tm)| { - document.set_last_modified(dom_last_modified(tm)); - }); - }); - - let base_url = load_response.metadata.final_url.clone(); - - { - // Store the final URL before we start parsing, so that DOM routines - // (e.g. HTMLImageElement::update_image) can resolve relative URLs - // correctly. - *page.mut_url() = Some((base_url.clone(), true)); - } - - (Some(base_url), Some(load_response)) - }, - InputString(_) => { - match *page.url() { - Some((ref page_url, _)) => (Some(page_url.clone()), None), - None => (None, None), - } - }, - }; - + base_url: Option, + load_response: Option) { let parser = ServoHTMLParser::new(base_url.clone(), document).root(); let parser: JSRef = *parser; diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 7da46d628ce..688a89757c2 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -51,7 +51,8 @@ use servo_msg::constellation_msg::{KeyModifiers, SUPER, SHIFT, CONTROL, ALT, Rep use servo_msg::constellation_msg::{Released}; use servo_msg::constellation_msg; use servo_net::image_cache_task::ImageCacheTask; -use servo_net::resource_task::ResourceTask; +use servo_net::resource_task::{ResourceTask, Load}; +use servo_net::resource_task::LoadData as NetLoadData; use servo_net::storage_task::StorageTask; use servo_util::geometry::to_frac_px; use servo_util::smallvec::{SmallVec1, SmallVec}; @@ -59,6 +60,8 @@ use servo_util::task::spawn_named_with_send_on_failure; use servo_util::task_state; use geom::point::Point2D; +use hyper::header::{Header, HeaderFormat}; +use hyper::header::common::util as header_util; use js::jsapi::{JS_SetWrapObjectCallbacks, JS_SetGCZeal, JS_DEFAULT_ZEAL_FREQ, JS_GC}; use js::jsapi::{JSContext, JSRuntime, JSTracer}; use js::jsapi::{JS_SetGCParameter, JSGC_MAX_BYTES}; @@ -71,9 +74,11 @@ use libc::size_t; use std::any::{Any, AnyRefExt}; use std::collections::HashSet; use std::comm::{channel, Sender, Receiver, Select}; +use std::fmt::{mod, Show}; use std::mem::replace; use std::rc::Rc; use std::u32; +use time::{Tm, strptime}; local_data_key!(pub StackRoots: *const RootCollection) @@ -748,7 +753,47 @@ impl ScriptTask { InputString(strval.unwrap_or("".to_string())) }; - parse_html(&*page, *document, parser_input, self.resource_task.clone(), load_data); + let (base_url, load_response) = match parser_input { + InputUrl(ref url) => { + // Wait for the LoadResponse so that the parser knows the final URL. + let (input_chan, input_port) = channel(); + self.resource_task.send(Load(NetLoadData { + url: url.clone(), + method: load_data.method, + headers: load_data.headers, + data: load_data.data, + cors: None, + consumer: input_chan, + })); + + let load_response = input_port.recv(); + + load_response.metadata.headers.as_ref().map(|headers| { + headers.get().map(|&LastModified(ref tm)| { + document.set_last_modified(dom_last_modified(tm)); + }); + }); + + let base_url = load_response.metadata.final_url.clone(); + + { + // Store the final URL before we start parsing, so that DOM routines + // (e.g. HTMLImageElement::update_image) can resolve relative URLs + // correctly. + *page.mut_url() = Some((base_url.clone(), true)); + } + + (Some(base_url), Some(load_response)) + }, + InputString(_) => { + match *page.url() { + Some((ref page_url, _)) => (Some(page_url.clone()), None), + None => (None, None), + } + }, + }; + + parse_html(*document, parser_input, base_url, load_response); url = page.get_url().clone(); document.set_ready_state(DocumentReadyStateValues::Interactive); @@ -1159,3 +1204,42 @@ pub fn get_page(page: &Rc, pipeline_id: PipelineId) -> Rc { message for a layout channel that is not associated with this script task.\ This is a bug.") } + +//FIXME(seanmonstar): uplift to Hyper +#[deriving(Clone)] +struct LastModified(pub Tm); + +impl Header for LastModified { + #[inline] + fn header_name(_: Option) -> &'static str { + "Last-Modified" + } + + // Parses an RFC 2616 compliant date/time string, + fn parse_header(raw: &[Vec]) -> Option { + header_util::from_one_raw_str(raw).and_then(|s: String| { + let s = s.as_slice(); + strptime(s, "%a, %d %b %Y %T %Z").or_else(|_| { + strptime(s, "%A, %d-%b-%y %T %Z") + }).or_else(|_| { + strptime(s, "%c") + }).ok().map(|tm| LastModified(tm)) + }) + } +} + +impl HeaderFormat for LastModified { + // a localized date/time string in a format suitable + // for document.lastModified. + fn fmt_header(&self, f: &mut fmt::Formatter) -> fmt::Result { + let LastModified(ref tm) = *self; + match tm.tm_gmtoff { + 0 => tm.rfc822().fmt(f), + _ => tm.to_utc().rfc822().fmt(f) + } + } +} + +fn dom_last_modified(tm: &Tm) -> String { + tm.to_local().strftime("%m/%d/%Y %H:%M:%S").unwrap() +} From 3e031bdaf8da146bbc0e2bca6db0edca75c20d0c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 6 Dec 2014 15:39:45 +0100 Subject: [PATCH 6/6] Make parse_html's base_url argument non-optional. It turns out the case where it woul be None can't happen. --- components/script/parse/html.rs | 6 +++--- components/script/script_task.rs | 9 +++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 5f34c87d3da..f78949d924c 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -163,9 +163,9 @@ impl<'a> TreeSink for servohtmlparser::Sink { pub fn parse_html(document: JSRef, input: HTMLInput, - base_url: Option, + base_url: Url, load_response: Option) { - let parser = ServoHTMLParser::new(base_url.clone(), document).root(); + let parser = ServoHTMLParser::new(Some(base_url.clone()), document).root(); let parser: JSRef = *parser; task_state::enter(IN_HTML_PARSER); @@ -178,7 +178,7 @@ pub fn parse_html(document: JSRef, let load_response = load_response.unwrap(); match load_response.metadata.content_type { Some((ref t, _)) if t.as_slice().eq_ignore_ascii_case("image") => { - let page = format!("", base_url.as_ref().unwrap().serialize()); + let page = format!("", base_url.serialize()); parser.parse_chunk(page); }, _ => { diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 688a89757c2..b36f7733a98 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -728,7 +728,7 @@ impl ScriptTask { } else { url.clone() }; - let document = Document::new(*window, Some(doc_url), HTMLDocument, + let document = Document::new(*window, Some(doc_url.clone()), HTMLDocument, None, FromParser).root(); window.init_browser_context(*document); @@ -783,13 +783,10 @@ impl ScriptTask { *page.mut_url() = Some((base_url.clone(), true)); } - (Some(base_url), Some(load_response)) + (base_url, Some(load_response)) }, InputString(_) => { - match *page.url() { - Some((ref page_url, _)) => (Some(page_url.clone()), None), - None => (None, None), - } + (doc_url, None) }, };