Let resources timing entries be added to performance entry buffer. #23536

gatoWololo · 2019-06-07T20:24:05Z

Update relevant tests to properly report and expect failure.

These changes are the first step for #23328. Not sure if I would say they fix the issue.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #___ (GitHub issue number if applicable)

There are tests for these changes: Existing tests have been updated to expect pass/failures.

This change is

highfive · 2019-06-07T20:24:13Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @avadacatavra (or someone else) soon.

highfive · 2019-06-07T20:24:14Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/network_listener.rs
@KiChjang: components/script/network_listener.rs

jdm · 2019-06-07T20:53:11Z

@bors-servo try=wpt

bors-servo · 2019-06-07T20:53:15Z

⌛ Trying commit af8520f with merge 952f1ab...


        Auto merge of #23536 - gatoWololo:omar_timing_entries, r=<try>

Let resources timing entries be added to performance entry buffer.  Update relevant tests to properly report and expect failure. These changes are the first step for #23328. Not sure if I would say they _fix_ the issue. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [X] There are tests for these changes: Existing tests have been updated to expect pass/failures.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23536)

bors-servo · 2019-06-07T21:17:14Z

💔 Test failed - linux-rel-css

jdm · 2019-06-07T21:43:24Z

Looks like a few more test results changed:

{
    "status": "FAIL", 
    "group": "default", 
    "message": "uncaught exception: unknown (can't convert to string)", 
    "stack": "image_was_loaded/<@http://web-platform.test:8000/html/semantics/embedded-content/the-img-element/srcset/avoid-reload-on-resize.html:15:5\nstep_timeout/<@http://web-platform.test:8000/resources/testharness.js:806:13\n", 
    "subtest": "Avoid srcset image reloads when viewport resizes", 
    "test": "/html/semantics/embedded-content/the-img-element/srcset/avoid-reload-on-resize.html", 
    "line": 179867, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "FAIL", 
    "group": "default", 
    "message": "assert_equals: expected 1 but got 3", 
    "stack": "request.onload<@http://web-platform.test:8000/_mozilla/mozilla/xmlhttprequest_url.html:22:5\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20\nTest.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:1634:17\n", 
    "subtest": "Performance entries should contain the URL where the XMLHttpRequest originated", 
    "test": "/_mozilla/mozilla/xmlhttprequest_url.html", 
    "line": 302755, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "FAIL", 
    "group": "default", 
    "message": "assert_equals: clearResourceTimings cleared the resource timings expected 0 but got 1", 
    "stack": "testPerformanceClearResourceTimings@http://web-platform.test:8000/workers/worker-performance.worker.js:95:5\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:21\n@http://web-platform.test:8000/workers/worker-performance.worker.js:93:1\n", 
    "subtest": "performance.clearResourceTimings in workers", 
    "test": "/workers/worker-performance.worker.html", 
    "line": 302815, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "FAIL", 
    "group": "default", 
    "message": "assert_equals: setResourceTimingBufferSize(0) prevents resource entries from being added expected 0 but got 2", 
    "stack": "testPerformanceSetResourceTimingBufferSize@http://web-platform.test:8000/workers/worker-performance.worker.js:103:5\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:21\n@http://web-platform.test:8000/workers/worker-performance.worker.js:98:1\n", 
    "subtest": "performance.setResourceTimingBufferSize in workers", 
    "test": "/workers/worker-performance.worker.html", 
    "line": 302816, 
    "action": "test_result", 
    "expected": "PASS"
}

jdm · 2019-06-07T21:44:39Z

/xmlhttprequest_url.html should be updated to accommodate the new changes. I expressed surprise that it passed in #23372 (comment), and I'm glad to see that this change makes it behave as I expected.

gatoWololo · 2019-06-10T20:44:08Z

This pull request has been marked as Queued since last Friday without any progress. I'm pretty sure it's stuck?

jdm · 2019-06-10T20:59:28Z

Indeed! Good thing the TravisCI tests are just early-warning systems instead of mission-critical.
@bors-servo r+

bors-servo · 2019-06-10T20:59:29Z

📌 Commit 4a8bdce has been approved by jdm

bors-servo · 2019-06-10T20:59:34Z

⌛ Testing commit 4a8bdce with merge 4d6b5eb...


        Auto merge of #23536 - gatoWololo:omar_timing_entries, r=jdm

Let resources timing entries be added to performance entry buffer.  Update relevant tests to properly report and expect failure. These changes are the first step for #23328. Not sure if I would say they _fix_ the issue. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [X] There are tests for these changes: Existing tests have been updated to expect pass/failures.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23536)

