ISSUE-21263: implemented attribute setting in http loader

components/net/http_loader.rs

@@ -42,7 +42,7 @@ use net_traits::request::{RedirectMode, Referrer, Request, RequestMode};
 use net_traits::request::{ResponseTainting, ServiceWorkersMode};
 use net_traits::response::{HttpsState, Response, ResponseBody, ResponseType};
 use net_traits::{CookieSource, FetchMetadata, NetworkError, ReferrerPolicy};
-use net_traits::{RedirectStartValue, ResourceAttribute};
+use net_traits::{RedirectStartValue, ResourceAttribute, ResourceFetchTiming};
 use openssl::ssl::SslConnectorBuilder;
 use servo_url::{ImmutableOrigin, ServoUrl};
 use std::collections::{HashMap, HashSet};
@@ -51,8 +51,7 @@ use std::iter::FromIterator;
 use std::mem;
 use std::ops::Deref;
 use std::str::FromStr;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex, RwLock};
-use std::sync::RwLock;
 use std::time::{Duration, SystemTime};
 use time::{self, Tm};
 use tokio::prelude::{future, Future, Stream};
@@ -452,7 +451,6 @@ fn obtain_response(
             })
             .map_err(move |e| NetworkError::from_hyper_error(&e)),
     )
-    // TODO(#21263) response_end (also needs to be set above if fetch is aborted due to an error)
 }
 
 /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch)
@@ -1146,6 +1144,27 @@ fn http_network_or_cache_fetch(
     response
 }
 
+// Convenience struct that implements Done, for setting responseEnd on function return
+struct ResponseEndTimer(Option<Arc<Mutex<ResourceFetchTiming>>>);
+
+impl ResponseEndTimer {
+    fn neuter(&mut self) {
+        self.0 = None;
+    }
+}
+
+impl Drop for ResponseEndTimer {
+    fn drop(&mut self) {
+        let ResponseEndTimer(resource_fetch_timing_opt) = self;
+
+        resource_fetch_timing_opt.as_ref().map_or((), |t| {
+            t.lock()
+                .unwrap()
+                .set_attribute(ResourceAttribute::ResponseEnd);
+        })
+    }
+}
+
 /// [HTTP network fetch](https://fetch.spec.whatwg.org/#http-network-fetch)
 fn http_network_fetch(
     request: &Request,
@@ -1153,6 +1172,7 @@ fn http_network_fetch(
     done_chan: &mut DoneChannel,
     context: &FetchContext,
 ) -> Response {
+    let mut response_end_timer = ResponseEndTimer(Some(context.timing.clone()));
     // Step 1
     // nothing to do here, since credentials_flag is already a boolean
 
@@ -1270,6 +1290,8 @@ fn http_network_fetch(
 
     let done_sender2 = done_sender.clone();
     let done_sender3 = done_sender.clone();
+    let timing_ptr2 = context.timing.clone();
+    let timing_ptr3 = context.timing.clone();
     HANDLE.lock().unwrap().spawn(
         res.into_body()
             .map_err(|_| ())
@@ -1293,6 +1315,10 @@ fn http_network_fetch(
                     _ => vec![],
                 };
                 *body = ResponseBody::Done(completed_body);
+                timing_ptr2
+                    .lock()
+                    .unwrap()
+                    .set_attribute(ResourceAttribute::ResponseEnd);
                 let _ = done_sender2.send(Data::Done);
                 future::ok(())
             })
@@ -1303,6 +1329,10 @@ fn http_network_fetch(
                     _ => vec![],
                 };
                 *body = ResponseBody::Done(completed_body);
+                timing_ptr3
+                    .lock()
+                    .unwrap()
+                    .set_attribute(ResourceAttribute::ResponseEnd);
                 let _ = done_sender3.send(Data::Done);
             }),
     );
@@ -1358,6 +1388,10 @@ fn http_network_fetch(
     // Substep 3
 
     // Step 16
+
+    // Ensure we don't override "responseEnd" on successful return of this function
+    response_end_timer.neuter();
+
     response
 }
 

components/net_traits/lib.rs

@@ -449,7 +449,7 @@ pub struct ResourceFetchTiming {
     pub request_start: u64,
     pub response_start: u64,
     pub fetch_start: u64,
-    // pub response_end: u64,
+    pub response_end: u64,
     pub redirect_start: u64,
     // pub redirect_end: u64,
     // pub connect_start: u64,
@@ -469,6 +469,7 @@ pub enum ResourceAttribute {
     RedirectStart(RedirectStartValue),
     FetchStart,
     ConnectEnd(u64),
+    ResponseEnd,
 }
 
 #[derive(Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
@@ -489,6 +490,7 @@ impl ResourceFetchTiming {
             fetch_start: 0,
             redirect_start: 0,
             connect_end: 0,
+            response_end: 0,
         }
     }
 
