PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
Committed: https://crrev.com/74e1a4f96a4915c9ad3b0ddeff87cc9797606453
Cr-Commit-Position: refs/heads/master@{#14246}

[hbos, 2016-09-13 10:05:00]

Description was changed from

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

BUG=chromium:627816
==========

to

==========
------------------------------------------------------------------------
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

Files moved & dependency changes:
- Moving rtcstats.h, rtcstats_objects.h and rtcstatsreport.h to
  webrtc/stats/. These + .cc files make up the rtc_stats target, which
  no longer depend on libjingle_peerconnection.
- rtcstatscollector.[h/cc] is moved to webrtc/api/ and made part of the
  libjingle_peerconnection target. libjingle_peerconnection depends on
  rtc_stats but the circular dependency in the other direction is
  avoided. The RTCStatsCollector is viewed as an implementation detail
  of PeerConnection.
- DEPS is updated so that webrtc/api/ can include from webrtc/stats/.

Other changes:
- PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added
  for the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

[hbos, 2016-09-13 10:05:12]

Description was changed from

==========
------------------------------------------------------------------------
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

Files moved & dependency changes:
- Moving rtcstats.h, rtcstats_objects.h and rtcstatsreport.h to
  webrtc/stats/. These + .cc files make up the rtc_stats target, which
  no longer depend on libjingle_peerconnection.
- rtcstatscollector.[h/cc] is moved to webrtc/api/ and made part of the
  libjingle_peerconnection target. libjingle_peerconnection depends on
  rtc_stats but the circular dependency in the other direction is
  avoided. The RTCStatsCollector is viewed as an implementation detail
  of PeerConnection.
- DEPS is updated so that webrtc/api/ can include from webrtc/stats/.

Other changes:
- PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added
  for the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

to

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

Files moved & dependency changes:
- Moving rtcstats.h, rtcstats_objects.h and rtcstatsreport.h to
  webrtc/stats/. These + .cc files make up the rtc_stats target, which
  no longer depend on libjingle_peerconnection.
- rtcstatscollector.[h/cc] is moved to webrtc/api/ and made part of the
  libjingle_peerconnection target. libjingle_peerconnection depends on
  rtc_stats but the circular dependency in the other direction is
  avoided. The RTCStatsCollector is viewed as an implementation detail
  of PeerConnection.
- DEPS is updated so that webrtc/api/ can include from webrtc/stats/.

Other changes:
- PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added
  for the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

[hbos, 2016-09-13 10:30:01]

hbos@webrtc.org changed reviewers:
+ hta@webrtc.org, kjellander@webrtc.org, nisse@webrtc.org

[hbos, 2016-09-13 10:30:02]

Please take a look nisse, kjellander and hta. This being instead of
https://codereview.webrtc.org/2335553006/.

[nisse-webrtc, 2016-09-13 11:25:37]

On 2016/09/13 10:30:02, hbos wrote:
> Please take a look nisse, kjellander and hta. This being instead of
> https://codereview.webrtc.org/2335553006/.

I take it the files moved into stats/ are internal only?

Which header files define the public interface to stats? There's
api/statstypes.h, and api/statscollector.h. The latter include some stats/
headers, to define RTCStatsReport (argument type to the StatsCollector
callback). 

If possible, I think it would be more correct to leave the definitions of all
types referenced by the public API in the api/ folder. Production targets in
stats/ should only be libraries (no executables), so they don't need any
link-time dependency to peerconnection, right? Unit tests need to be linked
including a lot more.

Hmm, quoting the doc you pointed to earlier:
 
  The current idea is to move all supported headers into webrtc/api. Only header
files (and GYP,
  GN and BUILD) should live in webrtc/api and the files in there may not depend
any files outside
  of webrtc/api.

To follow this guideline, you should leave headers declaring public types in
api/, but the implementation files should be moved to stats/. Files under stats/
may include headers from api/, but not the other way around. Don't know if there
are any other webrtc pieces that are organized that way, most things in api/
seems to be one .h file and a corresponding .cc.

[hbos, 2016-09-13 12:00:21]

On 2016/09/13 11:25:37, nisse-webrtc wrote:
> On 2016/09/13 10:30:02, hbos wrote:
> > Please take a look nisse, kjellander and hta. This being instead of
> > https://codereview.webrtc.org/2335553006/.
> 
> I take it the files moved into stats/ are internal only?
> 
> Which header files define the public interface to stats? There's
> api/statstypes.h, and api/statscollector.h. The latter include some stats/
> headers, to define RTCStatsReport (argument type to the StatsCollector
> callback). 

There are "old" and "new" stats collection types and classes. The new will
replace the old but that is a long way down the road so we have both.
statstypes.h and statscollecton.h are old.
The new stats are: rtcstats.[h/cc], rtcstats_objects.[h/cc],
rtcstatsreport.[h/cc] and rtcstatscollector.[h/cc].

The headers of these make sense to put in api/. But then they can't be in
rtc_stats target because a source from a different directory
("../api/rtcstats.h") is not allowed.
If I put them in libjingle_peerconnection I would get a circular dependency or
rely on a different target's headers without relying on that target. Since these
are just headers I technically don't need them in any build target, but that
seems like bad practice.
- I could create a source set target rtc_stats_headers in webrtc/api/ that
rtc_stats depends on. How about that? (You wouldn't be able to resolve linkage
of rtc_stats_headers without rtc_stats.)

> If possible, I think it would be more correct to leave the definitions of all
> types referenced by the public API in the api/ folder. Production targets in
> stats/ should only be libraries (no executables), so they don't need any
> link-time dependency to peerconnection, right? Unit tests need to be linked
> including a lot more.

Hmm, I guess.

> Hmm, quoting the doc you pointed to earlier:
>  
>   The current idea is to move all supported headers into webrtc/api. Only
header
> files (and GYP,
>   GN and BUILD) should live in webrtc/api and the files in there may not
depend
> any files outside
>   of webrtc/api.
> 
> To follow this guideline, you should leave headers declaring public types in
> api/, but the implementation files should be moved to stats/. Files under
stats/
> may include headers from api/, but not the other way around. Don't know if
there
> are any other webrtc pieces that are organized that way, most things in api/
> seems to be one .h file and a corresponding .cc.

[hbos, 2016-09-13 12:24:29]

What if?

webrtc/api/stats/rtcstats.h
webrtc/api/stats/rtcstats_objects.h
webrtc/api/stats/rtcstatsreport.h
= rtc_stats_headers

webrtc/stats/rtcstats.cc
webrtc/stats/rtcstats_objects.cc
webrtc/stats/rtcstatsreport.cc
= rtc_stats
  deps: rtc_stats_headers

webrtc/api/stats/rtcstatscollector.cc
webrtc/api/stats/rtcstatscollector.h
= part of libjingle_peerconnection
  deps: rtc_stats

[nisse-webrtc, 2016-09-13 12:43:45]

On 2016/09/13 12:24:29, hbos wrote:
> What if?

I'd really like to hear kjellander's opinion. I'm guessing a lot on how these
things are supposed to work.
 
> webrtc/api/stats/rtcstats.h
> webrtc/api/stats/rtcstats_objects.h
> webrtc/api/stats/rtcstatsreport.h
> = rtc_stats_headers

If you need a separate target, yes. But I'm not sure the additional subdirectory
api/stats adds any value, I think I'd prefer to keep api/ flat (and small).
 
> webrtc/stats/rtcstats.cc
> webrtc/stats/rtcstats_objects.cc
> webrtc/stats/rtcstatsreport.cc
> = rtc_stats
>   deps: rtc_stats_headers

Seems good.
 
> webrtc/api/stats/rtcstatscollector.cc
> webrtc/api/stats/rtcstatscollector.h
> = part of libjingle_peerconnection
>   deps: rtc_stats

Not sure. For one, that's a .cc file under api/. 

Why is rtcstatscollector.c not part of rtc_stats, because of its dependency on
peerconnection? To me, I would make some sense to move it to rtc_stats, I think
linking still should work, all executables that need statscollector (including
unittests) will be linked with libjingle_peerconnection, which depends on
rtc_stats, and then linking shouldn't care which of those targets build the
object file rtcstatscollector.o.

[hbos, 2016-09-13 14:21:03]

Description was changed from

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

Files moved & dependency changes:
- Moving rtcstats.h, rtcstats_objects.h and rtcstatsreport.h to
  webrtc/stats/. These + .cc files make up the rtc_stats target, which
  no longer depend on libjingle_peerconnection.
- rtcstatscollector.[h/cc] is moved to webrtc/api/ and made part of the
  libjingle_peerconnection target. libjingle_peerconnection depends on
  rtc_stats but the circular dependency in the other direction is
  avoided. The RTCStatsCollector is viewed as an implementation detail
  of PeerConnection.
- DEPS is updated so that webrtc/api/ can include from webrtc/stats/.

Other changes:
- PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added
  for the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

to

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

[hbos, 2016-09-13 14:34:41]

Please take a look, kjellander.

On 2016/09/13 12:43:45, nisse-webrtc wrote:
> On 2016/09/13 12:24:29, hbos wrote:
> > webrtc/api/stats/rtcstats.h
> > webrtc/api/stats/rtcstats_objects.h
> > webrtc/api/stats/rtcstatsreport.h
> > = rtc_stats_headers
> 
> If you need a separate target, yes. But I'm not sure the additional
subdirectory
> api/stats adds any value, I think I'd prefer to keep api/ flat (and small).

Ah, I followed the pattern of webrtc/api/call/ and design doc and added
webrtc/api/stats/. If you still don't like this I'll happily move it to
webrtc/api/.

> > webrtc/api/stats/rtcstatscollector.cc
> > webrtc/api/stats/rtcstatscollector.h
> > = part of libjingle_peerconnection
> >   deps: rtc_stats
> 
> Not sure. For one, that's a .cc file under api/. 
> 
> Why is rtcstatscollector.c not part of rtc_stats, because of its dependency on
> peerconnection? To me, I would make some sense to move it to rtc_stats, I
think
> linking still should work, all executables that need statscollector (including
> unittests) will be linked with libjingle_peerconnection, which depends on
> rtc_stats, and then linking shouldn't care which of those targets build the
> object file rtcstatscollector.o.

I spoke with phoglund and I think we want to avoid that dependency even if it
would work as you describe. Let's see what kjellander thinks.

[nisse-webrtc, 2016-09-14 06:40:33]

On 2016/09/13 14:34:41, hbos wrote:

> Ah, I followed the pattern of webrtc/api/call/ and design doc and added
> webrtc/api/stats/. If you still don't like this I'll happily move it to
> webrtc/api/.

If that's what the design doc recommends, I'm fine with the extra directory.

[hbos, 2016-09-14 07:52:56]

Description was changed from

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

to

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

[hta-webrtc, 2016-09-14 10:20:58]

As I remember the google3 rules (gn is, if I understand it, a descendant of
Blaze), the rule was that header targets had headers only,
and that code-producing targets should depend on header targets, not vice versa.

Reading the BUILD.gn files, this placement of concerns looks reasonable to me.

[hbos, 2016-09-14 13:18:56]

Ping kjellander.

[kjellander_webrtc, 2016-09-14 13:55:27]

kjellander@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[kjellander_webrtc, 2016-09-14 13:55:28]

+solenberg

Does it pass gn if you add these targets to check_targets at
https://chromium.googlesource.com/external/webrtc/+/master/.gn#22 ?

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:312: suppressed_configs += [
"//build/config/clang:find_bad_constructs" ]
For new code, I won't tolerate violating these excellent warnings. Are they hard
to fix?

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:358: "rtcstatscollector_unittest.cc",
nit: sort alphabetically.

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

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:18: #include
"webrtc/api/stats/rtcstats_objects.h"
Shall we decide on a naming scheme here? Underscore delimiter or not between
words?

[hbos, 2016-09-14 18:48:03]

PTAL kjellander.

On 2016/09/14 13:55:28, kjellander_webrtc wrote:
> +solenberg
> 
> Does it pass gn if you add these targets to check_targets at
> https://chromium.googlesource.com/external/webrtc/+/master/.gn#22 ?

I added them to check_targets and it passes. I can't add
libjingle_peerconnection to it because it fails for other reasons (using base/
everywhere), but if I would it wouldn't fail for anything rtcstats-related
(except the base/ stuff which it is already failing for).

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:312: suppressed_configs += [
"//build/config/clang:find_bad_constructs" ]
On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> For new code, I won't tolerate violating these excellent warnings. Are they
hard
> to fix?

Removed. It's not necessary in the header target, but it is necessary in the
rtc_stats target (there since before this CL). Now that rtc_stats does not
include libjingle_peerconnection stuff this can be addressed. I should do that
in a follow-up CL though because it requires changing a macro used (not simple
enough to do in this CL).

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:358: "rtcstatscollector_unittest.cc",
On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> nit: sort alphabetically.

It is sorted.

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

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:18: #include
"webrtc/api/stats/rtcstats_objects.h"
On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> Shall we decide on a naming scheme here? Underscore delimiter or not between
> words?

Sure. I don't use underscores in names (e.g. RTCStatsReport = rtcstatsreport,
not rtc_stats_report). The reason for using underscore here is because the
header contains a set of RTCStats-derived classes (RTCPeerConnectionStats,
RTCCertificateStats, etc). Nothing is actually named RTCStatsObjects. If you
don't like the use of underscore for that I can rename it rtcstatsobjects.

