Perf timing dom props #7242

g-k · 2015-08-16T21:39:02Z

I think this is closer to what #7045 describes, but it panics trying to load a page (trace: https://gist.github.com/g-k/a9911467889cdb6fdbf9) and all reftests fail.

Ms2ger · 2015-08-17T14:54:45Z

The issue is the set_ready_state call in Document::new(), which happens before we've had a chance to hook up the window.

g-k · 2015-08-19T18:23:47Z

Ah! That makes sense. I removed that call and it fixed the panics, but I'm not sure where to set the loading time. Should I add a call to set_ready_state after all Document::new calls or is there a cleaner way to do that?

Ms2ger · 2015-08-20T10:24:28Z

I've looked at this for a bit, but the specs are a mess; I'm not quite sure what's the best approach here.

bors-servo · 2015-08-20T14:17:59Z

☔ The latest upstream changes (presumably #7265) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2015-08-27T16:25:17Z

☔ The latest upstream changes (presumably #7401) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2015-09-08T12:07:40Z

☔ The latest upstream changes (presumably #7561) made this pull request unmergeable. Please resolve the merge conflicts.

g-k · 2015-09-28T17:05:15Z

r? @Ms2ger

Manishearth · 2015-10-13T18:11:26Z

@Ms2ger updates?

bors-servo · 2015-10-26T23:38:52Z

☔ The latest upstream changes (presumably #8094) made this pull request unmergeable. Please resolve the merge conflicts.

g-k · 2015-11-01T15:50:41Z

I think this is ready for another look.

I rebased and changed the performance timing constructor to take Root<Document> instead of JS<Document> to satisfy the unrooted_must_root lint check.

jdm · 2015-11-12T13:01:41Z

Looks great! One last piece of cleanup and this will be ready to go. Please squash all the commits into one, too!
-S-awaiting-review +S-needs-squash

Reviewed 1 of 3 files at r1, 1 of 1 files at r7, 1 of 2 files at r8, 1 of 1 files at r9, 3 of 3 files at r10, 1 of 1 files at r11.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

^{components/script/dom/document.rs, line 518 [r11] (raw file):}
Let's add a update_with_current_time(marker: &Cell<u64>) function that does this check and updates the value, then call that from each place in these methods that duplicates this logic.

Comments from the review on Reviewable.io

jdm · 2015-11-13T19:49:39Z

Only one question left about the spec link changes!
-S-awaiting-review -S-needs-squash +S-needs-code-changes

Reviewed 3 of 3 files at r12.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

^{components/script/dom/performancetiming.rs, line 50 [r12] (raw file):}
Why this change? Specs under /TR/ are strictly less interesting to us as implementors.

Comments from the review on Reviewable.io

g-k · 2015-11-13T22:18:31Z

Ah, the http://w3c.github.io/ links were 404ing last night. I'll change
them back.

On Fri, Nov 13, 2015 at 2:49 PM, Josh Matthews notifications@github.com
wrote:

Only one question left about the spec link changes!

-S-awaiting-review -S-needs-squash +S-needs-code-changes

Reviewed 3 of 3 files at r12.
Review status: all files reviewed at latest revision, 1 unresolved

discussion.

components/script/dom/performancetiming.rs, line 50 [r12]
https://reviewable.io:443/reviews/servo/servo/7242#-K31O9PAY41SCBFXOpsj
(raw file

servo/components/script/dom/performancetiming.rs

Line 50 in 3891a42

// https://www.w3.org/TR/navigation-timing/#dom-performancetiming-navigationstart

):
Why this change? Specs under /TR/ are strictly less interesting to us as

implementors.

Comments from the review on Reviewable.io
https://reviewable.io:443/reviews/servo/servo/7242

—
Reply to this email directly or view it on GitHub
#7242 (comment).


        Add DOM-related performance timing properties

jdm · 2015-11-14T05:56:22Z

@bors-servo: r+
Thanks!

Reviewed 3 of 3 files at r13.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

bors-servo · 2015-11-14T05:56:22Z

📌 Commit e49d592 has been approved by jdm

bors-servo · 2015-11-14T05:56:24Z

⌛ Testing commit e49d592 with merge 7de8b0a...


        Auto merge of #7242 - g-k:perf-timing-dom-props, r=jdm

Perf timing dom props I think this is closer to what #7045 describes, but it panics trying to load a page (trace: https://gist.github.com/g-k/a9911467889cdb6fdbf9) and all reftests fail.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7242)

bors-servo · 2015-11-14T06:28:02Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review label Aug 16, 2015

Ms2ger self-assigned this Aug 17, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 16979d7 to e68cd45 Aug 19, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Aug 20, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch 2 times, most recently from b726f86 to 53fb7f0 Aug 24, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Aug 27, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 53fb7f0 to f64fd44 Aug 29, 2015

jdm removed the S-needs-rebase label Aug 30, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 5b06cc9 to a88ef97 Sep 6, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Sep 8, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from a88ef97 to b1c0a01 Sep 26, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 4057f9c to c195e28 Oct 9, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from c195e28 to 6c3d287 Oct 23, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Oct 26, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 6c3d287 to c6cff46 Oct 31, 2015

jdm assigned jdm and unassigned Ms2ger Nov 11, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 144a754 to ee6ca42 Nov 12, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 12, 2015

jdm added S-needs-squash and removed S-awaiting-review labels Nov 12, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from ee6ca42 to 3891a42 Nov 13, 2015

highfive added the S-awaiting-review label Nov 13, 2015

jdm added S-needs-code-changes and removed S-awaiting-review S-needs-squash labels Nov 13, 2015

g-k force-pushed the g-k:perf-timing-dom-props branch from 3891a42 to 66e9be0 Nov 13, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 13, 2015

Add DOM-related performance timing properties

Loading status checks…

e49d592

g-k force-pushed the g-k:perf-timing-dom-props branch from 66e9be0 to e49d592 Nov 13, 2015

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 14, 2015

g-k deleted the g-k:perf-timing-dom-props branch Nov 15, 2015

KiChjang mentioned this pull request Nov 29, 2015

Implement DOM-related properties for PerformanceTiming #7045

Closed

servo / servo

Perf timing dom props #7242

Perf timing dom props #7242

g-k commented Aug 16, 2015

Ms2ger commented Aug 17, 2015

g-k commented Aug 19, 2015

Ms2ger commented Aug 20, 2015

bors-servo commented Aug 20, 2015

bors-servo commented Aug 27, 2015

bors-servo commented Sep 8, 2015

g-k commented Sep 28, 2015

Manishearth commented Oct 13, 2015

bors-servo commented Oct 26, 2015

g-k commented Nov 1, 2015

jdm commented Nov 12, 2015

jdm commented Nov 13, 2015

g-k commented Nov 13, 2015

-S-awaiting-review -S-needs-squash +S-needs-code-changes

discussion.

implementors.

jdm commented Nov 14, 2015

bors-servo commented Nov 14, 2015

bors-servo commented Nov 14, 2015

bors-servo commented Nov 14, 2015

servo / servo

Join GitHub today

Perf timing dom props #7242

Perf timing dom props #7242

Conversation

g-k commented Aug 16, 2015

Ms2ger commented Aug 17, 2015

g-k commented Aug 19, 2015

Ms2ger commented Aug 20, 2015

bors-servo commented Aug 20, 2015

bors-servo commented Aug 27, 2015

bors-servo commented Sep 8, 2015

g-k commented Sep 28, 2015

Manishearth commented Oct 13, 2015

bors-servo commented Oct 26, 2015

g-k commented Nov 1, 2015

jdm commented Nov 12, 2015

jdm commented Nov 13, 2015

g-k commented Nov 13, 2015

-S-awaiting-review -S-needs-squash +S-needs-code-changes

discussion.

implementors.

jdm commented Nov 14, 2015

bors-servo commented Nov 14, 2015

bors-servo commented Nov 14, 2015

bors-servo commented Nov 14, 2015