From 94ea24e47bd9bb41a95cf85820698484783577cb Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Fri, 14 Oct 2016 16:32:21 -0700 Subject: Allow Request's Headers to be created with various objects While Headers could be constructed correctly with an array or object (open ended dictionary/MozMap), Request's Headers failed to be created with non-Headers object (such as array or open ended dictionary/MozMap). Before, Request's Headers could be filled with only a Headers object in Step 28. This has been expanded to accommodate array and open ended dictionary. Step 29 empties the Request's Headers list after it had been filled in Step 28, thus resulting in an empty Headers object when it shouldn't be. This step has been removed with a comment in this commit. If a RequestInit Headers is *not* given, but a RequestInfo Headers is given, RequestInfo Headers should be used to construct Request Headers. That step has been added after Step 31. Corresponding wpt result is updated in this commit. --- components/script/dom/request.rs | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) (limited to 'components/script/dom/request.rs') diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index 4cad8753eeb..1ec304323cd 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -308,21 +308,26 @@ impl Request { // Step 27 let mut headers_copy = r.Headers(); - // This is equivalent to the specification's concept of - // "associated headers list". - if let RequestInfo::Request(ref input_request) = input { - headers_copy = input_request.Headers(); - } - // Step 28 if let Some(possible_header) = init.headers.as_ref() { - if let &HeadersInit::Headers(ref init_headers) = possible_header { - headers_copy = init_headers.clone(); + match possible_header { + &HeadersInit::Headers(ref init_headers) => { + headers_copy = init_headers.clone(); + } + &HeadersInit::ByteStringSequenceSequence(ref init_sequence) => { + try!(headers_copy.fill(Some( + HeadersInit::ByteStringSequenceSequence(init_sequence.clone())))); + }, + &HeadersInit::ByteStringMozMap(ref init_map) => { + try!(headers_copy.fill(Some( + HeadersInit::ByteStringMozMap(init_map.clone())))); + }, } } // Step 29 - r.Headers().empty_header_list(); + // When r.Headers()'s header list is emptied, + // r.Headers that was filled in Step 28 becomes empty. // Step 30 if r.request.borrow().mode == NetTraitsRequestMode::NoCORS { @@ -341,7 +346,19 @@ impl Request { } // Step 31 - try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))); + if let Some(HeadersInit::Headers(_)) = init.headers { + try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))); + }; + + // This is equivalent to the specification's concept of + // "associated headers list". If an init headers is not given, + // but an input with headers is given, set request's + // headers as the input's Headers. + if let None = init.headers { + if let RequestInfo::Request(ref input_request) = input { + try!(r.Headers().fill(Some(HeadersInit::Headers(input_request.Headers())))); + } + }; // Step 32 let mut input_body = if let RequestInfo::Request(ref input_request) = input { @@ -368,7 +385,6 @@ impl Request { } // Step 34 - // TODO: `ReadableStream` object is not implemented in Servo yet. if let Some(Some(ref init_body)) = init.body { // Step 34.2 let extracted_body_tmp = init_body.extract(); -- cgit v1.2.3 From baacc692f0b04a4b591293a1123600089c5d6105 Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Thu, 27 Oct 2016 15:42:09 -0700 Subject: Address review comments --- components/script/dom/request.rs | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) (limited to 'components/script/dom/request.rs') diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index 1ec304323cd..b9d54cecdbb 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -40,6 +40,7 @@ use std::cell::{Cell, Ref}; use std::rc::Rc; use url::Url; +use dom::bindings::js::RootedReference; #[dom_struct] pub struct Request { reflector_: Reflector, @@ -326,8 +327,13 @@ impl Request { } // Step 29 - // When r.Headers()'s header list is emptied, - // r.Headers that was filled in Step 28 becomes empty. + // We cannot empty `r.Headers().header_list` because + // we would undo the Step 27 above. One alternative is to set + // `headers_copy` as a deep copy of `r.Headers()`. However, + // `r.Headers()` is a `Root`, and therefore it is difficult + // to obtain a mutable reference to `r.Headers()`. Without the + // mutable reference, we cannot mutate `r.Headers()` to be the + // deep copied headers in Step 27. // Step 30 if r.request.borrow().mode == NetTraitsRequestMode::NoCORS { @@ -346,19 +352,18 @@ impl Request { } // Step 31 - if let Some(HeadersInit::Headers(_)) = init.headers { - try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))); - }; - - // This is equivalent to the specification's concept of - // "associated headers list". If an init headers is not given, - // but an input with headers is given, set request's - // headers as the input's Headers. - if let None = init.headers { - if let RequestInfo::Request(ref input_request) = input { - try!(r.Headers().fill(Some(HeadersInit::Headers(input_request.Headers())))); - } - }; + match init.headers { + None => { + // This is equivalent to the specification's concept of + // "associated headers list". If an init headers is not given, + // but an input with headers is given, set request's + // headers as the input's Headers. + if let RequestInfo::Request(ref input_request) = input { + try!(r.Headers().fill(Some(HeadersInit::Headers(input_request.Headers())))); + } + }, + Some(_) => try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))), + } // Step 32 let mut input_body = if let RequestInfo::Request(ref input_request) = input { -- cgit v1.2.3 From e821da81ce89c205a7708e1448c5ca4d1e181b17 Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Thu, 27 Oct 2016 15:54:08 -0700 Subject: Remove unused use statement --- components/script/dom/request.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'components/script/dom/request.rs') diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index b9d54cecdbb..ad328b2c269 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -40,7 +40,6 @@ use std::cell::{Cell, Ref}; use std::rc::Rc; use url::Url; -use dom::bindings::js::RootedReference; #[dom_struct] pub struct Request { reflector_: Reflector, -- cgit v1.2.3 From bfd999f71e198cffb7b1e84d1848c871937e3bd8 Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Mon, 7 Nov 2016 21:52:58 -0800 Subject: Fill r's headers with headers_copy when HeadersInit::Headers is given Instead of filling request's headers whenever a `HeadersInit` is given, this patch fills request's headers only when `HeadersInit` with a type of `Headers` is given. Previously, the constructor tried to fill request's headers with itself, causing Servo to crash. --- components/script/dom/request.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'components/script/dom/request.rs') diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index ad328b2c269..8b64bca337b 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -361,7 +361,8 @@ impl Request { try!(r.Headers().fill(Some(HeadersInit::Headers(input_request.Headers())))); } }, - Some(_) => try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))), + Some(HeadersInit::Headers(_)) => try!(r.Headers().fill(Some(HeadersInit::Headers(headers_copy)))), + _ => {}, } // Step 32 -- cgit v1.2.3