Make Response::url_list be a bare Vec<ServoUrl>

components/net/fetch/methods.rs

@@ -252,7 +252,7 @@ pub fn main_fetch(request: &mut Request,
     // Step 14.
     // We don't need to check whether response is a network error,
     // given its type would not be `Default` in this case.
-    let response = if response.response_type == ResponseType::Default {
+    let mut response = if response.response_type == ResponseType::Default {
         let response_type = match request.response_tainting {
             ResponseTainting::Basic => ResponseType::Basic,
             ResponseTainting::CorsTainting => ResponseType::Cors,
@@ -264,18 +264,22 @@ pub fn main_fetch(request: &mut Request,
     };
 
     let internal_error = {
+        // Tests for steps 17 and 18, before step 15 for borrowing concerns.
+        let response_is_network_error = response.is_network_error();
+        let should_replace_with_nosniff_error =
+            !response_is_network_error && should_be_blocked_due_to_nosniff(request.type_, &response.headers);
+
         // Step 15.
-        let network_error_response;
+        let mut network_error_response = response.get_network_error().cloned().map(Response::network_error);
-        let internal_response = if let Some(error) = response.get_network_error() {
+        let internal_response = if let Some(error_response) = network_error_response.as_mut() {
-            network_error_response = Response::network_error(error.clone());
+            error_response
-            &network_error_response
         } else {
-            response.actual_response()
+            response.actual_response_mut()
         };
 
         // Step 16.
-        if internal_response.url_list.borrow().is_empty() {
+        if internal_response.url_list.is_empty() {
-            *internal_response.url_list.borrow_mut() = request.url_list.clone();
+            internal_response.url_list = request.url_list.clone();
         }
 
         // Step 17.
@@ -284,7 +288,7 @@ pub fn main_fetch(request: &mut Request,
         // TODO: handle blocking due to MIME type.
         let blocked_error_response;
         let internal_response =
-            if !response.is_network_error() && should_be_blocked_due_to_nosniff(request.type_, &response.headers) {
+            if should_replace_with_nosniff_error {
                 // Defer rebinding result
                 blocked_error_response = Response::network_error(NetworkError::Internal("Blocked by nosniff".into()));
                 &blocked_error_response
@@ -295,7 +299,7 @@ pub fn main_fetch(request: &mut Request,
         // Step 18.
         // We check `internal_response` since we did not mutate `response`
         // in the previous step.
-        let not_network_error = !response.is_network_error() && !internal_response.is_network_error();
+        let not_network_error = !response_is_network_error && !internal_response.is_network_error();
         if not_network_error && (is_null_body_status(&internal_response.status) ||
             match request.method {
                 Method::Head | Method::Connect => true,

components/net/http_loader.rs

@@ -507,8 +507,7 @@ pub fn http_fetch(request: &mut Request,
                 request.mode != RequestMode::NoCors) ||
                (res.response_type == ResponseType::OpaqueRedirect &&
                 request.redirect_mode != RedirectMode::Manual) ||
-               (res.url_list.borrow().len() > 1 &&
+               (res.url_list.len() > 1 && request.redirect_mode != RedirectMode::Follow) ||
-                request.redirect_mode != RedirectMode::Follow) ||
                res.is_network_error() {
                 return Response::network_error(NetworkError::Internal("Request failed".into()));
             }

components/net_traits/response.rs

@@ -10,7 +10,6 @@ use hyper::status::StatusCode;
 use hyper_serde::Serde;
 use servo_url::ServoUrl;
 use std::ascii::AsciiExt;
-use std::cell::RefCell;
 use std::sync::{Arc, Mutex};
 
 /// [Response type](https://fetch.spec.whatwg.org/#concept-response-type)
@@ -81,7 +80,7 @@ pub struct Response {
     pub response_type: ResponseType,
     pub termination_reason: Option<TerminationReason>,
     url: Option<ServoUrl>,
-    pub url_list: RefCell<Vec<ServoUrl>>,
+    pub url_list: Vec<ServoUrl>,
     /// `None` can be considered a StatusCode of `0`.
     #[ignore_heap_size_of = "Defined in hyper"]
     pub status: Option<StatusCode>,
@@ -106,7 +105,7 @@ impl Response {
             response_type: ResponseType::Default,
             termination_reason: None,
             url: Some(url),
-            url_list: RefCell::new(Vec::new()),
+            url_list: vec![],
             status: Some(StatusCode::Ok),
             raw_status: Some((200, b"OK".to_vec())),
             headers: Headers::new(),
@@ -124,7 +123,7 @@ impl Response {
             response_type: ResponseType::Error(e),
             termination_reason: None,
             url: None,
-            url_list: RefCell::new(vec![]),
+            url_list: vec![],
             status: None,
             raw_status: None,
             headers: Headers::new(),
@@ -163,6 +162,14 @@ impl Response {
         }
     }
 
+    pub fn actual_response_mut(&mut self) -> &mut Response {
+        if self.return_internal && self.internal_response.is_some() {
+            &mut **self.internal_response.as_mut().unwrap()
+        } else {
+            self
+        }
+    }
+
     pub fn to_actual(self) -> Response {
         if self.return_internal && self.internal_response.is_some() {
             *self.internal_response.unwrap()
@@ -226,7 +233,7 @@ impl Response {
             },
 
             ResponseType::Opaque => {
-                response.url_list = RefCell::new(vec![]);
+                response.url_list = vec![];
                 response.url = None;
                 response.headers = Headers::new();
                 response.status = None;

tests/unit/net/fetch.rs

@@ -418,7 +418,7 @@ fn test_fetch_response_is_opaque_filtered() {
     assert_eq!(fetch_response.response_type, ResponseType::Opaque);
 
     assert!(fetch_response.url().is_none());
-    assert!(fetch_response.url_list.into_inner().len() == 0);
+    assert!(fetch_response.url_list.is_empty());
     // this also asserts that status message is "the empty byte sequence"
     assert!(fetch_response.status.is_none());
     assert_eq!(fetch_response.headers, Headers::new());

tests/unit/net/http_loader.rs

@@ -896,8 +896,7 @@ fn test_load_succeeds_with_a_redirect_loop() {
     let _ = server_b.close();
 
     let response = response.to_actual();
-    assert_eq!(*response.url_list.borrow(),
+    assert_eq!(response.url_list, [url_a.clone(), url_b, url_a]);
-               [url_a.clone(), url_b, url_a]);
     assert_eq!(*response.body.lock().unwrap(),
                ResponseBody::Done(b"Success".to_vec()));
 }