Auto merge of #29627 - CYBAI:respect-headers-ct, r=mukilan

Respect MIME type from headers instead of caching it

Based on the spec,

> A [Request](https://fetch.spec.whatwg.org/#request) object’s [MIME type](https://fetch.spec.whatwg.org/#concept-body-mime-type) is to return the result of [extracting a MIME type](https://fetch.spec.whatwg.org/#concept-header-extract-mime-type) from its [request](https://fetch.spec.whatwg.org/#concept-request-request)’s [header list](https://fetch.spec.whatwg.org/#concept-request-header-list).

request and response should always return the MIME type from their header list. As we're currently caching the mime type directly, this PR will help to remove the caching field and always retrieve from headers instead.

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
- [x] There are tests for these changes

4 files changed, 85 insertions, 148 deletions

diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs
index de81bd7dc4c..6d466ea0c61 100644
--- a/components/script/dom/headers.rs
+++ b/components/script/dom/headers.rs
@@ -272,10 +272,6 @@ impl Headers {
         self.guard.get()
     }
 
-    pub fn empty_header_list(&self) {
-        *self.header_list.borrow_mut() = HyperHeaders::new();
-    }
-
     pub fn set_headers(&self, hyper_headers: HyperHeaders) {
         *self.header_list.borrow_mut() = hyper_headers;
     }
diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs
index e301d360cc4..9bb857736a4 100644
--- a/components/script/dom/request.rs
+++ b/components/script/dom/request.rs
@@ -50,7 +50,6 @@ pub struct Request {
     request: DomRefCell<NetTraitsRequest>,
     body_stream: MutNullableDom<ReadableStream>,
     headers: MutNullableDom<Headers>,
-    mime_type: DomRefCell<Vec<u8>>,
 }
 
 impl Request {
@@ -60,7 +59,6 @@ impl Request {
             request: DomRefCell::new(net_request_from_global(global, url)),
             body_stream: MutNullableDom::new(None),
             headers: Default::default(),
-            mime_type: DomRefCell::new("".to_string().into_bytes()),
         }
     }
 
@@ -81,69 +79,71 @@ impl Request {
         // Step 2
         let mut fallback_mode: Option<NetTraitsRequestMode> = None;
 
-        // Step 3
+        // FIXME(cybai): As the spec changed in https://github.com/whatwg/fetch/pull/1153,
+        //               we will need to change the default value of credentials for
+        //               NetTraitsRequest and then remove fallback here.
         let mut fallback_credentials: Option<NetTraitsRequestCredentials> = None;
 
-        // Step 4
+        // Step 3
         let base_url = global.api_base_url();
 
-        // Step 5 TODO: "Let signal be null."
+        // Step 4 TODO: "Let signal be null."
 
         match input {
-            // Step 6
+            // Step 5
             RequestInfo::USVString(USVString(ref usv_string)) => {
-                // Step 6.1
+                // Step 5.1
                 let parsed_url = base_url.join(&usv_string);
-                // Step 6.2
+                // Step 5.2
                 if parsed_url.is_err() {
                     return Err(Error::Type("Url could not be parsed".to_string()));
                 }
-                // Step 6.3
+                // Step 5.3
                 let url = parsed_url.unwrap();
                 if includes_credentials(&url) {
                     return Err(Error::Type("Url includes credentials".to_string()));
                 }
-                // Step 6.4
+                // Step 5.4
                 temporary_request = net_request_from_global(global, url);
-                // Step 6.5
+                // Step 5.5
                 fallback_mode = Some(NetTraitsRequestMode::CorsMode);
-                // Step 6.6
+                // FIXME(cybai): remove this line when we can remove the fallback of credentials
                 fallback_credentials = Some(NetTraitsRequestCredentials::CredentialsSameOrigin);
             },
-            // Step 7
+            // Step 6
             RequestInfo::Request(ref input_request) => {
                 // This looks like Step 38
                 // TODO do this in the right place to not mask other errors
                 if request_is_disturbed(input_request) || request_is_locked(input_request) {
                     return Err(Error::Type("Input is disturbed or locked".to_string()));
                 }
-                // Step 7.1
+                // Step 6.1
                 temporary_request = input_request.request.borrow().clone();
-                // Step 7.2 TODO: "Set signal to input's signal."
+                // Step 6.2 TODO: "Set signal to input's signal."
             },
         }
 
-        // Step 8
+        // Step 7
         // TODO: `entry settings object` is not implemented yet.
         let origin = base_url.origin();
 
-        // Step 9
+        // Step 8
         let mut window = Window::Client;
 
-        // Step 10
+        // Step 9
         // TODO: `environment settings object` is not implemented in Servo yet.
 
-        // Step 11
+        // Step 10
         if !init.window.handle().is_null_or_undefined() {
             return Err(Error::Type("Window is present and is not null".to_string()));
         }
 
-        // Step 12
+        // Step 11
         if !init.window.handle().is_undefined() {
             window = Window::NoWindow;
         }
 
-        // Step 13
+        // Step 12
         let mut request: NetTraitsRequest;
         request = net_request_from_global(global, temporary_request.current_url());
         request.method = temporary_request.method;
@@ -160,7 +160,7 @@ impl Request {
         request.redirect_mode = temporary_request.redirect_mode;
         request.integrity_metadata = temporary_request.integrity_metadata;
 
-        // Step 14
+        // Step 13
         if init.body.is_some() ||
             init.cache.is_some() ||
             init.credentials.is_some() ||
@@ -173,33 +173,33 @@ impl Request {
             init.referrerPolicy.is_some() ||
             !init.window.handle().is_undefined()
         {
-            // Step 14.1
+            // Step 13.1
             if request.mode == NetTraitsRequestMode::Navigate {
                 request.mode = NetTraitsRequestMode::SameOrigin;
             }
-            // Step 14.2 TODO: "Unset request's reload-navigation flag."
-            // Step 14.3 TODO: "Unset request's history-navigation flag."
-            // Step 14.4
+            // Step 13.2 TODO: "Unset request's reload-navigation flag."
+            // Step 13.3 TODO: "Unset request's history-navigation flag."
+            // Step 13.4
             request.referrer = global.get_referrer();
-            // Step 14.5
+            // Step 13.5
             request.referrer_policy = None;
         }
 
-        // Step 15
+        // Step 14
         if let Some(init_referrer) = init.referrer.as_ref() {
-            // Step 15.1
+            // Step 14.1
             let ref referrer = init_referrer.0;
-            // Step 15.2
+            // Step 14.2
             if referrer.is_empty() {
                 request.referrer = NetTraitsRequestReferrer::NoReferrer;
             } else {
-                // Step 15.3.1
+                // Step 14.3.1
                 let parsed_referrer = base_url.join(referrer);
-                // Step 15.3.2
+                // Step 14.3.2
                 if parsed_referrer.is_err() {
                     return Err(Error::Type("Failed to parse referrer url".to_string()));
                 }
-                // Step 15.3.3
+                // Step 14.3.3
                 if let Ok(parsed_referrer) = parsed_referrer {
                     if (parsed_referrer.cannot_be_a_base() &&
                         parsed_referrer.scheme() == "about" &&
@@ -208,55 +208,54 @@ impl Request {
                     {
                         request.referrer = global.get_referrer();
                     } else {
-                        // Step 15.3.4
+                        // Step 14.3.4
                         request.referrer = NetTraitsRequestReferrer::ReferrerUrl(parsed_referrer);
                     }
                 }
             }
         }
 
-        // Step 16
+        // Step 15
         if let Some(init_referrerpolicy) = init.referrerPolicy.as_ref() {
             let init_referrer_policy = init_referrerpolicy.clone().into();
             request.referrer_policy = Some(init_referrer_policy);
         }
 
-        // Step 17
+        // Step 16
         let mode = init
             .mode
             .as_ref()
             .map(|m| m.clone().into())
             .or(fallback_mode);
 
-        // Step 18
+        // Step 17
         if let Some(NetTraitsRequestMode::Navigate) = mode {
             return Err(Error::Type("Request mode is Navigate".to_string()));
         }
 
-        // Step 19
+        // Step 18
         if let Some(m) = mode {
             request.mode = m;
         }
 
-        // Step 20
+        // Step 19
         let credentials = init
             .credentials
             .as_ref()
             .map(|m| m.clone().into())
             .or(fallback_credentials);
 
-        // Step 21
         if let Some(c) = credentials {
             request.credentials_mode = c;
         }
 
-        // Step 22
+        // Step 20
         if let Some(init_cache) = init.cache.as_ref() {
             let cache = init_cache.clone().into();
             request.cache_mode = cache;
         }
 
-        // Step 23
+        // Step 21
         if request.cache_mode == NetTraitsRequestCache::OnlyIfCached {
             if request.mode != NetTraitsRequestMode::SameOrigin {
                 return Err(Error::Type(
@@ -265,44 +264,46 @@ impl Request {
             }
         }
 
-        // Step 24
+        // Step 22
         if let Some(init_redirect) = init.redirect.as_ref() {
             let redirect = init_redirect.clone().into();
             request.redirect_mode = redirect;
         }
 
-        // Step 25
+        // Step 23
         if let Some(init_integrity) = init.integrity.as_ref() {
             let integrity = init_integrity.clone().to_string();
             request.integrity_metadata = integrity;
         }
 
-        // Step 26 TODO: "If init["keepalive"] exists..."
+        // Step 24 TODO: "If init["keepalive"] exists..."
 
-        // Step 27.1
+        // Step 25.1
         if let Some(init_method) = init.method.as_ref() {
-            // Step 27.2
             if !is_method(&init_method) {
                 return Err(Error::Type("Method is not a method".to_string()));
             }
+            // Step 25.2
             if is_forbidden_method(&init_method) {
                 return Err(Error::Type("Method is forbidden".to_string()));
             }
-            // Step 27.3
+            // Step 25.3
             let method = match init_method.as_str() {
                 Some(s) => normalize_method(s)
                     .map_err(|e| Error::Type(format!("Method is not valid: {:?}", e)))?,
                 None => return Err(Error::Type("Method is not a valid UTF8".to_string())),
             };
-            // Step 27.4
+            // Step 25.4
             request.method = method;
         }
 
-        // Step 28 TODO: "If init["signal"] exists..."
+        // Step 26 TODO: "If init["signal"] exists..."
+        // Step 27 TODO: "If init["priority"] exists..."
 
-        // Step 29
+        // Step 28
         let r = Request::from_net_request(global, request);
 
+        // Step 29 TODO: "Set this's signal to new AbortSignal object..."
         // Step 30 TODO: "If signal is not null..."
 
         // Step 31
@@ -310,7 +311,7 @@ impl Request {
         // hasn't had any other way to initialize its headers
         r.headers.or_init(|| Headers::for_request(&r.global()));
 
-        // Step 32 - but spec says this should only be when non-empty init?
+        // Step 33 - but spec says this should only be when non-empty init?
         let headers_copy = init
             .headers
             .as_ref()
@@ -323,30 +324,30 @@ impl Request {
                 },
             });
 
-        // Step 32.3
+        // Step 33.3
         // We cannot empty `r.Headers().header_list` because
-        // we would undo the Step 27 above.  One alternative is to set
+        // we would undo the Step 25 above.  One alternative is to set
         // `headers_copy` as a deep copy of `r.Headers()`. However,
         // `r.Headers()` is a `DomRoot<T>`, 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.
+        // deep copied headers in Step 25.
 
-        // Step 32.4
+        // Step 32
         if r.request.borrow().mode == NetTraitsRequestMode::NoCors {
             let borrowed_request = r.request.borrow();
-            // Step 32.4.1
+            // Step 32.1
             if !is_cors_safelisted_method(&borrowed_request.method) {
                 return Err(Error::Type(
                     "The mode is 'no-cors' but the method is not a cors-safelisted method"
                         .to_string(),
                 ));
             }
-            // Step 32.4.2
+            // Step 32.2
             r.Headers().set_guard(Guard::RequestNoCors);
         }
 
-        // Step 32.5
+        // Step 33.5
         match headers_copy {
             None => {
                 // This is equivalent to the specification's concept of
@@ -360,11 +361,11 @@ impl Request {
             Some(headers_copy) => r.Headers().fill(Some(headers_copy))?,
         }
 
-        // Step 32.5-6 depending on how we got here
+        // Step 33.5 depending on how we got here
         // Copy the headers list onto the headers of net_traits::Request
         r.request.borrow_mut().headers = r.Headers().get_headers_list();
 
-        // Step 33
+        // Step 34
         let mut input_body = if let RequestInfo::Request(ref mut input_request) = input {
             let mut input_request_request = input_request.request.borrow_mut();
             input_request_request.body.take()
@@ -372,7 +373,7 @@ impl Request {
             None
         };
 
-        // Step 34
+        // Step 35
         if let Some(init_body_option) = init.body.as_ref() {
             if init_body_option.is_some() || input_body.is_some() {
                 let req = r.request.borrow();
@@ -393,14 +394,14 @@ impl Request {
             }
         }
 
-        // Step 35-36
+        // Step 36-37
         if let Some(Some(ref init_body)) = init.body {
-            // Step 36.2 TODO "If init["keepalive"] exists and is true..."
+            // Step 37.1 TODO "If init["keepalive"] exists and is true..."
 
-            // Step 36.3
+            // Step 37.2
             let mut extracted_body = init_body.extract(global)?;
 
-            // Step 36.4
+            // Step 37.3
             if let Some(contents) = extracted_body.content_type.take() {
                 let ct_header_name = b"Content-Type";
                 if !r
@@ -414,6 +415,7 @@ impl Request {
                         ByteString::new(ct_header_val.to_vec()),
                     )?;
 
+                    // Step 37.4
                     // In Servo r.Headers's header list isn't a pointer to
                     // the same actual list as r.request's, and so we need to
                     // append to both lists to keep them in sync.
@@ -431,21 +433,16 @@ impl Request {
             input_body = Some(net_body);
         }
 
-        // Step 37 "TODO if body is non-null and body's source is null..."
-        // This looks like where we need to set the use-preflight flag
-        // if the request has a body and nothing else has set the flag.
-
         // Step 38 is done earlier
 
-        // Step 39
-        // TODO: `ReadableStream` object is not implemented in Servo yet.
+        // Step 39 "TODO if body is non-null and body's source is null..."
+        // This looks like where we need to set the use-preflight flag
+        // if the request has a body and nothing else has set the flag.
 
-        // Step 40
-        r.request.borrow_mut().body = input_body;
+        // Step 40 is done earlier
 
         // Step 41
-        let extracted_mime_type = r.Headers().extract_mime_type();
-        *r.mime_type.borrow_mut() = extracted_mime_type;
+        r.request.borrow_mut().body = input_body;
 
         // Step 42
         Ok(r)
@@ -462,7 +459,6 @@ impl Request {
     fn clone_from(r: &Request) -> Fallible<DomRoot<Request>> {
         let req = r.request.borrow();
         let url = req.url();
-        let mime_type = r.mime_type.borrow().clone();
         let headers_guard = r.Headers().get_guard();
         let r_clone = Request::new(&r.global(), url);
         r_clone.request.borrow_mut().pipeline_id = req.pipeline_id;
@@ -471,7 +467,6 @@ impl Request {
             borrowed_r_request.origin = req.origin.clone();
         }
         *r_clone.request.borrow_mut() = req.clone();
-        *r_clone.mime_type.borrow_mut() = mime_type;
         r_clone.Headers().copy_from_headers(r.Headers())?;
         r_clone.Headers().set_guard(headers_guard);
         Ok(r_clone)
@@ -682,7 +677,8 @@ impl BodyMixin for Request {
     }
 
     fn get_mime_type(&self) -> Vec<u8> {
-        self.mime_type.borrow().clone()
+        let headers = self.Headers();
+        headers.extract_mime_type()
     }
 }
 
diff --git a/components/script/dom/response.rs b/components/script/dom/response.rs
index ef183340a97..11f51b75414 100644
--- a/components/script/dom/response.rs
+++ b/components/script/dom/response.rs
@@ -37,7 +37,6 @@ use url::Position;
 pub struct Response {
     reflector_: Reflector,
     headers_reflector: MutNullableDom<Headers>,
-    mime_type: DomRefCell<Vec<u8>>,
     /// `None` can be considered a StatusCode of `0`.
     #[ignore_malloc_size_of = "Defined in hyper"]
     status: DomRefCell<Option<StatusCode>>,
@@ -62,7 +61,6 @@ impl Response {
         Response {
             reflector_: Reflector::new(),
             headers_reflector: Default::default(),
-            mime_type: DomRefCell::new("".to_string().into_bytes()),
             status: DomRefCell::new(Some(StatusCode::OK)),
             raw_status: DomRefCell::new(Some((200, b"".to_vec()))),
             response_type: DomRefCell::new(DOMResponseType::Default),
@@ -79,6 +77,7 @@ impl Response {
         reflect_dom_object(Box::new(Response::new_inherited(global)), global)
     }
 
+    // https://fetch.spec.whatwg.org/#initialize-a-response
     pub fn Constructor(
         global: &GlobalScope,
         body: Option<BodyInit>,
@@ -100,34 +99,29 @@ impl Response {
             ));
         }
 
-        // Step 3
         let r = Response::new(global);
 
-        // Step 4
+        // Step 3
         *r.status.borrow_mut() = Some(StatusCode::from_u16(init.status).unwrap());
 
-        // Step 5
+        // Step 4
         *r.raw_status.borrow_mut() = Some((init.status, init.statusText.clone().into()));
 
-        // Step 6
+        // Step 5
         if let Some(ref headers_member) = init.headers {
-            // Step 6.1
-            r.Headers().empty_header_list();
-
-            // Step 6.2
             r.Headers().fill(Some(headers_member.clone()))?;
         }
 
-        // Step 7
+        // Step 6
         if let Some(ref body) = body {
-            // Step 7.1
+            // Step 6.1
             if is_null_body_status(init.status) {
                 return Err(Error::Type(
                     "Body is non-null but init's status member is a null body status".to_string(),
                 ));
             };
 
-            // Step 7.3
+            // Step 6.2
             let ExtractedBody {
                 stream,
                 total_bytes: _,
@@ -137,7 +131,7 @@ impl Response {
 
             r.body_stream.set(Some(&*stream));
 
-            // Step 7.4
+            // Step 6.3
             if let Some(content_type_contents) = content_type {
                 if !r
                     .Headers()
@@ -152,16 +146,6 @@ impl Response {
             };
         }
 
-        // Step 8
-        *r.mime_type.borrow_mut() = r.Headers().extract_mime_type();
-
-        // Step 9
-        // TODO: `entry settings object` is not implemented in Servo yet.
-
-        // Step 10
-        // TODO: Write this step once Promises are merged in
-
-        // Step 11
         Ok(r)
     }
 
@@ -242,7 +226,8 @@ impl BodyMixin for Response {
     }
 
     fn get_mime_type(&self) -> Vec<u8> {
-        self.mime_type.borrow().clone()
+        let headers = self.Headers();
+        headers.extract_mime_type()
     }
 }
 
@@ -404,7 +389,6 @@ impl Response {
             Some(hyper_headers) => hyper_headers.into_inner(),
             None => HyperHeaders::new(),
         });
-        *self.mime_type.borrow_mut() = self.Headers().extract_mime_type();
     }
 
     pub fn set_raw_status(&self, status: Option<(u16, Vec<u8>)>) {
diff --git a/tests/wpt/metadata/fetch/api/body/mime-type.any.js.ini b/tests/wpt/metadata/fetch/api/body/mime-type.any.js.ini
deleted file mode 100644
index 9df4327e18f..00000000000
--- a/tests/wpt/metadata/fetch/api/body/mime-type.any.js.ini
+++ /dev/null
@@ -1,39 +0,0 @@
-[mime-type.any.html]
-  [Request: overriding explicit Content-Type]
-    expected: FAIL
-
-  [Response: setting missing Content-Type]
-    expected: FAIL
-
-  [Response: overriding explicit Content-Type]
-    expected: FAIL
-
-  [Request: setting missing Content-Type]
-    expected: FAIL
-
-  [Response: removing implicit Content-Type]
-    expected: FAIL
-
-  [Request: removing implicit Content-Type]
-    expected: FAIL
-
-
-[mime-type.any.worker.html]
-  [Request: overriding explicit Content-Type]
-    expected: FAIL
-
-  [Response: setting missing Content-Type]
-    expected: FAIL
-
-  [Response: overriding explicit Content-Type]
-    expected: FAIL
-
-  [Request: setting missing Content-Type]
-    expected: FAIL
-
-  [Response: removing implicit Content-Type]
-    expected: FAIL
-
-  [Request: removing implicit Content-Type]
-    expected: FAIL
-