bors-servo · 2019-06-10T21:36:07Z

💔 Test failed - linux-rel-wpt

gatoWololo · 2019-06-11T16:19:16Z

@jdm That makes sense thanks. I was just worried about erroneously modifying the tests.

gatoWololo · 2019-06-11T16:37:46Z

Well, this is fun. Calling ./mach test-wpt --release /performance-timeline several times. It randomly passes half the time, fails the other half.
Passes:

gatowololo@omar-mozilla ~/M/servo> ./mach test-wpt --release /performance-timeline
...
Running 32 tests in web-platform-tests

Ran 32 tests finished in 15.0 seconds.
  • 32 ran as expected. 0 tests skipped.

Fails:

gatowololo@omar-mozilla ~/M/servo> ./mach test-wpt --release /performance-timeline
Running 32 tests in web-platform-tests

  ▶ Unexpected subtest result in /performance-timeline/webtiming-resolution.any.worker.html:
  │ FAIL [expected PASS] Verifies the resolution of performance.now() is at least 5 microseconds.
  │   → assert_greater_than_equal: The second performance.now() should be much greater than the first expected a number greater than or equal to 0.0049900000000000005 but got 0.002425000000002342
  │ 
  │ testTimeResolution/<@http://web-platform.test:8000/performance-timeline/webtiming-resolution.any.js:9:9
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20
  │ test@http://web-platform.test:8000/resources/testharness.js:544:21
  │ testTimeResolution@http://web-platform.test:8000/performance-timeline/webtiming-resolution.any.js:2:5
  │ @http://web-platform.test:8000/performance-timeline/webtiming-resolution.any.js:24:1
  └ @http://web-platform.test:8000/performance-timeline/webtiming-resolution.any.worker.js:8:1

Ran 32 tests finished in 15.0 seconds.
  • 31 ran as expected. 0 tests skipped.
  • 1 tests had unexpected subtest results

jdm · 2019-06-11T16:40:11Z

That's a known intermittent failure: #21276


        Let resources timing entries be added to performance entry buffer.

Update relevant tests to properly report and expect failure. Update manifest.

asajeffrey · 2019-06-11T17:02:08Z

Searching the github repo for wpt test names quite often finds the right intermittent.

gatoWololo · 2019-06-11T17:03:57Z

Thanks, in the future I'll look them up first.

jdm · 2019-06-14T06:07:54Z

@bors-servo r+

bors-servo · 2019-06-14T06:07:56Z

📌 Commit 9eb3687 has been approved by jdm

bors-servo · 2019-06-14T06:35:34Z

⌛ Testing commit 9eb3687 with merge 08cc072...


        Auto merge of #23536 - gatoWololo:omar_timing_entries, r=jdm

Let resources timing entries be added to performance entry buffer.  Update relevant tests to properly report and expect failure. These changes are the first step for #23328. Not sure if I would say they _fix_ the issue. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [X] There are tests for these changes: Existing tests have been updated to expect pass/failures.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23536)

bors-servo · 2019-06-14T08:50:12Z

💔 Test failed - status-taskcluster

CYBAI · 2019-06-14T13:28:08Z

{
    "status": "FAIL", 
    "group": "default", 
    "message": "assert_true: responseEnd should not be 0 in timing-allow cross-origin request. expected true got false", 
    "stack": "test_greater_than/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:141:27\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:21\nwp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:5\ntest_greater_than@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:141:5\nonload_test@http://web-platform.test:8000/resource-timing/resource_TAO_origin.htm:43:5\n", 
    "subtest": "responseEnd should not be 0 in timing-allow cross-origin request.", 
    "test": "/resource-timing/resource_TAO_origin.htm", 
    "line": 66551, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "FAIL", 
    "group": "default", 
    "message": "assert_true: responseEnd should be greater than 0 in cross-origin request. expected true got false", 
    "stack": "test_greater_than/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:141:27\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:21\nwp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:5\ntest_greater_than@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:141:5\nonload_test@http://web-platform.test:8000/resource-timing/resource_TAO_zero.htm:42:5\n", 
    "subtest": "responseEnd should be greater than 0 in cross-origin request.", 
    "test": "/resource-timing/resource_TAO_zero.htm", 
    "line": 70352, 
    "action": "test_result", 
    "expected": "PASS"
}

jdm · 2019-06-14T13:39:33Z

I wonder if those are new intermittent results...

