Closed
Bug 1263428
Opened 8 years ago
Closed 8 years ago
Intermittent test_resource_timing.html | PerformanceEntry has correct order of timing attributes (link) - assert_greater_than_equal: connectStart after domainLookupEnd expected a number greater than or equal to 482.3286635493814 but got 482.28181595114637
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | --- | fixed |
firefox49 | --- | fixed |
People
(Reporter: aryx, Assigned: valentin)
Details
(Keywords: intermittent-failure, regression, Whiteboard: [necko-active])
Attachments
(1 file)
2.87 KB,
patch
|
mcmanus
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
https://treeherder.mozilla.org/logviewer.html#?job_id=25480412&repo=mozilla-inbound 23:53:17 INFO - TEST-PASS | /resource-timing/test_resource_timing.html | PerformanceEntry has correct name, initiatorType, startTime, and duration (img) 23:53:17 INFO - TEST-PASS | /resource-timing/test_resource_timing.html | PerformanceEntry has correct order of timing attributes (img) 23:53:17 INFO - TEST-PASS | /resource-timing/test_resource_timing.html | window.performance.getEntriesByName() and window.performance.getEntriesByNameType() return same data (link) 23:53:17 INFO - TEST-PASS | /resource-timing/test_resource_timing.html | PerformanceEntry has correct name, initiatorType, startTime, and duration (link) 23:53:17 INFO - TEST-UNEXPECTED-FAIL | /resource-timing/test_resource_timing.html | PerformanceEntry has correct order of timing attributes (link) - assert_greater_than_equal: connectStart after domainLookupEnd expected a number greater than or equal to 482.3286635493814 but got 482.28181595114637 23:53:17 INFO - test@http://web-platform.test:8000/resource-timing/test_resource_timing.js:171:9 23:53:17 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20 23:53:17 INFO - resource_load@http://web-platform.test:8000/resource-timing/test_resource_timing.js:162:5 23:53:17 INFO - inner/<@http://web-platform.test:8000/resource-timing/test_resource_timing.js:124:25 23:53:17 INFO - setTimeout handler*inner@http://web-platform.test:8000/resource-timing/test_resource_timing.js:123:21 23:53:17 INFO - setTimeout handler*inner@http://web-platform.test:8000/resource-timing/test_resource_timing.js:130:9 23:53:17 INFO - poll_for_stylesheet_load@http://web-platform.test:8000/resource-timing/test_resource_timing.js:132:5 23:53:17 INFO - onload@http://web-platform.test:8000/resource-timing/test_resource_timing.js:88:13 23:53:17 INFO - EventHandlerNonNull*@http://web-platform.test:8000/resource-timing/test_resource_timing.js:26:1 23:53:17 INFO - TEST-PASS | /resource-timing/test_resource_timing.html | window.performance.getEntriesByName() and window.performance.getEntriesByNameType() return same data (script)
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 2•8 years ago
|
||
This is a regression - e10s only - caused by a push that occurred on April 1st or before. Some of the orange failures were recorded in bug 1141370. I've looked at the change log and I don't think the networking changes can account for this regressions. There were a couple of e10s patches, but I don't see how it would lead to this failure.
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 5•8 years ago
|
||
Valentin, any ideas? This seems to have gotten a lot worse recently: https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1263428&startday=2016-03-28&endday=2016-04-15&tree=all
Flags: needinfo?(valentin.gosu)
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd04bd37115c1eaba4fc53cf3cd62dd3167565f3 Bug 1263428 - Add temporary code to debug intermittent WPT failure a=testonly
Comment hidden (Intermittent Failures Robot) |
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cd04bd37115c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 10•8 years ago
|
||
This is not yet fixed. Still debugging. Setting leave-open flag.
Status: RESOLVED → REOPENED
Flags: needinfo?(valentin.gosu)
Keywords: leave-open
Resolution: FIXED → ---
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 20•8 years ago
|
||
I actually think this should fix the issue. This was a racy condition, where we assumed that if requestStart is null, then this is not a persistant connection. The assumption was false, and when OnLookupComplete fired before had started sending the request, it would set a domainLookupEnd that was larger than connectStart.
Attachment #8746115 -
Flags: review?(mcmanus)
Comment 21•8 years ago
|
||
Comment on attachment 8746115 [details] [diff] [review] Make sure we don't set the domainLookupEnd timestamp after connectStart for non-persistant connections Review of attachment 8746115 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/http/nsHttpChannel.cpp @@ +6093,5 @@ > mTransactionPump = nullptr; > > // We no longer need the dns prefetch object > if (mDNSPrefetch && mDNSPrefetch->TimingsValid() > && !mTransactionTimings.requestStart.IsNull() do you still need the requestStart null check?
Attachment #8746115 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 22•8 years ago
|
||
(In reply to Patrick McManus [:mcmanus] from comment #21) > > && !mTransactionTimings.requestStart.IsNull() > > do you still need the requestStart null check? I left it in so we wouldn't change the behaviour - in case the connect fails.
Assignee | ||
Comment 23•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/119f55446e975341b3d09cd4036dc39a3b59ec82 Bug 1263428 - Make sure we don't set the domainLookupEnd timestamp after connectStart for non-persistant connections r=mcmanus
Comment hidden (Intermittent Failures Robot) |
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/119f55446e97
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 27•8 years ago
|
||
No failures since it landed on central. I think we can mark this as fixed.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Comment 28•8 years ago
|
||
Thanks!
status-firefox47:
--- → unaffected
status-firefox49:
--- → fixed
Keywords: leave-open
Target Milestone: mozilla48 → mozilla49
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 31•8 years ago
|
||
Can you please request Aurora approval on this when you get a chance?
Flags: needinfo?(valentin.gosu)
Assignee | ||
Comment 32•8 years ago
|
||
Comment on attachment 8746115 [details] [diff] [review] Make sure we don't set the domainLookupEnd timestamp after connectStart for non-persistant connections Approval Request Comment [Feature/regressing bug #]: Bug 1123920 [User impact if declined]: intermittent failures when DNS prefetch is involved. [Describe test coverage new/current, TreeHerder]: both mochitests and WPT [Risks and why]: low risk. Patch makes sure we only use DNS prefetch timings when they make sense. [String/UUID change made/needed]: none.
Flags: needinfo?(valentin.gosu)
Attachment #8746115 -
Flags: approval-mozilla-aurora?
Comment hidden (Intermittent Failures Robot) |
Comment 34•8 years ago
|
||
Comment on attachment 8746115 [details] [diff] [review] Make sure we don't set the domainLookupEnd timestamp after connectStart for non-persistant connections Fix an intermittent, taking it.
Attachment #8746115 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 35•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/9d9a6bbe53f4
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•