From 18178fa107204cdbfe5644e56e0dfbd482e0b2a2 Mon Sep 17 00:00:00 2001 From: Thomas Delacour Date: Wed, 17 Jul 2019 09:04:56 -0400 Subject: [PATCH] ISSUE-21257: set redirectEnd on PerformanceResourceTiming --- components/net/http_loader.rs | 44 ++++++++++++++++--- components/net_traits/lib.rs | 13 +++++- .../script/dom/performanceresourcetiming.rs | 9 ++-- .../webidls/PerformanceResourceTiming.webidl | 2 +- .../resource-timing/idlharness.any.js.ini | 12 ----- .../resource_TAO_origin.htm.ini | 3 -- .../resource-timing/resource_TAO_zero.htm.ini | 3 -- 7 files changed, 58 insertions(+), 28 deletions(-) diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 3080221f356..dfae8624041 100644 --- a/components/net/http_loader.rs +++ b/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, ResourceFetchTiming}; +use net_traits::{RedirectEndValue, RedirectStartValue, ResourceAttribute, ResourceFetchTiming}; use openssl::ssl::SslConnectorBuilder; use servo_url::{ImmutableOrigin, ServoUrl}; use std::collections::{HashMap, HashSet}; @@ -621,8 +621,6 @@ pub fn http_fetch( }; } - // TODO redirect_end: last byte of response of last redirect - // set back to default response.return_internal = true; context @@ -639,6 +637,27 @@ pub fn http_fetch( response } +// Convenience struct that implements Drop, for setting redirectEnd on function return +struct RedirectEndTimer(Option>>); + +impl RedirectEndTimer { + fn neuter(&mut self) { + self.0 = None; + } +} + +impl Drop for RedirectEndTimer { + fn drop(&mut self) { + let RedirectEndTimer(resource_fetch_timing_opt) = self; + + resource_fetch_timing_opt.as_ref().map_or((), |t| { + t.lock() + .unwrap() + .set_attribute(ResourceAttribute::RedirectEnd(RedirectEndValue::Zero)); + }) + } +} + /// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch) pub fn http_redirect_fetch( request: &mut Request, @@ -649,6 +668,8 @@ pub fn http_redirect_fetch( done_chan: &mut DoneChannel, context: &FetchContext, ) -> Response { + let mut redirect_end_timer = RedirectEndTimer(Some(context.timing.clone())); + // Step 1 assert!(response.return_internal); @@ -761,7 +782,7 @@ pub fn http_redirect_fetch( // Step 15 let recursive_flag = request.redirect_mode != RedirectMode::Manual; - main_fetch( + let fetch_response = main_fetch( request, cache, cors_flag, @@ -769,7 +790,19 @@ pub fn http_redirect_fetch( target, done_chan, context, - ) + ); + + // TODO: timing allow check + context + .timing + .lock() + .unwrap() + .set_attribute(ResourceAttribute::RedirectEnd( + RedirectEndValue::ResponseEnd, + )); + redirect_end_timer.neuter(); + + fetch_response } fn try_immutable_origin_to_hyper_origin(url_origin: &ImmutableOrigin) -> Option { @@ -1197,6 +1230,7 @@ fn http_network_fetch( 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 diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index 566a3e1093a..b30ed9e6877 100644 --- a/components/net_traits/lib.rs +++ b/components/net_traits/lib.rs @@ -450,7 +450,7 @@ pub struct ResourceFetchTiming { pub fetch_start: u64, pub response_end: u64, pub redirect_start: u64, - // pub redirect_end: u64, + pub redirect_end: u64, pub connect_start: u64, pub connect_end: u64, } @@ -461,12 +461,18 @@ pub enum RedirectStartValue { FetchStart, } +pub enum RedirectEndValue { + Zero, + ResponseEnd, +} + pub enum ResourceAttribute { RedirectCount(u16), DomainLookupStart, RequestStart, ResponseStart, RedirectStart(RedirectStartValue), + RedirectEnd(RedirectEndValue), FetchStart, ConnectStart(u64), ConnectEnd(u64), @@ -491,6 +497,7 @@ impl ResourceFetchTiming { response_start: 0, fetch_start: 0, redirect_start: 0, + redirect_end: 0, connect_start: 0, connect_end: 0, response_end: 0, @@ -513,6 +520,10 @@ impl ResourceFetchTiming { } }, }, + ResourceAttribute::RedirectEnd(val) => match val { + RedirectEndValue::Zero => self.redirect_end = 0, + RedirectEndValue::ResponseEnd => self.redirect_end = self.response_end, + }, ResourceAttribute::FetchStart => self.fetch_start = precise_time_ns(), ResourceAttribute::ConnectStart(val) => self.connect_start = val, ResourceAttribute::ConnectEnd(val) => self.connect_end = val, diff --git a/components/script/dom/performanceresourcetiming.rs b/components/script/dom/performanceresourcetiming.rs index 2457e4f23ba..5d755240458 100644 --- a/components/script/dom/performanceresourcetiming.rs +++ b/components/script/dom/performanceresourcetiming.rs @@ -59,8 +59,6 @@ pub struct PerformanceResourceTiming { // TODO(#21255): duration // TODO(#21269): next_hop // TODO(#21264): worker_start -// TODO(#21256): redirect_start -// TODO(#21257): redirect_end // TODO(#21258): fetch_start // TODO(#21259): domain_lookup_start // TODO(#21260): domain_lookup_end @@ -116,7 +114,7 @@ impl PerformanceResourceTiming { next_hop: next_hop, worker_start: 0., redirect_start: resource_timing.redirect_start as f64, - redirect_end: 0., + redirect_end: resource_timing.redirect_end as f64, fetch_start: resource_timing.fetch_start as f64, domain_lookup_start: resource_timing.domain_lookup_start as f64, domain_lookup_end: 0., @@ -187,6 +185,11 @@ impl PerformanceResourceTimingMethods for PerformanceResourceTiming { Finite::wrap(self.redirect_start) } + // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-redirectend + fn RedirectEnd(&self) -> DOMHighResTimeStamp { + Finite::wrap(self.redirect_end) + } + // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-responsestart fn ResponseStart(&self) -> DOMHighResTimeStamp { Finite::wrap(self.response_start) diff --git a/components/script/dom/webidls/PerformanceResourceTiming.webidl b/components/script/dom/webidls/PerformanceResourceTiming.webidl index 087db5f45cd..acf1682e800 100644 --- a/components/script/dom/webidls/PerformanceResourceTiming.webidl +++ b/components/script/dom/webidls/PerformanceResourceTiming.webidl @@ -13,7 +13,7 @@ interface PerformanceResourceTiming : PerformanceEntry { readonly attribute DOMString nextHopProtocol; // readonly attribute DOMHighResTimeStamp workerStart; readonly attribute DOMHighResTimeStamp redirectStart; - // readonly attribute DOMHighResTimeStamp redirectEnd; + readonly attribute DOMHighResTimeStamp redirectEnd; readonly attribute DOMHighResTimeStamp fetchStart; readonly attribute DOMHighResTimeStamp domainLookupStart; // readonly attribute DOMHighResTimeStamp domainLookupEnd; diff --git a/tests/wpt/metadata/resource-timing/idlharness.any.js.ini b/tests/wpt/metadata/resource-timing/idlharness.any.js.ini index d96d2f8262d..2145ca938b4 100644 --- a/tests/wpt/metadata/resource-timing/idlharness.any.js.ini +++ b/tests/wpt/metadata/resource-timing/idlharness.any.js.ini @@ -44,12 +44,6 @@ [PerformanceResourceTiming interface: attribute workerStart] expected: FAIL - [PerformanceResourceTiming interface: resource must inherit property "redirectEnd" with the proper type] - expected: FAIL - - [PerformanceResourceTiming interface: attribute redirectEnd] - expected: FAIL - [PerformanceResourceTiming interface: resource must inherit property "domainLookupEnd" with the proper type] expected: FAIL @@ -82,9 +76,6 @@ [PerformanceResourceTiming interface: resource must inherit property "toJSON()" with the proper type] expected: FAIL - [PerformanceResourceTiming interface: resource must inherit property "redirectEnd" with the proper type] - expected: FAIL - [PerformanceResourceTiming interface: resource must inherit property "domainLookupEnd" with the proper type] expected: FAIL @@ -106,9 +97,6 @@ [PerformanceResourceTiming interface: attribute workerStart] expected: FAIL - [PerformanceResourceTiming interface: attribute redirectEnd] - expected: FAIL - [PerformanceResourceTiming interface: attribute domainLookupEnd] expected: FAIL diff --git a/tests/wpt/metadata/resource-timing/resource_TAO_origin.htm.ini b/tests/wpt/metadata/resource-timing/resource_TAO_origin.htm.ini index d9f5956bc52..5c59982f790 100644 --- a/tests/wpt/metadata/resource-timing/resource_TAO_origin.htm.ini +++ b/tests/wpt/metadata/resource-timing/resource_TAO_origin.htm.ini @@ -5,9 +5,6 @@ [responseStart should not be 0 in timing-allow cross-origin request.] expected: FAIL - [redirectEnd should be 0 in cross-origin request since no redirect.] - expected: FAIL - [secureConnectionStart should be 0 in cross-origin request since no ssl!] expected: FAIL diff --git a/tests/wpt/metadata/resource-timing/resource_TAO_zero.htm.ini b/tests/wpt/metadata/resource-timing/resource_TAO_zero.htm.ini index 841d4050464..aa36a14c6a6 100644 --- a/tests/wpt/metadata/resource-timing/resource_TAO_zero.htm.ini +++ b/tests/wpt/metadata/resource-timing/resource_TAO_zero.htm.ini @@ -5,9 +5,6 @@ [domainLookupEnd should be 0 in cross-origin request.] expected: FAIL - [redirectEnd should be 0 in cross-origin request.] - expected: FAIL - [connectEnd should be 0 in cross-origin request.] expected: FAIL