Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-04 05:00:08 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #23178 - tdelacour:ISSUE-21263, r=jdm

Browse source 
Add PerformanceResourceTiming: ResponseEnd

<!-- Please describe your changes on the following line: -->
1. Added `ResponseEnd` to `ResourceAttribute` enum in `net_traits` and added it to the `set_attribute` function on `ResourceFetchTiming`
2. Added `response_end` field to `performanceresourcetiming.rs`
3. In `http_loader.rs`, set ResponseEnd after response body read is complete, or before return due to network error.

I could use a little guidance on testing. After building and running `wpt` tests, I noticed that some tests now "Pass" when they were expected to "Fail". As per the wiki instructions, I've removed those expectations from the `metadata`.

I noticed that there are a handful of other "failing" test expectations associated with `responseEnd`, but those still do not pass. I looked through some similar PRs (`connectEnd`, `redirectStart`, etc) and saw that they also still have a few failing test expectations here and there. Does that mean this is OK for now? How can I better understand which tests we expect to resolve for a given issue? Thanks!

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21263 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23178)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-04-23 16:49:12 -04:00 • committed by GitHub
commit 03b005c7b2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 49 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

42
components/net/http_loader.rs
View file

@ -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::RwLock;
use std::sync::{Arc, Mutex, 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
}

5
components/net_traits/lib.rs
View file

@ -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(),
}
}
}

10
components/script/dom/performanceresourcetiming.rs
View file

@ -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)
}
}

2
components/script/dom/webidls/PerformanceResourceTiming.webidl
View file

@ -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;

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

@ -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

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

@ -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