diff --git a/cpp/core/internal/mediums/webrtc/connection_flow.cc b/cpp/core/internal/mediums/webrtc/connection_flow.cc index ff8582c9..d90dace1 100644 --- a/cpp/core/internal/mediums/webrtc/connection_flow.cc +++ b/cpp/core/internal/mediums/webrtc/connection_flow.cc @@ -32,6 +32,7 @@ namespace connections { namespace mediums { constexpr absl::Duration ConnectionFlow::kTimeout; +constexpr absl::Duration ConnectionFlow::kPeerConnectionTimeout; namespace { // This is the same as the nearby data channel name. @@ -136,6 +137,7 @@ SessionDescriptionWrapper ConnectionFlow::CreateOffer() { return std::move(result.result()); } + NEARBY_LOG(ERROR, "Failed to create offer: %d", result.exception()); return SessionDescriptionWrapper(); } @@ -159,6 +161,7 @@ SessionDescriptionWrapper ConnectionFlow::CreateAnswer() { return std::move(result.result()); } + NEARBY_LOG(ERROR, "Failed to create answer: %d", result.exception()); return SessionDescriptionWrapper(); } @@ -175,7 +178,12 @@ bool ConnectionFlow::SetLocalSessionDescription(SessionDescriptionWrapper sdp) { peer_connection_->SetLocalDescription(observer, sdp.Release()); ExceptionOr result = success_future->Get(kTimeout); - return result.ok() && result.result(); + bool success = result.ok() && result.result(); + if (!success) { + NEARBY_LOG(ERROR, "Failed to set local session description: %d", + result.exception()); + } + return success; } bool ConnectionFlow::SetRemoteSessionDescription( @@ -190,7 +198,12 @@ bool ConnectionFlow::SetRemoteSessionDescription( peer_connection_->SetRemoteDescription(observer, sdp.Release()); ExceptionOr result = success_future->Get(kTimeout); - return result.ok() && result.result(); + bool success = result.ok() && result.result(); + if (!success) { + NEARBY_LOG(ERROR, "Failed to set remote description: %d", + result.exception()); + } + return success; } bool ConnectionFlow::OnOfferReceived(SessionDescriptionWrapper offer) { @@ -267,8 +280,13 @@ bool ConnectionFlow::InitPeerConnection(WebRtcMedium& webrtc_medium) { success_future.Set(true); }); - ExceptionOr result = success_future.Get(kTimeout); - return result.ok() && result.result(); + ExceptionOr result = success_future.Get(kPeerConnectionTimeout); + bool success = result.ok() && result.result(); + if (!success) { + NEARBY_LOG(ERROR, "Failed to create peer connection: %d", + result.exception()); + } + return success; } void ConnectionFlow::OnSignalingStable() { diff --git a/cpp/core/internal/mediums/webrtc/connection_flow.h b/cpp/core/internal/mediums/webrtc/connection_flow.h index f524794a..438a9401 100644 --- a/cpp/core/internal/mediums/webrtc/connection_flow.h +++ b/cpp/core/internal/mediums/webrtc/connection_flow.h @@ -133,6 +133,8 @@ class ConnectionFlow { // TODO(bfranz): Consider whether this needs to be configurable per platform static constexpr absl::Duration kTimeout = absl::Milliseconds(250); + static constexpr absl::Duration kPeerConnectionTimeout = + absl::Milliseconds(2500); bool InitPeerConnection(WebRtcMedium& webrtc_medium); diff --git a/cpp/core/internal/mediums/webrtc/connection_flow_test.cc b/cpp/core/internal/mediums/webrtc/connection_flow_test.cc index c29d7577..96c9cca4 100644 --- a/cpp/core/internal/mediums/webrtc/connection_flow_test.cc +++ b/cpp/core/internal/mediums/webrtc/connection_flow_test.cc @@ -223,6 +223,22 @@ TEST_F(ConnectionFlowTest, NullPeerConnection) { EXPECT_EQ(answerer, nullptr); } +TEST_F(ConnectionFlowTest, PeerConnectionTimeout) { + MediumEnvironment::Instance().SetUseValidPeerConnection( + /*use_valid_peer_connection=*/true); + WebRtcMedium medium1; + std::unique_ptr flow1 = ConnectionFlow::Create( + LocalIceCandidateListener(), DataChannelListener(), medium1); + EXPECT_NE(flow1, nullptr); + + // Attempt to trigger the 2.5s peer connection timeout. + MediumEnvironment::Instance().SetPeerConnectionLatency(absl::Seconds(5)); + WebRtcMedium medium2; + std::unique_ptr flow2 = ConnectionFlow::Create( + LocalIceCandidateListener(), DataChannelListener(), medium2); + EXPECT_EQ(flow2, nullptr); +} + } // namespace } // namespace mediums } // namespace connections diff --git a/cpp/platform/base/medium_environment.cc b/cpp/platform/base/medium_environment.cc index 42733cd8..bdbee203 100644 --- a/cpp/platform/base/medium_environment.cc +++ b/cpp/platform/base/medium_environment.cc @@ -511,6 +511,15 @@ bool MediumEnvironment::GetUseValidPeerConnection() { return use_valid_peer_connection_; } +void MediumEnvironment::SetPeerConnectionLatency( + absl::Duration peer_connection_latency) { + peer_connection_latency_ = peer_connection_latency; +} + +absl::Duration MediumEnvironment::GetPeerConnectionLatency() { + return peer_connection_latency_; +} + void MediumEnvironment::RegisterWifiLanMedium(api::WifiLanMedium& medium) { if (!enabled_) return; RunOnMediumEnvironmentThread([this, &medium]() { diff --git a/cpp/platform/base/medium_environment.h b/cpp/platform/base/medium_environment.h index bd7126dc..15eb0bec 100644 --- a/cpp/platform/base/medium_environment.h +++ b/cpp/platform/base/medium_environment.h @@ -153,6 +153,11 @@ class MediumEnvironment { bool GetUseValidPeerConnection(); + // Used to set latency when creating the peer connection in tests. + void SetPeerConnectionLatency(absl::Duration peer_connection_latency); + + absl::Duration GetPeerConnectionLatency(); + // Adds medium-related info to allow for scanning/advertising to work. // This provides acccess to this medium from other mediums, when protocol // expects they should communicate. @@ -319,6 +324,7 @@ class MediumEnvironment { wifi_lan_mediums_; bool use_valid_peer_connection_ = true; + absl::Duration peer_connection_latency_ = absl::ZeroDuration(); }; } // namespace nearby diff --git a/cpp/platform/impl/g3/webrtc.cc b/cpp/platform/impl/g3/webrtc.cc index cd5f467c..c02cde71 100644 --- a/cpp/platform/impl/g3/webrtc.cc +++ b/cpp/platform/impl/g3/webrtc.cc @@ -48,6 +48,8 @@ void WebRtcSignalingMessenger::StopReceivingMessages() { env.UnregisterWebRtcSignalingMessenger(self_id_); } +WebRtcMedium::~WebRtcMedium() { single_thread_executor_.Shutdown(); } + const std::string WebRtcMedium::GetDefaultCountryCode() { return "US"; } @@ -72,9 +74,17 @@ void WebRtcMedium::CreatePeerConnection( webrtc::CreateDefaultTaskQueueFactory(); factory_dependencies.signaling_thread = signaling_thread.release(); - callback(webrtc::CreateModularPeerConnectionFactory( - std::move(factory_dependencies)) - ->CreatePeerConnection(rtc_config, std::move(dependencies))); + rtc::scoped_refptr peer_connection = + webrtc::CreateModularPeerConnectionFactory( + std::move(factory_dependencies)) + ->CreatePeerConnection(rtc_config, std::move(dependencies)); + + single_thread_executor_.Execute( + [&env, callback = std::move(callback), + peer_connection = std::move(peer_connection)]() { + absl::SleepFor(env.GetPeerConnectionLatency()); + callback(peer_connection); + }); } std::unique_ptr diff --git a/cpp/platform/impl/g3/webrtc.h b/cpp/platform/impl/g3/webrtc.h index 235fb649..14930f59 100644 --- a/cpp/platform/impl/g3/webrtc.h +++ b/cpp/platform/impl/g3/webrtc.h @@ -18,6 +18,7 @@ #include #include "platform/api/webrtc.h" +#include "platform/impl/g3/single_thread_executor.h" #include "absl/strings/string_view.h" #include "webrtc/api/peer_connection_interface.h" @@ -54,7 +55,7 @@ class WebRtcMedium : public api::WebRtcMedium { using PeerConnectionCallback = api::WebRtcMedium::PeerConnectionCallback; WebRtcMedium() = default; - ~WebRtcMedium() override = default; + ~WebRtcMedium() override; const std::string GetDefaultCountryCode() override; @@ -69,6 +70,8 @@ class WebRtcMedium : public api::WebRtcMedium { const connections::LocationHint& location_hint) override; private: + // Executor for handling calls to create a peer connection. + SingleThreadExecutor single_thread_executor_; }; } // namespace g3