Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User Timing API #18283

Merged
merged 1 commit into from Sep 7, 2017

Conversation

@ferjm
Copy link
Member

commented Aug 28, 2017

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #18109
  • There are tests for these changes. I enabled the peformance-timeline API WPTs but some of them are still failing because of implementation bugs or missing APIs (Resource Timing, for instance) the tests are dependent of. I'll file issues to fix them.

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Aug 28, 2017

Heads up! This PR modifies the following files:

  • @jgraham: tests/wpt/include.ini
  • @fitzgen: components/script/dom/performance.rs, components/script/dom/webidls/PerformanceMark.webidl, components/script/dom/macros.rs, components/script/dom/performanceobserver.rs, components/script/dom/performancemark.rs and 4 more
  • @KiChjang: components/script/dom/performance.rs, components/script/dom/webidls/PerformanceMark.webidl, components/script/dom/macros.rs, components/script/dom/performanceobserver.rs, components/script/dom/performancemark.rs and 4 more
@ferjm

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2017

r? @jdm

@highfive highfive assigned jdm and unassigned mbrubeck Aug 28, 2017
@jdm

This comment has been minimized.

Copy link
Member

commented Aug 28, 2017

@bors-servo: r+
Thanks!

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2017

📌 Commit 8cbd47e has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2017

⌛️ Testing commit 8cbd47e with merge a91250e...

bors-servo added a commit that referenced this pull request Aug 29, 2017
User Timing API

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #18109
- [X] There are tests for these changes. I enabled the peformance-timeline API WPTs but some of them are still failing because of implementation bugs or missing APIs (Resource Timing, for instance) the tests are dependent of. I'll file issues to fix them.

<!-- 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/18283)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2017

💔 Test failed - linux-rel-wpt

@ferjm ferjm force-pushed the ferjm:user.timing.api branch from 8cbd47e to 221aef9 Aug 29, 2017
@ferjm

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2017

⌛️ Trying commit 221aef9 with merge 845a6ea...

bors-servo added a commit that referenced this pull request Aug 29, 2017
User Timing API

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #18109
- [X] There are tests for these changes. I enabled the peformance-timeline API WPTs but some of them are still failing because of implementation bugs or missing APIs (Resource Timing, for instance) the tests are dependent of. I'll file issues to fix them.

<!-- 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/18283)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2017

💔 Test failed - linux-rel-wpt

@ferjm

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2017

I'll try to fix #18286 before merging this one.

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2017

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

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2017

@ferjm ferjm force-pushed the ferjm:user.timing.api branch from 24daea3 to 0e0519b Sep 4, 2017
@ferjm ferjm force-pushed the ferjm:user.timing.api branch from 0e0519b to 4ef68bd Sep 4, 2017
@tigercosmos tigercosmos referenced this pull request Sep 4, 2017
3 of 5 tasks complete
@ferjm ferjm force-pushed the ferjm:user.timing.api branch 2 times, most recently from 14b1e2e to 88df5ea Sep 5, 2017
.rev()
.position(|e| *e.entry_type() == *entry_type &&
*e.name() == *name) {
Some(pos) => self.entries[pos].start_time(),

This comment has been minimized.

Copy link
@jdm

jdm Sep 5, 2017

Member

If we use find instead of position, we can access the entry directly instead of using an index.

let global = self.global();
// Step 1.
if global.is::<Window>() && INVALID_ENTRY_NAMES.contains(&mark_name.as_ref()) {
return Err((Error::Syntax));

This comment has been minimized.

Copy link
@jdm

jdm Sep 5, 2017

Member

Nit: remove the extra ()s.

@@ -28,3 +28,14 @@ partial interface Performance {
PerformanceEntryList getEntriesByName(DOMString name,
optional DOMString type);
};

// http://www.w3.org/TR/user-timing/
@ferjm ferjm force-pushed the ferjm:user.timing.api branch from 88df5ea to 8412f54 Sep 6, 2017
@ferjm

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2017

r? @jdm

@jdm

This comment has been minimized.

Copy link
Member

commented Sep 7, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2017

📌 Commit 8412f54 has been approved by jdm

@jdm
jdm approved these changes Sep 7, 2017
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2017

⌛️ Testing commit 8412f54 with merge 1e93749...

bors-servo added a commit that referenced this pull request Sep 7, 2017
User Timing API

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #18109
- [X] There are tests for these changes. I enabled the peformance-timeline API WPTs but some of them are still failing because of implementation bugs or missing APIs (Resource Timing, for instance) the tests are dependent of. I'll file issues to fix them.

<!-- 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/18283)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2017

@bors-servo bors-servo merged commit 8412f54 into servo:master Sep 7, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
pylbrecht added a commit to pylbrecht/servo that referenced this pull request Sep 7, 2017
User Timing API

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix servo#18109
- [X] There are tests for these changes. I enabled the peformance-timeline API WPTs but some of them are still failing because of implementation bugs or missing APIs (Resource Timing, for instance) the tests are dependent of. I'll file issues to fix them.

<!-- 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/18283)
<!-- Reviewable:end -->

Make Performance Timeline API work in Workers
bors-servo added a commit that referenced this pull request Sep 8, 2017
An observer disconnected after a mark must receive the mark

<!-- Please describe your changes on the following line: -->
I worked on top of #18283 as suggested [here](#18284 (comment)).

r? @ferjm

<!-- 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 #18284 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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/18370)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Sep 11, 2017
TEST: fix and add case of po-observe.any.js

<!-- Please describe your changes on the following line: -->
1. implementing step 3 from the [`PerformanceObserver.observe()`](https://w3c.github.io/performance-timeline/#dom-performanceobserver-observe()) method spec properly.
2. also add cases about step 1 & 2

work on the top of #18283

 r? @ferjm
---
<!-- 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 #18285 (github issue number if applicable).

<!-- Either: -->
- [ ] 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/18372)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Sep 11, 2017
TEST: fix and add case of po-observe.any.js

<!-- Please describe your changes on the following line: -->
1. implementing step 3 from the [`PerformanceObserver.observe()`](https://w3c.github.io/performance-timeline/#dom-performanceobserver-observe()) method spec properly.
2. also add cases about step 1 & 2

work on the top of #18283

 r? @ferjm
---
<!-- 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 #18285 (github issue number if applicable).

<!-- Either: -->
- [ ] 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/18372)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.