1678312 - TLS handshake timings are not correct

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Dragana Damjanovic [:dragana]]

Handling of socket notification in nsHttpTransaction is not correct.

Comment 1

[Dragana Damjanovic [:dragana]]

I will not have time to work on this bug, so I am unassigning myself.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Do we know in what way the notification is not correct?

Comment 3

[Kershaw Chang [:kershaw]]

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #2)

Do we know in what way the notification is not correct?

Sorry, I haven't looked into this. I'll do this soon.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Btw, when trying to land bug 1801292 that fixed some of the TLS timings.

Comment 5

[Kershaw Chang [:kershaw]]

In our current code, we set the timing secureConnectionStart to the time when the HTTP/3 connection is establised here, which is not right.
To fix this, we might need to expose this information from neqo.

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Having those timings be correct might help with performance profiling.

Comment 7

[Kershaw Chang [:kershaw]]

Attachment: Bug 1678312 - Make sure page load telemetry probes are reported properly, r=#necko

Before this patch, TelemetryReportChannel(timedChannel, true) was only called within nsLoadGroup::TelemetryReport() when all requests were completed in the load group.
However, SetDefaultLoadRequest(nullptr) could be called before this, resulting in mDefaultLoadRequest being set to null and causing page load metrics to sometimes not be reported.
To fix this, this patch moves TelemetryReportChannel(timedChannel, true) to the point where mDefaultLoadRequest is completed.

Comment 8

[Kershaw Chang [:kershaw]]

Attachment: Bug 1678312 - Mapping Http3Event::Tag::AuthenticationNeeded to NS_NET_STATUS_TLS_HANDSHAKE_STARTING, r=#necko

Depends on D221597

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfc31bf59096
Make sure page load telemetry probes are reported properly, r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/aa6305a65fb5
Mapping Http3Event::Tag::AuthenticationNeeded to NS_NET_STATUS_TLS_HANDSHAKE_STARTING, r=necko-reviewers,valentin

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/dfc31bf59096
https://hg.mozilla.org/mozilla-central/rev/aa6305a65fb5