Add Stats to Stun ping.

Add Stats to Stun ping.

Add sent_ping_requests, recv_ping_responses to ConnectionInfo.
recv_ping_responses_ will be incremented when OnConnectionRequestResponse() is called.
ent_ping_requests_ will be incremented when OnConnectionRequestSent() is called.

BUG=webrtc:5695
Committed: https://crrev.com/5ecf16c072cef53ecb270794577c39a66dcbac63
Cr-Commit-Position: refs/heads/master@{#13001}

[Zhi Huang, 2016-04-29 20:41:14]

zhihuang@webrtc.org changed reviewers:
+ deadbeef@webrtc.org, pthatcher@webrtc.org

[Zhi Huang, 2016-04-29 20:41:14]

One question about the type.
I noticed that the type of some stats in the Connection objects, such as rtt,
sent_packets_total_, are 32bit integer. Why do me make them 64bit when
generating the StatsReport in StatsCollecor?

[pthatcher1, 2016-05-05 23:30:57]

This looks good, but it would be nice to have the other stats as well.  If you
want to land this CL and do a second one after, that's fine too.

I don't know why some things are 32bit and others are 64bit.

https://codereview.webrtc.org/1940493002/diff/1/webrtc/api/statstypes.cc
File webrtc/api/statstypes.cc (right):

https://codereview.webrtc.org/1940493002/diff/1/webrtc/api/statstypes.cc#newc...
webrtc/api/statstypes.cc:421: return "consentResponsesReceived";
Can we also add consentRequestsReceived and consentResponsesSent?

That's what we decided at this W3C meeting:

https://www.w3.org/2011/04/webrtc/wiki/images/5/50/WebRTCWG-2016-02-25.pdf

https://codereview.webrtc.org/1940493002/diff/1/webrtc/p2p/base/transport.h
File webrtc/p2p/base/transport.h (right):

https://codereview.webrtc.org/1940493002/diff/1/webrtc/p2p/base/transport.h#n...
webrtc/p2p/base/transport.h:116: size_t sent_ping_requests;  // Number of STUN
ping request sent.
request => requests

https://codereview.webrtc.org/1940493002/diff/1/webrtc/p2p/base/transport.h#n...
webrtc/p2p/base/transport.h:120: size_t recv_ping_responses;  // Number of STUN
ping response received.
response => responses

[pthatcher1, 2016-05-27 21:27:15]

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/api/statstypes.cc
File webrtc/api/statstypes.cc (right):

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/api/statstypes.cc#...
webrtc/api/statstypes.cc:420: return "consentRequestsSent";
I think it's more likely this will end up "requestsSent" and similar for all of
these except for the next one.  So, just take of the "consent" part.

But who knows :).

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/api/statstypes.cc#...
webrtc/api/statstypes.cc:422: return "consentRequestsSentBeforeFirstResponse";
And this might be "consentRequestsSent".

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:658: uint32_t recv_ping_responses_;
Can we just store a ConnectionInfo value here instead?  That would avoid a lot
of duplication, and let us reduce the number of methods down to like info() or
stats() (I think we should rename it to IceCandidatePairStats).

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:669: bool received_first_response_;
Can you tell by "recv_ping_reponses_ > 0"?

[zhihuang1, 2016-06-01 21:30:05]

zhihuang@chromium.org changed reviewers:
+ zhihuang@chromium.org

[zhihuang1, 2016-06-01 21:30:05]

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/api/statstypes.cc
File webrtc/api/statstypes.cc (right):

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/api/statstypes.cc#...
webrtc/api/statstypes.cc:420: return "consentRequestsSent";
On 2016/05/27 21:27:15, pthatcher1 wrote:
> I think it's more likely this will end up "requestsSent" and similar for all
of
> these except for the next one.  So, just take of the "consent" part.
> 
> But who knows :).

Then I will keep the TODO part.

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:658: uint32_t recv_ping_responses_;
On 2016/05/27 21:27:15, pthatcher1 wrote:
> Can we just store a ConnectionInfo value here instead?  That would avoid a lot
> of duplication, and let us reduce the number of methods down to like info() or
> stats() (I think we should rename it to IceCandidatePairStats).

Do you mean renaming ConnectionInfo -> IceCandidatePairStats? Should I do it in
this CL or in a seperate CL?

[pthatcher1, 2016-06-01 21:39:49]

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/1940493002/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:658: uint32_t recv_ping_responses_;
On 2016/06/01 21:30:05, zhihuang1 wrote:
> On 2016/05/27 21:27:15, pthatcher1 wrote:
> > Can we just store a ConnectionInfo value here instead?  That would avoid a
lot
> > of duplication, and let us reduce the number of methods down to like info()
or
> > stats() (I think we should rename it to IceCandidatePairStats).
> 
> Do you mean renaming ConnectionInfo -> IceCandidatePairStats? Should I do it
in
> this CL or in a seperate CL?

In a separate CL is fine.

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:465: ConnectionInfo connection_info();
Can you name this stats() instead of connection_info()?

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:636: ConnectionInfo connection_info_;
And this stats_ instead of connection_info_?

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.c...
webrtc/p2p/base/tcpport.cc:354: //  sent_packets_total_++;
This comment isn't needed.

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.c...
webrtc/p2p/base/tcpport.cc:358: //    sent_packets_discarded_++;
Nor this one.

[zhihuang1, 2016-06-01 22:34:04]

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:465: ConnectionInfo connection_info();
On 2016/06/01 21:39:48, pthatcher1 wrote:
> Can you name this stats() instead of connection_info()?

Done.

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:636: ConnectionInfo connection_info_;
On 2016/06/01 21:39:48, pthatcher1 wrote:
> And this stats_ instead of connection_info_?

Done.

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.c...
webrtc/p2p/base/tcpport.cc:354: //  sent_packets_total_++;
On 2016/06/01 21:39:49, pthatcher1 wrote:
> This comment isn't needed.

Done.

https://codereview.webrtc.org/1940493002/diff/40001/webrtc/p2p/base/tcpport.c...
webrtc/p2p/base/tcpport.cc:358: //    sent_packets_discarded_++;
On 2016/06/01 21:39:49, pthatcher1 wrote:
> Nor this one.

Done.

[pthatcher1, 2016-06-01 23:24:37]

lgtm

[zhihuang1, 2016-06-02 00:03:49]

The CQ bit was checked by zhihuang@chromium.org

[commit-bot: I haz the power, 2016-06-02 00:03:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1940493002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1940493002/60001

[commit-bot: I haz the power, 2016-06-02 00:09:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-02 00:09:27]

Description was changed from

==========
Add Stats to Stun ping.

Add sent_ping_requests, recv_ping_responses to ConnectionInfo.
recv_ping_responses_ will be incremented when OnConnectionRequestResponse() is
called.
ent_ping_requests_ will be incremented when OnConnectionRequestSent() is called.

BUG=webrtc:5695
==========

to

==========
Add Stats to Stun ping.

Add sent_ping_requests, recv_ping_responses to ConnectionInfo.
recv_ping_responses_ will be incremented when OnConnectionRequestResponse() is
called.
ent_ping_requests_ will be incremented when OnConnectionRequestSent() is called.

BUG=webrtc:5695

Committed: https://crrev.com/5ecf16c072cef53ecb270794577c39a66dcbac63
Cr-Commit-Position: refs/heads/master@{#13001}
==========

[commit-bot: I haz the power, 2016-06-02 00:09:28]

Patchset 4 (id:??) landed as
https://crrev.com/5ecf16c072cef53ecb270794577c39a66dcbac63
Cr-Commit-Position: refs/heads/master@{#13001}