Closed
Bug 1343600
Opened 8 years ago
Closed 8 years ago
Add TLS handshake Start/Stop events
Categories
(Core :: Networking: HTTP, enhancement)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: dragana, Assigned: dragana)
References
Details
(Whiteboard: [necko-active])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
dragana
:
review+
|
Details | Diff | Splinter Review |
We do not have tls handshake events and there for tls time is not counted for navigationTiming
Assignee | ||
Updated•8 years ago
|
Whiteboard: [necko-active]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8842550 -
Flags: review?(valentin.gosu)
Comment 2•8 years ago
|
||
Comment on attachment 8842550 [details] [diff] [review]
bug_1343600.patch
Review of attachment 8842550 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/protocol/http/nsHttpTransaction.cpp
@@ +2109,5 @@
> }
>
> void
> +nsHttpTransaction::SetConnectEnd(mozilla::TimeStamp timeStamp, bool onlyIfNull,
> + bool tlsEnd)
default argument mismatch! See header.
::: netwerk/protocol/http/nsHttpTransaction.h
@@ +152,5 @@
> void SetDomainLookupStart(mozilla::TimeStamp timeStamp, bool onlyIfNull = false);
> void SetDomainLookupEnd(mozilla::TimeStamp timeStamp, bool onlyIfNull = false);
> void SetConnectStart(mozilla::TimeStamp timeStamp, bool onlyIfNull = false);
> + void SetConnectEnd(mozilla::TimeStamp timeStamp, bool tlsEnd,
> + bool onlyIfNull = false);
Here you have tlsEnd as the second argument, but in the definition of the method you have it as the last argument, so it probably doesn't do what you want it to.
I would leave the signature as it was, and call it with onlyIfNull = true for CONNECTED_TO and onlyIfNull = false for HANDSHAKE_ENDED since we want it to overwrite the connectEnd timestamp.
Attachment #8842550 -
Flags: review?(valentin.gosu) → review-
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8842550 -
Attachment is obsolete: true
Attachment #8842808 -
Flags: review?(valentin.gosu)
Comment 4•8 years ago
|
||
Comment on attachment 8842808 [details] [diff] [review]
bug_1343600.patch
Review of attachment 8842808 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/protocol/http/nsHttpTransaction.cpp
@@ +605,5 @@
> SetConnectStart(TimeStamp::Now());
> } else if (status == NS_NET_STATUS_CONNECTED_TO) {
> + SetConnectEnd(TimeStamp::Now(), true);
> + } else if (status == NS_NET_STATUS_TLS_HANDSHAKE_ENDED) {
> + SetConnectEnd(TimeStamp::Now(), flase);
typo: false
Attachment #8842808 -
Flags: review?(valentin.gosu) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8842808 -
Attachment is obsolete: true
Attachment #8842819 -
Flags: review+
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8842819 -
Attachment is obsolete: true
Attachment #8842820 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/08df0e07f0ba
add TLS handshake Start/Stop events. r=:valentin
Keywords: checkin-needed
Comment 8•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8564a2c30a3
add text for NS_NET_STATUS_TLS_HANDSHAKE_STARTING and NS_NET_STATUS_TLS_HANDSHAKE_ENDED tansport states. r=valentin
Comment 10•8 years ago
|
||
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2013bea9eec
Backed out changeset a8564a2c30a3 for landing with wrong bug number
You need to log in
before you can comment on or make changes to this bug.
Description
•