As part of the work to make httpd.js support keep-alive connections test docshell/test/test_bug570341.html needs a little tweak. When the iframe is loaded using a keep-alive connection, some events (dnsLookup and connection) are not stamped, because they simply don't occur. Solution is to force cache bypass reload of the iframe. Such force reload closes all keep-alive connections with the host.

Other (probably cleaner) solution is to publish API for closing connections per host. We may do that in a followup as it is not that trivial as this solution.

Does test fail for persistent connection? If some events don't occur, there should be a reasonable default for them, as described in the spec, most likely fetchStart like for domainLookupStart, see https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/NavigationTiming/Overview.html#nt-domain-lookup-start Test shouldn't break in this case. We probably need tests for both cases if there is a way to enforce persistent connection

Yes, we could simply run the test twice: ones as it is in the unfixed version and ones with the hard reload (what is my fix). I will go through the spec, but problem is that fixing the code according the spec might take much longer time then having the test server support keep-alive. And it would block more important things to finish.

I'd expect that implementation is correct and should use fetchStart for persistent connection, so not much changes are necessary. Do we need to fix tests because of breakage or because we need to cover the case with new connection? If tests break, I actually need to fix the API code, not hide the issue by enforcing a new connection in tests.

(In reply to comment #4) > I'd expect that implementation is correct and should use fetchStart for > persistent connection, so not much changes are necessary. Aha, so the test just needs some extending, do I understand correctly? > Do we need to fix tests because of breakage or because we need to cover the > case with new connection? I think the letter. Mochitest testing server is by now always closing a connection after a request has been satisfied. So, if the test is not already prepared to cover also keep-alive connections, then it must be extended to do it. > If tests break, I actually need to fix the API code, not hide the issue by > enforcing a new connection in tests. You seem to know more then me about this. Feel free to obsolete my patch and make yours. My patch just enforces a new connection to let the test run on it. Patches to make the tests server use keep-alive connections are in bug 469228 (there is also a single merged patch to apply), then build network/test/httpserver and testing/mochitest dirs.

Fix to the navigation.timing implementation. It turns out I was not implement fallback to fetchStart for missing channel-level events. Bugzilla doesn't let me obsolete the previous attachment which seems unnecessary now--with new implementation test passes.

Comment on attachment 549778 [details] [diff] [review] v1 I'll just drop the review request for now. Let's see how Igor's patch goes and obsolete after it gets in.

Comment on attachment 550708 [details] [diff] [review] Fix to navigation timing implementation for keep-alive connections Yeah, Fetch start is probably the best value we can get here.

Confirming this works with keep-alive support on httpd.js. Thanks.

Actually, fetchStart is what the spec says, and it was in the code before some of refactorings. Thanks for the review, I also need help with running the patch trough try server and submitting it.

Honza, since you've been testing the patch, could you push it to try, and if everything looks ok, to m-c also?

I'll try both, but might not happen sooner then on Monday.

Pushed to try: http://tbpl.mozilla.org/?tree=Try&pusher=cbiesinger@gmail.com&rev=53b8006946b0 We'll want this on aurora, right? (did webtiming end up on beta yet?)