[Stats] Add more rtc::Thread::ScopedDisallowBlockingCalls to getStats().

This ensures with DCHECK-crashes that we don't accidentally do more
blocking invokes than we think.

Remaining blocking invokes FYI:
- PrepareTransceiverStatsInfos_s_w() does 1 blocking invoke (regardless
  of the number of transceivers or channels) to the worker thread. This
  is because VoiceMediaChannel, VideoMediaChannel and GetParameters()
  execute on the worker thread, and the result of these operations are
  needed on the signalling thread.
- pc_->GetCallStats() does 1 blocking invoke to the worker thread.

These two blocking invokes can be merged, reducing the total number of
blocking invokes from 2 to 1, but this CL does not attempt to do that.
I filed https://crbug.com/webrtc/11767 for that.

Bug: webrtc:11716
Change-Id: Iebc2ab350d253fd037211cdd283825b4e5b2d446
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178867
Reviewed-by: Evan Shrubsole <eshr@google.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31670}

3 files changed, 83 insertions, 36 deletions

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 3def31a441..071867db9f 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -6557,6 +6557,7 @@ Call::Stats PeerConnection::GetCallStats() {
         RTC_FROM_HERE, rtc::Bind(&PeerConnection::GetCallStats, this));
   }
   RTC_DCHECK_RUN_ON(worker_thread());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
   if (call_) {
     return call_->GetStats();
   } else {
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index e1527d5a60..53b970c53d 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1040,7 +1040,7 @@ void RTCStatsCollector::GetStatsReportInternal(
     // Prepare |transceiver_stats_infos_| for use in
     // |ProducePartialResultsOnNetworkThread| and
     // |ProducePartialResultsOnSignalingThread|.
-    transceiver_stats_infos_ = PrepareTransceiverStatsInfos_s();
+    transceiver_stats_infos_ = PrepareTransceiverStatsInfos_s_w();
     // Prepare |transport_names_| for use in
     // |ProducePartialResultsOnNetworkThread|.
     transport_names_ = PrepareTransportNames_s();
@@ -1049,6 +1049,10 @@ void RTCStatsCollector::GetStatsReportInternal(
     // thread.
     // TODO(holmer): To avoid the hop we could move BWE and BWE stats to the
     // network thread, where it more naturally belongs.
+    // TODO(https://crbug.com/webrtc/11767): In the meantime we can piggyback on
+    // the blocking-invoke that is already performed in
+    // PrepareTransceiverStatsInfos_s_w() so that we can call GetCallStats()
+    // without additional blocking-invokes.
     call_stats_ = pc_->GetCallStats();
 
     // Don't touch |network_report_| on the signaling thread until
@@ -1078,6 +1082,8 @@ void RTCStatsCollector::WaitForPendingRequest() {
 void RTCStatsCollector::ProducePartialResultsOnSignalingThread(
     int64_t timestamp_us) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   partial_report_ = RTCStatsReport::Create(timestamp_us);
 
   ProducePartialResultsOnSignalingThreadImpl(timestamp_us,
@@ -1095,6 +1101,8 @@ void RTCStatsCollector::ProducePartialResultsOnSignalingThreadImpl(
     int64_t timestamp_us,
     RTCStatsReport* partial_report) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   ProduceDataChannelStats_s(timestamp_us, partial_report);
   ProduceMediaStreamStats_s(timestamp_us, partial_report);
   ProduceMediaStreamTrackStats_s(timestamp_us, partial_report);
@@ -1105,6 +1113,8 @@ void RTCStatsCollector::ProducePartialResultsOnSignalingThreadImpl(
 void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
     int64_t timestamp_us) {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   // Touching |network_report_| on this thread is safe by this method because
   // |network_report_event_| is reset before this method is invoked.
   network_report_ = RTCStatsReport::Create(timestamp_us);
@@ -1132,6 +1142,8 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThreadImpl(
     const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
     RTCStatsReport* partial_report) {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   ProduceCertificateStats_n(timestamp_us, transport_cert_stats, partial_report);
   ProduceCodecStats_n(timestamp_us, transceiver_stats_infos_, partial_report);
   ProduceIceCandidateAndPairStats_n(timestamp_us, transport_stats_by_name,
@@ -1218,6 +1230,8 @@ void RTCStatsCollector::ProduceCertificateStats_n(
     const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
     RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const auto& transport_cert_stats_pair : transport_cert_stats) {
     if (transport_cert_stats_pair.second.local) {
       ProduceCertificateStatsFromSSLCertificateStats(
@@ -1235,6 +1249,8 @@ void RTCStatsCollector::ProduceCodecStats_n(
     const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
     RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const auto& stats : transceiver_stats_infos) {
     if (!stats.mid) {
       continue;
@@ -1276,6 +1292,8 @@ void RTCStatsCollector::ProduceDataChannelStats_s(
     int64_t timestamp_us,
     RTCStatsReport* report) const {
   RTC_DCHECK_RUN_ON(signaling_thread_);
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   std::vector<DataChannel::Stats> data_stats = pc_->GetDataChannelStats();
   for (const auto& stats : data_stats) {
     std::unique_ptr<RTCDataChannelStats> data_channel_stats(
@@ -1301,6 +1319,8 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
     const Call::Stats& call_stats,
     RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const auto& entry : transport_stats_by_name) {
     const std::string& transport_name = entry.first;
     const cricket::TransportStats& transport_stats = entry.second;
@@ -1381,6 +1401,7 @@ void RTCStatsCollector::ProduceMediaStreamStats_s(
     int64_t timestamp_us,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
 
   std::map<std::string, std::vector<std::string>> track_ids;
 
@@ -1417,6 +1438,8 @@ void RTCStatsCollector::ProduceMediaStreamTrackStats_s(
     int64_t timestamp_us,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const RtpTransceiverStatsInfo& stats : transceiver_stats_infos_) {
     std::vector<rtc::scoped_refptr<RtpSenderInternal>> senders;
     for (const auto& sender : stats.transceiver->senders()) {
@@ -1438,6 +1461,8 @@ void RTCStatsCollector::ProduceMediaSourceStats_s(
     int64_t timestamp_us,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const RtpTransceiverStatsInfo& transceiver_stats_info :
        transceiver_stats_infos_) {
     const auto& track_media_info_map =
@@ -1519,6 +1544,8 @@ void RTCStatsCollector::ProducePeerConnectionStats_s(
     int64_t timestamp_us,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   std::unique_ptr<RTCPeerConnectionStats> stats(
       new RTCPeerConnectionStats("RTCPeerConnection", timestamp_us));
   stats->data_channels_opened = internal_record_.data_channels_opened;
@@ -1531,6 +1558,7 @@ void RTCStatsCollector::ProduceRTPStreamStats_n(
     const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
     RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
 
   for (const RtpTransceiverStatsInfo& stats : transceiver_stats_infos) {
     if (stats.media_type == cricket::MEDIA_TYPE_AUDIO) {
@@ -1547,6 +1575,9 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n(
     int64_t timestamp_us,
     const RtpTransceiverStatsInfo& stats,
     RTCStatsReport* report) const {
+  RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   if (!stats.mid || !stats.transport_name) {
     return;
   }
@@ -1625,6 +1656,9 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n(
     int64_t timestamp_us,
     const RtpTransceiverStatsInfo& stats,
     RTCStatsReport* report) const {
+  RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   if (!stats.mid || !stats.transport_name) {
     return;
   }
@@ -1705,6 +1739,8 @@ void RTCStatsCollector::ProduceTransportStats_n(
     const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
     RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   for (const auto& entry : transport_stats_by_name) {
     const std::string& transport_name = entry.first;
     const cricket::TransportStats& transport_stats = entry.second;
@@ -1796,6 +1832,8 @@ RTCStatsCollector::PrepareTransportCertificateStats_n(
     const std::map<std::string, cricket::TransportStats>&
         transport_stats_by_name) const {
   RTC_DCHECK(network_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   std::map<std::string, CertificateStatsPair> transport_cert_stats;
   for (const auto& entry : transport_stats_by_name) {
     const std::string& transport_name = entry.first;
@@ -1820,9 +1858,10 @@ RTCStatsCollector::PrepareTransportCertificateStats_n(
 }
 
 std::vector<RTCStatsCollector::RtpTransceiverStatsInfo>
-RTCStatsCollector::PrepareTransceiverStatsInfos_s() const {
-  std::vector<RtpTransceiverStatsInfo> transceiver_stats_infos;
+RTCStatsCollector::PrepareTransceiverStatsInfos_s_w() const {
+  RTC_DCHECK(signaling_thread_->IsCurrent());
 
+  std::vector<RtpTransceiverStatsInfo> transceiver_stats_infos;
   // These are used to invoke GetStats for all the media channels together in
   // one worker thread hop.
   std::map<cricket::VoiceMediaChannel*,
@@ -1832,39 +1871,43 @@ RTCStatsCollector::PrepareTransceiverStatsInfos_s() const {
            std::unique_ptr<cricket::VideoMediaInfo>>
       video_stats;
 
-  for (const auto& transceiver : pc_->GetTransceiversInternal()) {
-    cricket::MediaType media_type = transceiver->media_type();
-
-    // Prepare stats entry. The TrackMediaInfoMap will be filled in after the
-    // stats have been fetched on the worker thread.
-    transceiver_stats_infos.emplace_back();
-    RtpTransceiverStatsInfo& stats = transceiver_stats_infos.back();
-    stats.transceiver = transceiver->internal();
-    stats.media_type = media_type;
-
-    cricket::ChannelInterface* channel = transceiver->internal()->channel();
-    if (!channel) {
-      // The remaining fields require a BaseChannel.
-      continue;
-    }
+  {
+    rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
 
-    stats.mid = channel->content_name();
-    stats.transport_name = channel->transport_name();
-
-    if (media_type == cricket::MEDIA_TYPE_AUDIO) {
-      auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
-      RTC_DCHECK(voice_stats.find(voice_channel->media_channel()) ==
-                 voice_stats.end());
-      voice_stats[voice_channel->media_channel()] =
-          std::make_unique<cricket::VoiceMediaInfo>();
-    } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
-      auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
-      RTC_DCHECK(video_stats.find(video_channel->media_channel()) ==
-                 video_stats.end());
-      video_stats[video_channel->media_channel()] =
-          std::make_unique<cricket::VideoMediaInfo>();
-    } else {
-      RTC_NOTREACHED();
+    for (const auto& transceiver : pc_->GetTransceiversInternal()) {
+      cricket::MediaType media_type = transceiver->media_type();
+
+      // Prepare stats entry. The TrackMediaInfoMap will be filled in after the
+      // stats have been fetched on the worker thread.
+      transceiver_stats_infos.emplace_back();
+      RtpTransceiverStatsInfo& stats = transceiver_stats_infos.back();
+      stats.transceiver = transceiver->internal();
+      stats.media_type = media_type;
+
+      cricket::ChannelInterface* channel = transceiver->internal()->channel();
+      if (!channel) {
+        // The remaining fields require a BaseChannel.
+        continue;
+      }
+
+      stats.mid = channel->content_name();
+      stats.transport_name = channel->transport_name();
+
+      if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+        auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
+        RTC_DCHECK(voice_stats.find(voice_channel->media_channel()) ==
+                   voice_stats.end());
+        voice_stats[voice_channel->media_channel()] =
+            std::make_unique<cricket::VoiceMediaInfo>();
+      } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
+        auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
+        RTC_DCHECK(video_stats.find(video_channel->media_channel()) ==
+                   video_stats.end());
+        video_stats[video_channel->media_channel()] =
+            std::make_unique<cricket::VideoMediaInfo>();
+      } else {
+        RTC_NOTREACHED();
+      }
     }
   }
 
@@ -1924,6 +1967,9 @@ RTCStatsCollector::PrepareTransceiverStatsInfos_s() const {
 }
 
 std::set<std::string> RTCStatsCollector::PrepareTransportNames_s() const {
+  RTC_DCHECK(signaling_thread_->IsCurrent());
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
   std::set<std::string> transport_names;
   for (const auto& transceiver : pc_->GetTransceiversInternal()) {
     if (transceiver->internal()->channel()) {
diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h
index cd5ec21041..24b9ef21cf 100644
--- a/pc/rtc_stats_collector.h
+++ b/pc/rtc_stats_collector.h
@@ -215,7 +215,7 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
   PrepareTransportCertificateStats_n(
       const std::map<std::string, cricket::TransportStats>&
           transport_stats_by_name) const;
-  std::vector<RtpTransceiverStatsInfo> PrepareTransceiverStatsInfos_s() const;
+  std::vector<RtpTransceiverStatsInfo> PrepareTransceiverStatsInfos_s_w() const;
   std::set<std::string> PrepareTransportNames_s() const;
 
   // Stats gathering on a particular thread.