Closed Bug 1253379 Opened 6 years ago Closed 6 years ago

Not all timing data are carried to the child process

Categories

(Core :: Networking: HTTP, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- wontfix
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: mayhemer, Assigned: valentin)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file)

Apparently the patch [1], bug 1064706, doesn't carry at least the cache load timings.  This is a major telemetry and timing break.

Valentin, can you please take a look and check what is missing and fix it?


[1] https://bugzilla.mozilla.org/attachment.cgi?id=8500326&action=diff#a/netwerk/protocol/http/HttpChannelParent.cpp_sec3
Flags: needinfo?(valentin.gosu)
Status: ASSIGNED → NEW
Duplicate of this bug: 1225097
Attachment #8726385 - Flags: review?(honzab.moz) → review+
Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

https://reviewboard.mozilla.org/r/37951/#review34517

::: netwerk/protocol/http/TimingStruct.h:33
(Diff revision 1)
> +  // the rest of the timings.

please say also WHY.
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED
Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37951/diff/1-2/
Whiteboard: [necko-active]
https://hg.mozilla.org/mozilla-central/rev/96bf29982e94
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Valentin, please nominate this up to beta.
Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Approval Request Comment

[Feature/regressing bug #]:
Bug 1064706 did not pass all of the timing data to the content process.

[User impact if declined]:
Telementry of cache timing on the content process will report wrong values.

[Describe test coverage new/current, TreeHerder]:
We have tests for other timings, but not cacheReadStart/End.

[Risks and why]: 
This is low risk. It only sends the missing cacheReadStart/End timestamps to the content process. It does not change behaviour, only the data reported in the child process.

[String/UUID change made/needed]:
None.
Flags: needinfo?(valentin.gosu)
Attachment #8726385 - Flags: approval-mozilla-beta?
Attachment #8726385 - Flags: approval-mozilla-aurora?
Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Too late for 45
Attachment #8726385 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Fix for telemetry issue, we want good info in 46. 
Please uplift to beta (46 is beta now, we just did the merge an hour ago)
Attachment #8726385 - Flags: approval-mozilla-beta-
Attachment #8726385 - Flags: approval-mozilla-beta+
Attachment #8726385 - Flags: approval-mozilla-aurora?
Attachment #8726385 - Flags: approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.