RTCDataChannelStats added.

RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
Committed: https://crrev.com/cc555c5019850f16343d0c6b00fb1abad9ba5d2a
Cr-Commit-Position: refs/heads/master@{#14670}

[hbos, 2016-10-12 21:23:37]

Description was changed from

==========
RTCDataChannelStats[1] added, all members supported.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
==========

to

==========
RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
==========

[hbos, 2016-10-12 21:23:37]

hbos@webrtc.org changed reviewers:
+ deadbeef@webrtc.org, hta@webrtc.org

[hbos, 2016-10-12 21:23:45]

Patchset #1 (id:1) has been deleted

[hbos, 2016-10-12 21:23:52]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 21:23:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[hbos, 2016-10-12 21:24:25]

Please take a look, deadbeef and hta.

[commit-bot: I haz the power, 2016-10-12 21:48:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 21:48:51]

Dry run: This issue passed the CQ dry run.

[hta-webrtc, 2016-10-12 22:23:39]

lgtm % test function name comment
(I just learned this myself, so eager to spread the word)

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.h (right):

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.h:125: const char* DataStateToRTCDataChannelState(
mcasas told me that functions that are exposed for testing should always have
names ending in "ForTesting" - there's a presubmit tool that will flag usage of
such functions outside tests.

"Functions used only for testing should be restricted 
to test-only scenarios either by #ifdefing them 
appropriately (e.g. #if defined(UNIT_TEST)) or by 
naming them with a ForTesting suffix. The latter will 
be checked at presubmit time to ensure they're only 
called by test files."

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/test/mock_data...
File webrtc/api/test/mock_datachannel.h (right):

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/test/mock_data...
webrtc/api/test/mock_datachannel.h:23: }
This constructor is pretty hideous (4 positional arguments of the same type),
but since we're only using it for this specific test, I suppose that's OK.

[hbos, 2016-10-17 17:33:16]

Please take a look, deadbeef.

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.h (right):

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.h:125: const char* DataStateToRTCDataChannelState(
On 2016/10/12 22:23:39, hta-webrtc wrote:
> mcasas told me that functions that are exposed for testing should always have
> names ending in "ForTesting" - there's a presubmit tool that will flag usage
of
> such functions outside tests.
> 
> "Functions used only for testing should be restricted 
> to test-only scenarios either by #ifdefing them 
> appropriately (e.g. #if defined(UNIT_TEST)) or by 
> naming them with a ForTesting suffix. The latter will 
> be checked at presubmit time to ensure they're only 
> called by test files."
> 
> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

Cool. So they're helper functions used by the .cc file (not a test) that if not
for usage in test files would live in the .cc's anonymous namespace. Sounds like
the presubmit would not allowed these to be called ForTesting.

I moved them to anonymous namespace and made "ForTesting" versions invoking them
that are only used in unittests.

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/test/mock_data...
File webrtc/api/test/mock_datachannel.h (right):

https://codereview.webrtc.org/2420473002/diff/20001/webrtc/api/test/mock_data...
webrtc/api/test/mock_datachannel.h:23: }
On 2016/10/12 22:23:39, hta-webrtc wrote:
> This constructor is pretty hideous (4 positional arguments of the same type),
> but since we're only using it for this specific test, I suppose that's OK.

Acknowledged.

[hta-webrtc, 2016-10-17 17:43:46]

lgtm

[Taylor Brandstetter, 2016-10-18 17:28:11]

lgtm

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:246: const
std::vector<rtc::scoped_refptr<DataChannel>>& data_channels =
nit: Don't need this temporary variable, could just put
pc_->sctp_data_channels() in the for statement.

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector_unittest.cc (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:333: EXPECT_LE(stats.timestamp_us(),
after);
Should there be a separate test for the stats timestamp, using
https://cs.chromium.org/chromium/src/third_party/webrtc/base/fakeclock.h maybe?

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/test/mock_data...
File webrtc/api/test/mock_datachannel.h (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/test/mock_data...
webrtc/api/test/mock_datachannel.h:23: }
Instead of these constructors, why not use EXPECT_CALL in the unit test itself?
That would make the unit test more clear. But I guess we've already started the
pattern with this object, so little point in changing it now.

[hbos, 2016-10-18 19:13:38]

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:246: const
std::vector<rtc::scoped_refptr<DataChannel>>& data_channels =
On 2016/10/18 17:28:11, Taylor Brandstetter wrote:
> nit: Don't need this temporary variable, could just put
> pc_->sctp_data_channels() in the for statement.

Done.

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector_unittest.cc (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:333: EXPECT_LE(stats.timestamp_us(),
after);
On 2016/10/18 17:28:11, Taylor Brandstetter wrote:
> Should there be a separate test for the stats timestamp, using
> https://cs.chromium.org/chromium/src/third_party/webrtc/base/fakeclock.h
maybe?

Good idea. The cache uses mockable time, but the timestamp used on the reports
are based on current UTC time, which at the moment isn't mockable. So not
addressing in this CL.

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/test/mock_data...
File webrtc/api/test/mock_datachannel.h (right):

https://codereview.webrtc.org/2420473002/diff/40001/webrtc/api/test/mock_data...
webrtc/api/test/mock_datachannel.h:23: }
On 2016/10/18 17:28:11, Taylor Brandstetter wrote:
> Instead of these constructors, why not use EXPECT_CALL in the unit test
itself?
> That would make the unit test more clear. But I guess we've already started
the
> pattern with this object, so little point in changing it now.

Acknowledge. I could create a helper function that creates and does EXPECT to
avoid this pattern here if you want to? But landing this as-is.

[hbos, 2016-10-18 19:13:43]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2016-10-18 19:13:44]

The patchset sent to the CQ was uploaded after l-g-t-m from hta@webrtc.org,
deadbeef@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2420473002/#ps60001 (title:
"Addressed nit")

[commit-bot: I haz the power, 2016-10-18 19:13:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-10-18 19:48:33]

Description was changed from

==========
RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
==========

to

==========
RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
==========

[commit-bot: I haz the power, 2016-10-18 19:48:34]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-10-18 19:48:43]

Description was changed from

==========
RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816
==========

to

==========
RTCDataChannelStats[1] added, supporting all stats members.

Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816

Committed: https://crrev.com/cc555c5019850f16343d0c6b00fb1abad9ba5d2a
Cr-Commit-Position: refs/heads/master@{#14670}
==========

[commit-bot: I haz the power, 2016-10-18 19:48:44]

Patchset 3 (id:??) landed as
https://crrev.com/cc555c5019850f16343d0c6b00fb1abad9ba5d2a
Cr-Commit-Position: refs/heads/master@{#14670}