@@ -509,6 +511,7 @@ impl ResourceFetchTiming {
             },
             ResourceAttribute::FetchStart => self.fetch_start = precise_time_ns(),
             ResourceAttribute::ConnectEnd(val) => self.connect_end = val,
+            ResourceAttribute::ResponseEnd => self.response_end = precise_time_ns(),
         }
     }
 }

components/script/dom/performanceresourcetiming.rs

@@ -66,7 +66,6 @@ pub struct PerformanceResourceTiming {
 // TODO(#21260): domain_lookup_end
 // TODO(#21261): connect_start
 // TODO(#21262): connect_end
-// TODO(#21263): response_end
 impl PerformanceResourceTiming {
     pub fn new_inherited(
         url: ServoUrl,
@@ -126,7 +125,7 @@ impl PerformanceResourceTiming {
             secure_connection_start: 0.,
             request_start: resource_timing.request_start as f64,
             response_start: resource_timing.response_start as f64,
-            response_end: 0.,
+            response_end: resource_timing.response_end as f64,
         }
     }
 
@@ -175,7 +174,6 @@ impl PerformanceResourceTimingMethods for PerformanceResourceTiming {
 
     // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-requeststart
     fn RequestStart(&self) -> DOMHighResTimeStamp {
-        // TODO
         Finite::wrap(self.request_start)
     }
 
@@ -186,7 +184,6 @@ impl PerformanceResourceTimingMethods for PerformanceResourceTiming {
 
     // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-responsestart
     fn ResponseStart(&self) -> DOMHighResTimeStamp {
-        // TODO
         Finite::wrap(self.response_start)
     }
 
@@ -199,4 +196,9 @@ impl PerformanceResourceTimingMethods for PerformanceResourceTiming {
     fn ConnectEnd(&self) -> DOMHighResTimeStamp {
         Finite::wrap(self.connect_end)
     }
+
+    // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-responseend
+    fn ResponseEnd(&self) -> DOMHighResTimeStamp {
+        Finite::wrap(self.response_end)
+    }
 }

components/script/dom/webidls/PerformanceResourceTiming.webidl

@@ -22,7 +22,7 @@ interface PerformanceResourceTiming : PerformanceEntry {
     // readonly attribute DOMHighResTimeStamp secureConnectionStart;
     readonly attribute DOMHighResTimeStamp requestStart;
     readonly attribute DOMHighResTimeStamp responseStart;
-    // readonly attribute DOMHighResTimeStamp responseEnd;
+    readonly attribute DOMHighResTimeStamp responseEnd;
     /// readonly attribute unsigned long long  transferSize;
     /// readonly attribute unsigned long long  encodedBodySize;
     /// readonly attribute unsigned long long  decodedBodySize;

tests/wpt/metadata/resource-timing/idlharness.any.js.ini

@@ -35,9 +35,6 @@
   [PerformanceResourceTiming interface: resource must inherit property "workerStart" with the proper type]
     expected: FAIL
 
-  [PerformanceResourceTiming interface: attribute responseEnd]
-    expected: FAIL
-
   [PerformanceResourceTiming interface: attribute secureConnectionStart]
     expected: FAIL
 
@@ -59,9 +56,6 @@
   [PerformanceResourceTiming interface: attribute connectStart]
     expected: FAIL
 
-  [PerformanceResourceTiming interface: resource must inherit property "responseEnd" with the proper type]
-    expected: FAIL
-
   [PerformanceResourceTiming interface: resource must inherit property "redirectEnd" with the proper type]
     expected: FAIL
 
@@ -154,9 +148,6 @@
   [PerformanceResourceTiming must be primary interface of resource]
     expected: FAIL
 
-  [PerformanceResourceTiming interface: attribute responseEnd]
-    expected: FAIL
-
   [PerformanceResourceTiming interface: attribute secureConnectionStart]
     expected: FAIL
 

tests/wpt/metadata/resource-timing/resource_reuse.sub.html.ini

@@ -8,9 +8,6 @@
   [Entry name should end with file name]
     expected: FAIL
 
-  [responseEnd should not be before startTime]
-    expected: FAIL
-
   [requestStart should be non-zero on the same-origin request]
     expected: FAIL