[kjellander_webrtc, 2016-09-15 10:16:37]

Thanks for adding to check_targets. 
lgtm

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:312: suppressed_configs += [
"//build/config/clang:find_bad_constructs" ]
On 2016/09/14 18:48:03, hbos wrote:
> On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> > For new code, I won't tolerate violating these excellent warnings. Are they
> hard
> > to fix?
> 
> Removed. It's not necessary in the header target, but it is necessary in the
> rtc_stats target (there since before this CL). Now that rtc_stats does not
> include libjingle_peerconnection stuff this can be addressed. I should do that
> in a follow-up CL though because it requires changing a macro used (not simple
> enough to do in this CL).

Acknowledged.

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:358: "rtcstatscollector_unittest.cc",
On 2016/09/14 18:48:03, hbos wrote:
> On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> > nit: sort alphabetically.
> 
> It is sorted.

Sorry, I misread rtc/rtp :P

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

https://codereview.webrtc.org/2331373004/diff/40001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:18: #include
"webrtc/api/stats/rtcstats_objects.h"
On 2016/09/14 18:48:03, hbos wrote:
> On 2016/09/14 13:55:27, kjellander_webrtc wrote:
> > Shall we decide on a naming scheme here? Underscore delimiter or not between
> > words?
> 
> Sure. I don't use underscores in names (e.g. RTCStatsReport = rtcstatsreport,
> not rtc_stats_report). The reason for using underscore here is because the
> header contains a set of RTCStats-derived classes (RTCPeerConnectionStats,
> RTCCertificateStats, etc). Nothing is actually named RTCStatsObjects. If you
> don't like the use of underscore for that I can rename it rtcstatsobjects.

