Wire up BWE stats through WebrtcSession so that they are filled in both for audio and video calls.

webrtc/pc/rtcstatscollector.cc

Index: webrtc/pc/rtcstatscollector.cc
diff --git a/webrtc/pc/rtcstatscollector.cc b/webrtc/pc/rtcstatscollector.cc
index d9da0729eda2275986ded8535f334a5816c82d4b..d5c83249a47e2d62cc141aea0f2411a34886a3f8 100644
--- a/webrtc/pc/rtcstatscollector.cc
+++ b/webrtc/pc/rtcstatscollector.cc
@@ -696,6 +696,7 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
 
   std::unique_ptr<SessionStats> session_stats =
       pc_->session()->GetStats(*channel_name_pairs_);
+  Call::Stats call_stats = pc_->session()->GetCallStats();

[hbos, 2017/05/09 12:48:14]

Is this OK? Here we are on the worker thread, but GetCallStats might want call_
to be touched on the worker thread. See other comment.

Also (how) does this end up invoking FillBandwidthInfo? The
webrtc::Call::GetStats and FillBandwidthInfo seem to have separated in this CL.
Does this only use one of them now? Why?

[holmer, 2017/05/30 14:44:28]

Made sure the necessary thread hop is done.
It doesn't, and AFAICT it doesn't use the members (transmit_bitrate,
retransmit_bitrate, target_enc_bitrate and actual_enc_bitrate) filled in by
FillBitrateInfo().

I guess that is because those are only used to generate the webrtc-internals
page?

   if (session_stats) {
     std::map<std::string, CertificateStatsPair> transport_cert_stats =
         PrepareTransportCertificateStats_n(*session_stats);
@@ -704,9 +705,9 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
         timestamp_us, transport_cert_stats, report.get());
     ProduceCodecStats_n(
         timestamp_us, *track_media_info_map_, report.get());
-    ProduceIceCandidateAndPairStats_n(
-        timestamp_us, *session_stats, track_media_info_map_->video_media_info(),
-        report.get());
+    ProduceIceCandidateAndPairStats_n(timestamp_us, *session_stats,
+                                      track_media_info_map_->video_media_info(),
+                                      call_stats, report.get());
     ProduceRTPStreamStats_n(
         timestamp_us, *session_stats, *track_media_info_map_, report.get());
     ProduceTransportStats_n(
@@ -835,9 +836,11 @@ void RTCStatsCollector::ProduceDataChannelStats_s(
 }
 
 void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
-      int64_t timestamp_us, const SessionStats& session_stats,
-      const cricket::VideoMediaInfo* video_media_info,
-      RTCStatsReport* report) const {
+    int64_t timestamp_us,
+    const SessionStats& session_stats,
+    const cricket::VideoMediaInfo* video_media_info,
+    const Call::Stats& call_stats,
+    RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
   for (const auto& transport_stats : session_stats.transport_stats) {
     for (const auto& channel_stats : transport_stats.second.channel_stats) {
@@ -879,24 +882,18 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
               static_cast<double>(*info.current_round_trip_time_ms) /
               rtc::kNumMillisecsPerSec;
         }
-        if (info.best_connection && video_media_info &&
-            !video_media_info->bw_estimations.empty()) {
+        if (info.best_connection) {
           // The bandwidth estimations we have are for the selected candidate
           // pair ("info.best_connection").
-          RTC_DCHECK_EQ(video_media_info->bw_estimations.size(), 1);
-          RTC_DCHECK_GE(
-              video_media_info->bw_estimations[0].available_send_bandwidth, 0);
-          RTC_DCHECK_GE(
-              video_media_info->bw_estimations[0].available_recv_bandwidth, 0);
-          if (video_media_info->bw_estimations[0].available_send_bandwidth) {
+          RTC_DCHECK_GE(call_stats.send_bandwidth_bps, 0);
+          RTC_DCHECK_GE(call_stats.recv_bandwidth_bps, 0);
+          if (call_stats.send_bandwidth_bps > 0) {
             candidate_pair_stats->available_outgoing_bitrate =
-                static_cast<double>(video_media_info->bw_estimations[0]
-                                        .available_send_bandwidth);
+                static_cast<double>(call_stats.send_bandwidth_bps);
           }
-          if (video_media_info->bw_estimations[0].available_recv_bandwidth) {
+          if (call_stats.recv_bandwidth_bps > 0) {
             candidate_pair_stats->available_incoming_bitrate =
-                static_cast<double>(video_media_info->bw_estimations[0]
-                                        .available_recv_bandwidth);
+                static_cast<double>(call_stats.recv_bandwidth_bps);

[hbos, 2017/05/09 12:48:14]

It would be nice to turn these into rtc::Optional<uint32_t> instead of checking
if an int > 0 since 0 could theoretically be an estimate and negative values are
not allowed. available_outgoing/incoming_bitrate defaults to undefined, not 0,
so the distinction is meaningful. webrtc::Call::GetStats invokes
AvailableBandwidth which returns a boolean if it is able to set or not, so looks
simple to change. Can you change to optinal unless that is a lot of hassle?
Otherwise you can skip that in this CL.

As it happens AvailableBandwidth is implemented so that 0 means undefined so it
wouldn't be noticeable but it would be a step in the right direction if
AvailableBandwidth is ever updated.

[holmer, 2017/05/30 14:44:28]

I don't think it will be trivial since it will affect APIs used in other places.
Let's do it separately and try to make this CL as small as possible.

TBH, I don't know why we even do this check though. Why can't we report 0? I
can't see that there'd be much of a difference?

           }
         }
         candidate_pair_stats->requests_received =