From fbedf030d4a654fda6e7e14c8169fdc25cb3cd66 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 8 Dec 2014 19:21:41 +0100 Subject: [PATCH 1/3] Combine two conditionals in ScriptTask::load. Now that the code lives in the same function, I see no reason for them to remain separate. --- components/script/script_task.rs | 71 ++++++++++++++------------------ 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index b36f7733a98..da043150b53 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -744,50 +744,41 @@ impl ScriptTask { }); } - let parser_input = if !is_javascript { - InputUrl(url.clone()) + let (parser_input, base_url, load_response) = if !is_javascript { + // 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)); + } + + (InputUrl(url.clone()), base_url, Some(load_response)) } else { let evalstr = load_data.url.non_relative_scheme_data().unwrap(); let jsval = window.evaluate_js_with_result(evalstr); let strval = FromJSValConvertible::from_jsval(self.get_cx(), jsval, Empty); - InputString(strval.unwrap_or("".to_string())) - }; - - 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)); - } - - (base_url, Some(load_response)) - }, - InputString(_) => { - (doc_url, None) - }, + (InputString(strval.unwrap_or("".to_string())), doc_url, None) }; parse_html(*document, parser_input, base_url, load_response); From e76c3386ce226db45cd4d4d74828fa380034ec49 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 8 Dec 2014 23:51:46 +0100 Subject: [PATCH 2/3] Move the load response into the HTMLInput enum. It is None iff the parser input is an InputString variant, so it makes more sense to pass it in the same enum. --- components/script/parse/html.rs | 8 +++----- components/script/script_task.rs | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index f78949d924c..040629bf672 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -33,7 +33,7 @@ use string_cache::QualName; pub enum HTMLInput { InputString(String), - InputUrl(Url), + InputUrl(Url, LoadResponse), } trait SinkHelpers { @@ -163,8 +163,7 @@ impl<'a> TreeSink for servohtmlparser::Sink { pub fn parse_html(document: JSRef, input: HTMLInput, - base_url: Url, - load_response: Option) { + base_url: Url) { let parser = ServoHTMLParser::new(Some(base_url.clone()), document).root(); let parser: JSRef = *parser; @@ -174,8 +173,7 @@ pub fn parse_html(document: JSRef, InputString(s) => { parser.parse_chunk(s); } - InputUrl(url) => { - let load_response = load_response.unwrap(); + InputUrl(url, load_response) => { match load_response.metadata.content_type { Some((ref t, _)) if t.as_slice().eq_ignore_ascii_case("image") => { let page = format!("", base_url.serialize()); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index da043150b53..02c30e52a42 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -744,7 +744,7 @@ impl ScriptTask { }); } - let (parser_input, base_url, load_response) = if !is_javascript { + let (parser_input, base_url) = if !is_javascript { // Wait for the LoadResponse so that the parser knows the final URL. let (input_chan, input_port) = channel(); self.resource_task.send(Load(NetLoadData { @@ -773,15 +773,15 @@ impl ScriptTask { *page.mut_url() = Some((base_url.clone(), true)); } - (InputUrl(url.clone()), base_url, Some(load_response)) + (InputUrl(url.clone(), load_response), base_url) } else { let evalstr = load_data.url.non_relative_scheme_data().unwrap(); let jsval = window.evaluate_js_with_result(evalstr); let strval = FromJSValConvertible::from_jsval(self.get_cx(), jsval, Empty); - (InputString(strval.unwrap_or("".to_string())), doc_url, None) + (InputString(strval.unwrap_or("".to_string())), doc_url) }; - parse_html(*document, parser_input, base_url, load_response); + parse_html(*document, parser_input, base_url); url = page.get_url().clone(); document.set_ready_state(DocumentReadyStateValues::Interactive); From 0fc65d4088ada55e17045a08aac0f6d31489056a Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 9 Dec 2014 01:07:13 +0100 Subject: [PATCH 3/3] Remove the url from the InputUrl variant. This url is the pre-redirect url, which is not particularly meaningful, and it is used only in a panic message. --- components/script/parse/html.rs | 10 +++++----- components/script/script_task.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 040629bf672..c629305e931 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -33,7 +33,7 @@ use string_cache::QualName; pub enum HTMLInput { InputString(String), - InputUrl(Url, LoadResponse), + InputUrl(LoadResponse), } trait SinkHelpers { @@ -163,8 +163,8 @@ impl<'a> TreeSink for servohtmlparser::Sink { pub fn parse_html(document: JSRef, input: HTMLInput, - base_url: Url) { - let parser = ServoHTMLParser::new(Some(base_url.clone()), document).root(); + url: Url) { + let parser = ServoHTMLParser::new(Some(url.clone()), document).root(); let parser: JSRef = *parser; task_state::enter(IN_HTML_PARSER); @@ -173,10 +173,10 @@ pub fn parse_html(document: JSRef, InputString(s) => { parser.parse_chunk(s); } - InputUrl(url, load_response) => { + InputUrl(load_response) => { match load_response.metadata.content_type { Some((ref t, _)) if t.as_slice().eq_ignore_ascii_case("image") => { - let page = format!("", base_url.serialize()); + let page = format!("", url.serialize()); parser.parse_chunk(page); }, _ => { diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 02c30e52a42..01473c34681 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -748,7 +748,7 @@ impl ScriptTask { // 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(), + url: url, method: load_data.method, headers: load_data.headers, data: load_data.data, @@ -773,7 +773,7 @@ impl ScriptTask { *page.mut_url() = Some((base_url.clone(), true)); } - (InputUrl(url.clone(), load_response), base_url) + (InputUrl(load_response), base_url) } else { let evalstr = load_data.url.non_relative_scheme_data().unwrap(); let jsval = window.evaluate_js_with_result(evalstr);