Oh well, I guess we don't have a solid convention so that reasoning sounds fine.

[hbos, 2016-09-15 10:55:11]

hbos@webrtc.org changed reviewers:
+ glaznev@webrtc.org

[hbos, 2016-09-15 10:55:12]

Please take a look glaznev (webrtc/api/ OWNER) and PTAL nisse.

[the sun, 2016-09-15 12:14:31]

On 2016/09/15 10:55:12, hbos wrote:
> Please take a look glaznev (webrtc/api/ OWNER) and PTAL nisse.

lgtm - thanks for doing this!

I had some issues at first with moving rtcstatscollector.cc into api/ but seeing
that it belongs to the PC API, it does make sense to first move it in here (so
at least we see clearly it is part of the public API). Eventually we'll move all
.cc for the PC api out from api/.

[nisse-webrtc, 2016-09-15 12:26:44]

lgtm

[hbos, 2016-09-15 13:28:54]

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

[commit-bot: I haz the power, 2016-09-15 13:28:59]

Dry run: 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-09-15 13:30:49]

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

[commit-bot: I haz the power, 2016-09-15 13:30:50]

Dry run: Try jobs failed on following builders:
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/17653)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/14367)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_gyp_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gyp_dbg/builds/638)
  mac_gyp_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gyp_rel/builds/642)

