Closed
Bug 1373656
Opened 7 years ago
Closed 7 years ago
ReportRcwnStats relies on null mTransactionPump, which is skewed
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla56
| Tracking | Status | |
|---|---|---|
| firefox56 | --- | fixed |
People
(Reporter: CuveeHsu, Assigned: CuveeHsu)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-active])
Attachments
(1 file, 1 obsolete file)
|
4.03 KB,
patch
|
michal
:
review+
|
Details | Diff | Splinter Review |
No description provided.
|
Assignee | |
Comment 1•7 years ago
|
||
Current ReportRcwnStats will use the null mTransactionPump since http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.cpp#7233
Attachment #8878504 -
Flags: review?(michal.novotny)
|
||
Comment 2•7 years ago
|
||
Comment on attachment 8878504 [details] [diff] [review] correctProbe - v1 With this change we will call ReportRcwnStats in cases when we didn't before because the method returned early at: http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.cpp#7254 http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.cpp#7278 http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.cpp#7357 Is this something we want?
Flags: needinfo?(juhsu)
|
|
Assignee | |
Comment 3•7 years ago
|
||
> With this change we will call ReportRcwnStats in cases when we didn't before
> because the method returned early at:
>
> http://searchfox.org/mozilla-central/rev/
> 20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.
> cpp#7254
> http://searchfox.org/mozilla-central/rev/
> 20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.
> cpp#7278
> http://searchfox.org/mozilla-central/rev/
> 20d16dadd336e0c6b97e3e19dc4ff907744b5734/netwerk/protocol/http/nsHttpChannel.
> cpp#7357
>
> Is this something we want?
I didn't think this before.
We care about the time we send OnStopRequest to the mListener.
IMO The best place to report is right before mListener->OnStopRequest.
Thanks for catching this!Flags: needinfo?(juhsu)
|
|
Assignee | |
Comment 4•7 years ago
|
||
Attachment #8878504 -
Attachment is obsolete: true
Attachment #8878504 -
Flags: review?(michal.novotny)
Attachment #8880317 -
Flags: review?(michal.novotny)
|
|
Assignee | |
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac0e25064b69f7b4d37d97854e977355b9ccd5e4
|
|
||
Updated•7 years ago
|
Attachment #8880317 -
Flags: review?(michal.novotny) → review+
|
|
Assignee | |
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9a5197a02e7 Move |ReportRcwnStats| before we set mTransactionPump to nullptr. r=michal
Keywords: checkin-needed
|
||
Comment 7•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e9a5197a02e7
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•