jdm · 2019-06-24T14:36:46Z

@bors-servo retry
#23625

bors-servo · 2019-06-24T14:48:41Z

⌛ Testing commit 9eb3687 with merge 7490dd6...


        Auto merge of #23536 - gatoWololo:omar_timing_entries, r=jdm

Let resources timing entries be added to performance entry buffer.  Update relevant tests to properly report and expect failure. These changes are the first step for #23328. Not sure if I would say they _fix_ the issue. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [X] There are tests for these changes: Existing tests have been updated to expect pass/failures.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23536)

bors-servo · 2019-06-24T17:39:05Z

☀️ Test successful - arm64, linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 7490dd6 to master...

highfive assigned avadacatavra Jun 7, 2019

highfive added the S-awaiting-review label Jun 7, 2019

highfive added the S-tests-failed label Jun 7, 2019

jdm assigned jdm and unassigned avadacatavra Jun 7, 2019

jdm added S-needs-code-changes and removed S-awaiting-review labels Jun 7, 2019

gatoWololo force-pushed the gatoWololo:omar_timing_entries branch from af8520f to ff20e35 Jun 7, 2019

highfive added S-awaiting-review and removed S-tests-failed S-needs-code-changes labels Jun 7, 2019

gatoWololo force-pushed the gatoWololo:omar_timing_entries branch from ff20e35 to 4a8bdce Jun 7, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 10, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 10, 2019

Let resources timing entries be added to performance entry buffer.

Loading status checks…

9eb3687

Update relevant tests to properly report and expect failure. Update manifest.

gatoWololo force-pushed the gatoWololo:omar_timing_entries branch from 4a8bdce to 9eb3687 Jun 11, 2019

highfive added S-awaiting-review and removed S-tests-failed labels Jun 11, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 14, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 14, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Jun 24, 2019

bors-servo mentioned this pull request Jun 24, 2019

style: Sync changes from mozilla-central. #23609

Merged

highfive removed the S-awaiting-merge label Jun 24, 2019

jdm mentioned this pull request Jul 12, 2019

Resource timing entries are never added to the performance entry buffer #23328

Closed

servo / servo

Let resources timing entries be added to performance entry buffer. #23536

Let resources timing entries be added to performance entry buffer. #23536

gatoWololo commented Jun 7, 2019 •

edited by jdm

highfive commented Jun 7, 2019

highfive commented Jun 7, 2019

jdm commented Jun 7, 2019

bors-servo commented Jun 7, 2019

bors-servo commented Jun 7, 2019

jdm commented Jun 7, 2019

jdm commented Jun 7, 2019

gatoWololo commented Jun 10, 2019

jdm commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

gatoWololo commented Jun 11, 2019

gatoWololo commented Jun 11, 2019

jdm commented Jun 11, 2019

asajeffrey commented Jun 11, 2019

gatoWololo commented Jun 11, 2019

jdm commented Jun 14, 2019

bors-servo commented Jun 14, 2019

bors-servo commented Jun 14, 2019

bors-servo commented Jun 14, 2019

CYBAI commented Jun 14, 2019

jdm commented Jun 14, 2019

jdm commented Jun 24, 2019

bors-servo commented Jun 24, 2019

bors-servo commented Jun 24, 2019

servo / servo

Join GitHub today

Let resources timing entries be added to performance entry buffer. #23536

Let resources timing entries be added to performance entry buffer. #23536

Conversation

gatoWololo commented Jun 7, 2019 • edited by jdm

highfive commented Jun 7, 2019

highfive commented Jun 7, 2019

jdm commented Jun 7, 2019

bors-servo commented Jun 7, 2019

bors-servo commented Jun 7, 2019

jdm commented Jun 7, 2019

jdm commented Jun 7, 2019

gatoWololo commented Jun 10, 2019

jdm commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

gatoWololo commented Jun 11, 2019

gatoWololo commented Jun 11, 2019

jdm commented Jun 11, 2019

asajeffrey commented Jun 11, 2019

gatoWololo commented Jun 11, 2019

jdm commented Jun 14, 2019

bors-servo commented Jun 14, 2019

bors-servo commented Jun 14, 2019

bors-servo commented Jun 14, 2019

CYBAI commented Jun 14, 2019

jdm commented Jun 14, 2019

jdm commented Jun 24, 2019

bors-servo commented Jun 24, 2019

bors-servo commented Jun 24, 2019

gatoWololo commented Jun 7, 2019 •

edited by jdm