|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Tommi <tommi@webrtc.org> |
| 3 | +Date: Thu, 1 Jun 2023 16:08:52 +0200 |
| 4 | +Subject: Move transceiver iteration loop over to the signaling thread. |
| 5 | + |
| 6 | +This is required for ReportTransportStats since iterating over the |
| 7 | +transceiver list from the network thread is not safe. |
| 8 | + |
| 9 | +(cherry picked from commit dba22d31909298161318e00d43a80cdb0abc940f) |
| 10 | + |
| 11 | +No-Try: true |
| 12 | +Bug: chromium:1446274, webrtc:12692 |
| 13 | +Change-Id: I7c514df9f029112c4b1da85826af91217850fb26 |
| 14 | +Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/307340 |
| 15 | +Reviewed-by: Harald Alvestrand <hta@webrtc.org> |
| 16 | +Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> |
| 17 | +Cr-Original-Commit-Position: refs/heads/main@{#40197} |
| 18 | +Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308001 |
| 19 | +Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> |
| 20 | +Cr-Commit-Position: refs/branch-heads/5735@{#3} |
| 21 | +Cr-Branched-From: df7df199abd619e75b9f1d9a7e12fc3f3f748775-refs/heads/main@{#39949} |
| 22 | + |
| 23 | +diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc |
| 24 | +index 4615ce5a2c413dba798c3f9f8f7d4c1ae78bf9af..62179cd44c57dc6d808579353b398412db5e1ed6 100644 |
| 25 | +--- a/pc/peer_connection.cc |
| 26 | ++++ b/pc/peer_connection.cc |
| 27 | +@@ -716,9 +716,6 @@ JsepTransportController* PeerConnection::InitializeTransportController_n( |
| 28 | + transport_controller_->SubscribeIceConnectionState( |
| 29 | + [this](cricket::IceConnectionState s) { |
| 30 | + RTC_DCHECK_RUN_ON(network_thread()); |
| 31 | +- if (s == cricket::kIceConnectionConnected) { |
| 32 | +- ReportTransportStats(); |
| 33 | +- } |
| 34 | + signaling_thread()->PostTask( |
| 35 | + SafeTask(signaling_thread_safety_.flag(), [this, s]() { |
| 36 | + RTC_DCHECK_RUN_ON(signaling_thread()); |
| 37 | +@@ -2372,6 +2369,20 @@ void PeerConnection::OnTransportControllerConnectionState( |
| 38 | + case cricket::kIceConnectionConnected: |
| 39 | + RTC_LOG(LS_INFO) << "Changing to ICE connected state because " |
| 40 | + "all transports are writable."; |
| 41 | ++ { |
| 42 | ++ std::vector<RtpTransceiverProxyRefPtr> transceivers; |
| 43 | ++ if (ConfiguredForMedia()) { |
| 44 | ++ transceivers = rtp_manager()->transceivers()->List(); |
| 45 | ++ } |
| 46 | ++ |
| 47 | ++ network_thread()->PostTask( |
| 48 | ++ SafeTask(network_thread_safety_, |
| 49 | ++ [this, transceivers = std::move(transceivers)] { |
| 50 | ++ RTC_DCHECK_RUN_ON(network_thread()); |
| 51 | ++ ReportTransportStats(std::move(transceivers)); |
| 52 | ++ })); |
| 53 | ++ } |
| 54 | ++ |
| 55 | + SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected); |
| 56 | + NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED); |
| 57 | + break; |
| 58 | +@@ -2701,20 +2712,18 @@ void PeerConnection::OnTransportControllerGatheringState( |
| 59 | + } |
| 60 | + |
| 61 | + // Runs on network_thread(). |
| 62 | +-void PeerConnection::ReportTransportStats() { |
| 63 | ++void PeerConnection::ReportTransportStats( |
| 64 | ++ std::vector<RtpTransceiverProxyRefPtr> transceivers) { |
| 65 | + TRACE_EVENT0("webrtc", "PeerConnection::ReportTransportStats"); |
| 66 | + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; |
| 67 | + std::map<std::string, std::set<cricket::MediaType>> |
| 68 | + media_types_by_transport_name; |
| 69 | +- if (ConfiguredForMedia()) { |
| 70 | +- for (const auto& transceiver : |
| 71 | +- rtp_manager()->transceivers()->UnsafeList()) { |
| 72 | +- if (transceiver->internal()->channel()) { |
| 73 | +- std::string transport_name( |
| 74 | +- transceiver->internal()->channel()->transport_name()); |
| 75 | +- media_types_by_transport_name[transport_name].insert( |
| 76 | +- transceiver->media_type()); |
| 77 | +- } |
| 78 | ++ for (const auto& transceiver : transceivers) { |
| 79 | ++ if (transceiver->internal()->channel()) { |
| 80 | ++ std::string transport_name( |
| 81 | ++ transceiver->internal()->channel()->transport_name()); |
| 82 | ++ media_types_by_transport_name[transport_name].insert( |
| 83 | ++ transceiver->media_type()); |
| 84 | + } |
| 85 | + } |
| 86 | + |
| 87 | +diff --git a/pc/peer_connection.h b/pc/peer_connection.h |
| 88 | +index 36a9d2ac743a81e2c4ac8b4abd73d2fd40c3dd40..ea221e697322bd8b8c161edada7a448a68fa7f68 100644 |
| 89 | +--- a/pc/peer_connection.h |
| 90 | ++++ b/pc/peer_connection.h |
| 91 | +@@ -563,7 +563,8 @@ class PeerConnection : public PeerConnectionInternal, |
| 92 | + |
| 93 | + // Invoked when TransportController connection completion is signaled. |
| 94 | + // Reports stats for all transports in use. |
| 95 | +- void ReportTransportStats() RTC_RUN_ON(network_thread()); |
| 96 | ++ void ReportTransportStats(std::vector<RtpTransceiverProxyRefPtr> transceivers) |
| 97 | ++ RTC_RUN_ON(network_thread()); |
| 98 | + |
| 99 | + // Gather the usage of IPv4/IPv6 as best connection. |
| 100 | + static void ReportBestConnectionState(const cricket::TransportStats& stats); |
| 101 | +diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc |
| 102 | +index 19cc6ce3cfc8b8b38d45f5df9c28fe6dd01572f5..da8a53ef5d91c9c14dc6f42a10d176c7f6089ada 100644 |
| 103 | +--- a/pc/peer_connection_integrationtest.cc |
| 104 | ++++ b/pc/peer_connection_integrationtest.cc |
| 105 | +@@ -1831,6 +1831,10 @@ TEST_P(PeerConnectionIntegrationTest, |
| 106 | + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected, |
| 107 | + callee()->ice_connection_state(), kDefaultTimeout); |
| 108 | + |
| 109 | ++ // Part of reporting the stats will occur on the network thread, so flush it |
| 110 | ++ // before checking NumEvents. |
| 111 | ++ SendTask(network_thread(), [] {}); |
| 112 | ++ |
| 113 | + EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents( |
| 114 | + "WebRTC.PeerConnection.CandidatePairType_UDP", |
| 115 | + webrtc::kIceCandidatePairHostNameHostName)); |
| 116 | +@@ -1959,6 +1963,10 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, MAYBE_VerifyBestConnection) { |
| 117 | + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected, |
| 118 | + callee()->ice_connection_state(), kDefaultTimeout); |
| 119 | + |
| 120 | ++ // Part of reporting the stats will occur on the network thread, so flush it |
| 121 | ++ // before checking NumEvents. |
| 122 | ++ SendTask(network_thread(), [] {}); |
| 123 | ++ |
| 124 | + // TODO(bugs.webrtc.org/9456): Fix it. |
| 125 | + const int num_best_ipv4 = webrtc::metrics::NumEvents( |
| 126 | + "WebRTC.PeerConnection.IPMetrics", webrtc::kBestConnections_IPv4); |
0 commit comments