[hbos, 2016-09-15 13:36:57]

Please take a look, glaznev.

Note: I just added temporary files webrtc/api/rtcstats[report].h files that
include webrtc/api/stats/rtcstats[report].h. This is to not break Chromium which
has a couple of these includes (from rtc_stats.[cc/h]). As soon as this CL lands
I will update the inclusion in Chromium and delete these files.

[hbos, 2016-09-15 13:56:29]

On 2016/09/15 13:36:57, hbos wrote:
> Please take a look, glaznev.
> 
> Note: I just added temporary files webrtc/api/rtcstats[report].h files that
> include webrtc/api/stats/rtcstats[report].h. This is to not break Chromium
which
> has a couple of these includes (from rtc_stats.[cc/h]). As soon as this CL
lands
> I will update the inclusion in Chromium and delete these files.

For the same reason, I just added a default implementation of GetStats in
peerconnectioninterface.h that does nothing with TODO to change back to "= 0;".

[AlexG, 2016-09-15 17:17:45]

lgtm

[hbos, 2016-09-15 20:50:14]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2016-09-15 20:50:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, nisse@webrtc.org, kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2331373004/#ps180001 (title:
"(Corrected an incorrect inclusion)")

[commit-bot: I haz the power, 2016-09-15 20:50:24]

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-09-15 22:50:51]

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

[commit-bot: I haz the power, 2016-09-15 22:50:52]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[hbos, 2016-09-16 06:24:24]

The CQ bit was checked by hbos@webrtc.org

[commit-bot: I haz the power, 2016-09-16 06:24:30]

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-09-16 06:33:04]

Description was changed from

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

to

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

[commit-bot: I haz the power, 2016-09-16 06:33:05]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-09-16 06:33:11]

Description was changed from

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816
==========

to

==========
PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.

New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816

Committed: https://crrev.com/74e1a4f96a4915c9ad3b0ddeff87cc9797606453
Cr-Commit-Position: refs/heads/master@{#14246}
==========

[commit-bot: I haz the power, 2016-09-16 06:33:13]

Patchset 10 (id:??) landed as
https://crrev.com/74e1a4f96a4915c9ad3b0ddeff87cc9797606453
Cr-Commit-Position: refs/heads/master@{#14246}