From 7a5fa6cd6173adbe32aedc1aedc872478121f5ed Mon Sep 17 00:00:00 2001 From: deadbeef Date: Sat, 24 Dec 2016 00:47:59 -0800 Subject: [PATCH] Adding error output param to SetConfiguration, using new RTCError type. Most notably, will return "INVALID_MODIFICATION" if a field in the configuration was modified and modification of that field isn't supported. Also changing RTCError to a class that wraps an enum type, because it will eventually need to hold other information (like SDP line number), to match the RTCError that was recently added to the spec: https://github.com/w3c/webrtc-pc/pull/850 BUG=webrtc:6916 Review-Url: https://codereview.webrtc.org/2587133004 Cr-Commit-Position: refs/heads/master@{#15777} --- webrtc/api/peerconnection.cc | 251 +++++++++++++----- webrtc/api/peerconnection.h | 24 +- webrtc/api/peerconnection_unittest.cc | 47 ++-- webrtc/api/peerconnectioninterface.h | 56 +++- .../api/peerconnectioninterface_unittest.cc | 201 +++++++++++--- webrtc/api/peerconnectionproxy.h | 5 +- webrtc/media/base/mediachannel.h | 14 + 7 files changed, 465 insertions(+), 133 deletions(-) diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index cdf5a97b68..ca646651ef 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -48,6 +48,8 @@ using webrtc::DataChannel; using webrtc::MediaConstraintsInterface; using webrtc::MediaStreamInterface; using webrtc::PeerConnectionInterface; +using webrtc::RTCError; +using webrtc::RTCErrorType; using webrtc::RtpSenderInternal; using webrtc::RtpSenderInterface; using webrtc::RtpSenderProxy; @@ -207,10 +209,11 @@ bool ParseHostnameAndPortFromString(const std::string& in_str, // Adds a STUN or TURN server to the appropriate list, // by parsing |url| and using the username/password in |server|. -bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, - const std::string& url, - cricket::ServerAddresses* stun_servers, - std::vector* turn_servers) { +RTCErrorType ParseIceServerUrl( + const PeerConnectionInterface::IceServer& server, + const std::string& url, + cricket::ServerAddresses* stun_servers, + std::vector* turn_servers) { // draft-nandakumar-rtcweb-stun-uri-01 // stunURI = scheme ":" stun-host [ ":" stun-port ] // scheme = "stun" / "stuns" @@ -238,14 +241,14 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, rtc::tokenize_with_empty_tokens(uri_transport_param, '=', &tokens); if (tokens[0] != kTransport) { LOG(LS_WARNING) << "Invalid transport parameter key."; - return false; + return RTCErrorType::SYNTAX_ERROR; } if (tokens.size() < 2 || !cricket::StringToProto(tokens[1].c_str(), &turn_transport_type) || (turn_transport_type != cricket::PROTO_UDP && turn_transport_type != cricket::PROTO_TCP)) { LOG(LS_WARNING) << "Transport param should always be udp or tcp."; - return false; + return RTCErrorType::SYNTAX_ERROR; } } @@ -255,7 +258,7 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, &service_type, &hoststring)) { LOG(LS_WARNING) << "Invalid transport parameter in ICE URI: " << url; - return false; + return RTCErrorType::SYNTAX_ERROR; } // GetServiceTypeAndHostnameFromUri should never give an empty hoststring @@ -268,12 +271,12 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, std::string username(server.username); if (tokens.size() > kTurnHostTokensNum) { LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring; - return false; + return RTCErrorType::SYNTAX_ERROR; } if (tokens.size() == kTurnHostTokensNum) { if (tokens[0].empty() || tokens[1].empty()) { LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring; - return false; + return RTCErrorType::SYNTAX_ERROR; } username.assign(rtc::s_url_decode(tokens[0])); hoststring = tokens[1]; @@ -290,12 +293,12 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, std::string address; if (!ParseHostnameAndPortFromString(hoststring, &address, &port)) { LOG(WARNING) << "Invalid hostname format: " << uri_without_transport; - return false; + return RTCErrorType::SYNTAX_ERROR; } if (port <= 0 || port > 0xffff) { LOG(WARNING) << "Invalid port: " << port; - return false; + return RTCErrorType::SYNTAX_ERROR; } switch (service_type) { @@ -305,16 +308,22 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server, break; case TURN: case TURNS: { + if (username.empty() || server.password.empty()) { + // The WebRTC spec requires throwing an InvalidAccessError when username + // or credential are ommitted; this is the native equivalent. + return RTCErrorType::INVALID_PARAMETER; + } turn_servers->push_back(cricket::RelayServerConfig( address, port, username, server.password, turn_transport_type)); break; } - case INVALID: default: - LOG(WARNING) << "Configuration not supported: " << url; - return false; + // We shouldn't get to this point with an invalid service_type, we should + // have returned an error already. + RTC_DCHECK(false) << "Unexpected service type"; + return RTCErrorType::INTERNAL_ERROR; } - return true; + return RTCErrorType::NONE; } // Check if we can send |new_stream| on a PeerConnection. @@ -426,11 +435,19 @@ void SetChannelOnSendersAndReceivers(CHANNEL* channel, } } +// Helper to set an error and return from a method. +bool SafeSetError(webrtc::RTCErrorType type, webrtc::RTCError* error) { + if (error) { + error->set_type(type); + } + return type == webrtc::RTCErrorType::NONE; +} + } // namespace namespace webrtc { -static const char* const kRtcErrorNames[] = { +static const char* const kRTCErrorTypeNames[] = { "NONE", "UNSUPPORTED_PARAMETER", "INVALID_PARAMETER", @@ -441,12 +458,82 @@ static const char* const kRtcErrorNames[] = { "NETWORK_ERROR", "INTERNAL_ERROR", }; +static_assert(static_cast(RTCErrorType::INTERNAL_ERROR) == + (arraysize(kRTCErrorTypeNames) - 1), + "kRTCErrorTypeNames must have as many strings as RTCErrorType " + "has values."); -std::ostream& operator<<(std::ostream& stream, RtcError error) { +std::ostream& operator<<(std::ostream& stream, RTCErrorType error) { int index = static_cast(error); - RTC_CHECK(index < static_cast(sizeof(kRtcErrorNames) / - sizeof(kRtcErrorNames[0]))); - return stream << kRtcErrorNames[index]; + return stream << kRTCErrorTypeNames[index]; +} + +bool PeerConnectionInterface::RTCConfiguration::operator==( + const PeerConnectionInterface::RTCConfiguration& o) const { + // This static_assert prevents us from accidentally breaking operator==. + struct stuff_being_tested_for_equality { + IceTransportsType type; + IceServers servers; + BundlePolicy bundle_policy; + RtcpMuxPolicy rtcp_mux_policy; + TcpCandidatePolicy tcp_candidate_policy; + CandidateNetworkPolicy candidate_network_policy; + int audio_jitter_buffer_max_packets; + bool audio_jitter_buffer_fast_accelerate; + int ice_connection_receiving_timeout; + int ice_backup_candidate_pair_ping_interval; + ContinualGatheringPolicy continual_gathering_policy; + std::vector> certificates; + bool prioritize_most_likely_ice_candidate_pairs; + struct cricket::MediaConfig media_config; + bool disable_ipv6; + bool enable_rtp_data_channel; + bool enable_quic; + rtc::Optional screencast_min_bitrate; + rtc::Optional combined_audio_video_bwe; + rtc::Optional enable_dtls_srtp; + int ice_candidate_pool_size; + bool prune_turn_ports; + bool presume_writable_when_fully_relayed; + bool enable_ice_renomination; + bool redetermine_role_on_ice_restart; + }; + static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this), + "Did you add something to RTCConfiguration and forget to " + "update operator==?"); + return type == o.type && servers == o.servers && + bundle_policy == o.bundle_policy && + rtcp_mux_policy == o.rtcp_mux_policy && + tcp_candidate_policy == o.tcp_candidate_policy && + candidate_network_policy == o.candidate_network_policy && + audio_jitter_buffer_max_packets == o.audio_jitter_buffer_max_packets && + audio_jitter_buffer_fast_accelerate == + o.audio_jitter_buffer_fast_accelerate && + ice_connection_receiving_timeout == + o.ice_connection_receiving_timeout && + ice_backup_candidate_pair_ping_interval == + o.ice_backup_candidate_pair_ping_interval && + continual_gathering_policy == o.continual_gathering_policy && + certificates == o.certificates && + prioritize_most_likely_ice_candidate_pairs == + o.prioritize_most_likely_ice_candidate_pairs && + media_config == o.media_config && disable_ipv6 == o.disable_ipv6 && + enable_rtp_data_channel == o.enable_rtp_data_channel && + enable_quic == o.enable_quic && + screencast_min_bitrate == o.screencast_min_bitrate && + combined_audio_video_bwe == o.combined_audio_video_bwe && + enable_dtls_srtp == o.enable_dtls_srtp && + ice_candidate_pool_size == o.ice_candidate_pool_size && + prune_turn_ports == o.prune_turn_ports && + presume_writable_when_fully_relayed == + o.presume_writable_when_fully_relayed && + enable_ice_renomination == o.enable_ice_renomination && + redetermine_role_on_ice_restart == o.redetermine_role_on_ice_restart; +} + +bool PeerConnectionInterface::RTCConfiguration::operator!=( + const PeerConnectionInterface::RTCConfiguration& o) const { + return !(*this == o); } // Generate a RTCP CNAME when a PeerConnection is created. @@ -548,28 +635,33 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints, return mandatory_constraints_satisfied == constraints->GetMandatory().size(); } -bool ParseIceServers(const PeerConnectionInterface::IceServers& servers, - cricket::ServerAddresses* stun_servers, - std::vector* turn_servers) { +RTCErrorType ParseIceServers( + const PeerConnectionInterface::IceServers& servers, + cricket::ServerAddresses* stun_servers, + std::vector* turn_servers) { for (const webrtc::PeerConnectionInterface::IceServer& server : servers) { if (!server.urls.empty()) { for (const std::string& url : server.urls) { if (url.empty()) { LOG(LS_ERROR) << "Empty uri."; - return false; + return RTCErrorType::SYNTAX_ERROR; } - if (!ParseIceServerUrl(server, url, stun_servers, turn_servers)) { - return false; + RTCErrorType err = + ParseIceServerUrl(server, url, stun_servers, turn_servers); + if (err != RTCErrorType::NONE) { + return err; } } } else if (!server.uri.empty()) { // Fallback to old .uri if new .urls isn't present. - if (!ParseIceServerUrl(server, server.uri, stun_servers, turn_servers)) { - return false; + RTCErrorType err = + ParseIceServerUrl(server, server.uri, stun_servers, turn_servers); + if (err != RTCErrorType::NONE) { + return err; } } else { LOG(LS_ERROR) << "Empty uri."; - return false; + return RTCErrorType::SYNTAX_ERROR; } } // Candidates must have unique priorities, so that connectivity checks @@ -579,7 +671,7 @@ bool ParseIceServers(const PeerConnectionInterface::IceServers& servers, // First in the list gets highest priority. turn_server.priority = priority--; } - return true; + return RTCErrorType::NONE; } PeerConnection::PeerConnection(PeerConnectionFactory* factory) @@ -626,12 +718,18 @@ bool PeerConnection::Initialize( std::unique_ptr cert_generator, PeerConnectionObserver* observer) { TRACE_EVENT0("webrtc", "PeerConnection::Initialize"); - RTC_DCHECK(observer != nullptr); + if (!allocator) { + LOG(LS_ERROR) << "PeerConnection initialized without a PortAllocator? " + << "This shouldn't happen if using PeerConnectionFactory."; + return false; + } if (!observer) { + // TODO(deadbeef): Why do we do this? + LOG(LS_ERROR) << "PeerConnection initialized without a " + << "PeerConnectionObserver"; return false; } observer_ = observer; - port_allocator_ = std::move(allocator); // The port allocator lives on the network thread and should be initialized @@ -1287,7 +1385,8 @@ PeerConnectionInterface::RTCConfiguration PeerConnection::GetConfiguration() { return configuration_; } -bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) { +bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, + RTCError* error) { TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration"); if (session_->local_description() && @@ -1295,32 +1394,61 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) { configuration_.ice_candidate_pool_size) { LOG(LS_ERROR) << "Can't change candidate pool size after calling " "SetLocalDescription."; - return false; - } - // TODO(deadbeef): Return false and log an error if there are any unsupported - // modifications. - if (port_allocator_) { - if (!network_thread()->Invoke( - RTC_FROM_HERE, - rtc::Bind(&PeerConnection::ReconfigurePortAllocator_n, this, - configuration))) { - LOG(LS_ERROR) << "Failed to apply configuration to PortAllocator."; - return false; - } + return SafeSetError(RTCErrorType::INVALID_MODIFICATION, error); } - // TODO(deadbeef): Shouldn't have to hop to the network thread twice... - session_->SetIceConfig(session_->ParseIceConfig(configuration)); + // The simplest (and most future-compatible) way to tell if the config was + // modified in an invalid way is to copy each property we do support + // modifying, then use operator==. There are far more properties we don't + // support modifying than those we do, and more could be added. + RTCConfiguration modified_config = configuration_; + modified_config.servers = configuration.servers; + modified_config.type = configuration.type; + modified_config.ice_candidate_pool_size = + configuration.ice_candidate_pool_size; + modified_config.prune_turn_ports = configuration.prune_turn_ports; + if (configuration != modified_config) { + LOG(LS_ERROR) << "Modifying the configuration in an unsupported way."; + return SafeSetError(RTCErrorType::INVALID_MODIFICATION, error); + } + + // Note that this isn't possible through chromium, since it's an unsigned + // short in WebIDL. + if (configuration.ice_candidate_pool_size < 0 || + configuration.ice_candidate_pool_size > UINT16_MAX) { + return SafeSetError(RTCErrorType::INVALID_RANGE, error); + } + + // Parse ICE servers before hopping to network thread. + cricket::ServerAddresses stun_servers; + std::vector turn_servers; + RTCErrorType parse_error = + ParseIceServers(configuration.servers, &stun_servers, &turn_servers); + if (parse_error != RTCErrorType::NONE) { + return SafeSetError(parse_error, error); + } + + // In theory this shouldn't fail. + if (!network_thread()->Invoke( + RTC_FROM_HERE, + rtc::Bind(&PeerConnection::ReconfigurePortAllocator_n, this, + stun_servers, turn_servers, modified_config.type, + modified_config.ice_candidate_pool_size, + modified_config.prune_turn_ports))) { + LOG(LS_ERROR) << "Failed to apply configuration to PortAllocator."; + return SafeSetError(RTCErrorType::INTERNAL_ERROR, error); + } // As described in JSEP, calling setConfiguration with new ICE servers or // candidate policy must set a "needs-ice-restart" bit so that the next offer // triggers an ICE restart which will pick up the changes. - if (configuration.servers != configuration_.servers || - configuration.type != configuration_.type) { + if (modified_config.servers != configuration_.servers || + modified_config.type != configuration_.type || + modified_config.prune_turn_ports != configuration_.prune_turn_ports) { session_->SetNeedsIceRestartFlag(); } - configuration_ = configuration; - return true; + configuration_ = modified_config; + return SafeSetError(RTCErrorType::NONE, error); } bool PeerConnection::AddIceCandidate( @@ -1347,7 +1475,7 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { } // Send information about IPv4/IPv6 status. - if (uma_observer_ && port_allocator_) { + if (uma_observer_) { port_allocator_->SetMetricsObserver(uma_observer_); if (port_allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6) { uma_observer_->IncrementEnumCounter( @@ -2360,7 +2488,8 @@ bool PeerConnection::InitializePortAllocator_n( const RTCConfiguration& configuration) { cricket::ServerAddresses stun_servers; std::vector turn_servers; - if (!ParseIceServers(configuration.servers, &stun_servers, &turn_servers)) { + if (ParseIceServers(configuration.servers, &stun_servers, &turn_servers) != + RTCErrorType::NONE) { return false; } @@ -2406,19 +2535,17 @@ bool PeerConnection::InitializePortAllocator_n( } bool PeerConnection::ReconfigurePortAllocator_n( - const RTCConfiguration& configuration) { - cricket::ServerAddresses stun_servers; - std::vector turn_servers; - if (!ParseIceServers(configuration.servers, &stun_servers, &turn_servers)) { - return false; - } + const cricket::ServerAddresses& stun_servers, + const std::vector& turn_servers, + IceTransportsType type, + int candidate_pool_size, + bool prune_turn_ports) { port_allocator_->set_candidate_filter( - ConvertIceTransportTypeToCandidateFilter(configuration.type)); + ConvertIceTransportTypeToCandidateFilter(type)); // Call this last since it may create pooled allocator sessions using the // candidate filter set above. return port_allocator_->SetConfiguration( - stun_servers, turn_servers, configuration.ice_candidate_pool_size, - configuration.prune_turn_ports); + stun_servers, turn_servers, candidate_pool_size, prune_turn_ports); } bool PeerConnection::StartRtcEventLog_w(rtc::PlatformFile file, diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index 5269e3ae49..51c2ea048d 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -54,10 +54,12 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints, cricket::MediaSessionOptions* session_options); // Parses the URLs for each server in |servers| to build |stun_servers| and -// |turn_servers|. -bool ParseIceServers(const PeerConnectionInterface::IceServers& servers, - cricket::ServerAddresses* stun_servers, - std::vector* turn_servers); +// |turn_servers|. Can return SYNTAX_ERROR if the URL is malformed, or +// INVALID_PARAMETER if a TURN server is missing |username| or |password|. +RTCErrorType ParseIceServers( + const PeerConnectionInterface::IceServers& servers, + cricket::ServerAddresses* stun_servers, + std::vector* turn_servers); // PeerConnection implements the PeerConnectionInterface interface. // It uses WebRtcSession to implement the PeerConnection functionality. @@ -137,7 +139,8 @@ class PeerConnection : public PeerConnectionInterface, SessionDescriptionInterface* desc) override; PeerConnectionInterface::RTCConfiguration GetConfiguration() override; bool SetConfiguration( - const PeerConnectionInterface::RTCConfiguration& configuration) override; + const PeerConnectionInterface::RTCConfiguration& configuration, + RTCError* error = nullptr) override; bool AddIceCandidate(const IceCandidateInterface* candidate) override; bool RemoveIceCandidates( const std::vector& candidates) override; @@ -373,9 +376,14 @@ class PeerConnection : public PeerConnectionInterface, // Called when first configuring the port allocator. bool InitializePortAllocator_n(const RTCConfiguration& configuration); - // Called when SetConfiguration is called. Only a subset of the configuration - // is applied. - bool ReconfigurePortAllocator_n(const RTCConfiguration& configuration); + // Called when SetConfiguration is called to apply the supported subset + // of the configuration on the network thread. + bool ReconfigurePortAllocator_n( + const cricket::ServerAddresses& stun_servers, + const std::vector& turn_servers, + IceTransportsType type, + int candidate_pool_size, + bool prune_turn_ports); // Starts recording an Rtc EventLog using the supplied platform file. // This function should only be called from the worker thread. diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc index 4f34cfa83e..c8dc12103c 100644 --- a/webrtc/api/peerconnection_unittest.cc +++ b/webrtc/api/peerconnection_unittest.cc @@ -2620,6 +2620,10 @@ class IceServerParsingTest : public testing::Test { return ParseUrl(url, std::string(), std::string()); } + bool ParseTurnUrl(const std::string& url) { + return ParseUrl(url, "username", "password"); + } + bool ParseUrl(const std::string& url, const std::string& username, const std::string& password) { @@ -2629,7 +2633,8 @@ class IceServerParsingTest : public testing::Test { server.username = username; server.password = password; servers.push_back(server); - return webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_); + return webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_) == + webrtc::RTCErrorType::NONE; } protected: @@ -2649,13 +2654,13 @@ TEST_F(IceServerParsingTest, ParseStunPrefixes) { EXPECT_EQ(0U, turn_servers_.size()); stun_servers_.clear(); - EXPECT_TRUE(ParseUrl("turn:hostname")); + EXPECT_TRUE(ParseTurnUrl("turn:hostname")); EXPECT_EQ(0U, stun_servers_.size()); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(cricket::PROTO_UDP, turn_servers_[0].ports[0].proto); turn_servers_.clear(); - EXPECT_TRUE(ParseUrl("turns:hostname")); + EXPECT_TRUE(ParseTurnUrl("turns:hostname")); EXPECT_EQ(0U, stun_servers_.size()); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(cricket::PROTO_TLS, turn_servers_[0].ports[0].proto); @@ -2670,14 +2675,14 @@ TEST_F(IceServerParsingTest, ParseStunPrefixes) { TEST_F(IceServerParsingTest, VerifyDefaults) { // TURNS defaults - EXPECT_TRUE(ParseUrl("turns:hostname")); + EXPECT_TRUE(ParseTurnUrl("turns:hostname")); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(5349, turn_servers_[0].ports[0].address.port()); EXPECT_EQ(cricket::PROTO_TLS, turn_servers_[0].ports[0].proto); turn_servers_.clear(); // TURN defaults - EXPECT_TRUE(ParseUrl("turn:hostname")); + EXPECT_TRUE(ParseTurnUrl("turn:hostname")); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(3478, turn_servers_[0].ports[0].address.port()); EXPECT_EQ(cricket::PROTO_UDP, turn_servers_[0].ports[0].proto); @@ -2742,33 +2747,33 @@ TEST_F(IceServerParsingTest, ParseHostnameAndPort) { // Test parsing the "?transport=xxx" part of the URL. TEST_F(IceServerParsingTest, ParseTransport) { - EXPECT_TRUE(ParseUrl("turn:hostname:1234?transport=tcp")); + EXPECT_TRUE(ParseTurnUrl("turn:hostname:1234?transport=tcp")); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(cricket::PROTO_TCP, turn_servers_[0].ports[0].proto); turn_servers_.clear(); - EXPECT_TRUE(ParseUrl("turn:hostname?transport=udp")); + EXPECT_TRUE(ParseTurnUrl("turn:hostname?transport=udp")); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(cricket::PROTO_UDP, turn_servers_[0].ports[0].proto); turn_servers_.clear(); - EXPECT_FALSE(ParseUrl("turn:hostname?transport=invalid")); - EXPECT_FALSE(ParseUrl("turn:hostname?transport=")); - EXPECT_FALSE(ParseUrl("turn:hostname?=")); - EXPECT_FALSE(ParseUrl("?")); + EXPECT_FALSE(ParseTurnUrl("turn:hostname?transport=invalid")); + EXPECT_FALSE(ParseTurnUrl("turn:hostname?transport=")); + EXPECT_FALSE(ParseTurnUrl("turn:hostname?=")); + EXPECT_FALSE(ParseTurnUrl("?")); } // Test parsing ICE username contained in URL. TEST_F(IceServerParsingTest, ParseUsername) { - EXPECT_TRUE(ParseUrl("turn:user@hostname")); + EXPECT_TRUE(ParseTurnUrl("turn:user@hostname")); EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ("user", turn_servers_[0].credentials.username); turn_servers_.clear(); - EXPECT_FALSE(ParseUrl("turn:@hostname")); - EXPECT_FALSE(ParseUrl("turn:username@")); - EXPECT_FALSE(ParseUrl("turn:@")); - EXPECT_FALSE(ParseUrl("turn:user@name@hostname")); + EXPECT_FALSE(ParseTurnUrl("turn:@hostname")); + EXPECT_FALSE(ParseTurnUrl("turn:username@")); + EXPECT_FALSE(ParseTurnUrl("turn:@")); + EXPECT_FALSE(ParseTurnUrl("turn:user@name@hostname")); } // Test that username and password from IceServer is copied into the resulting @@ -2786,8 +2791,11 @@ TEST_F(IceServerParsingTest, ParseMultipleUrls) { PeerConnectionInterface::IceServer server; server.urls.push_back("stun:hostname"); server.urls.push_back("turn:hostname"); + server.username = "foo"; + server.password = "bar"; servers.push_back(server); - EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); + EXPECT_EQ(webrtc::RTCErrorType::NONE, + webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); EXPECT_EQ(1U, stun_servers_.size()); EXPECT_EQ(1U, turn_servers_.size()); } @@ -2799,8 +2807,11 @@ TEST_F(IceServerParsingTest, TurnServerPrioritiesUnique) { PeerConnectionInterface::IceServer server; server.urls.push_back("turn:hostname"); server.urls.push_back("turn:hostname2"); + server.username = "foo"; + server.password = "bar"; servers.push_back(server); - EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); + EXPECT_EQ(webrtc::RTCErrorType::NONE, + webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); EXPECT_EQ(2U, turn_servers_.size()); EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority); } diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index e81eee2fa0..1fac5c73cf 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -150,9 +150,10 @@ class MetricsObserverInterface : public rtc::RefCountInterface { typedef MetricsObserverInterface UMAObserver; // Enumeration to represent distinct classes of errors that an application -// may wish to act upon differently. These roughly map to DOMExceptions in -// the web API, as described in the comments below. -enum class RtcError { +// may wish to act upon differently. These roughly map to DOMExceptions or +// RTCError "errorDetailEnum" values in the web API, as described in the +// comments below. +enum class RTCErrorType { // No error. NONE, // A supplied parameter is valid, but currently unsupported. @@ -183,9 +184,26 @@ enum class RtcError { INTERNAL_ERROR, }; +// Roughly corresponds to RTCError in the web api. Holds an error type and +// possibly additional information specific to that error. +// +// Doesn't contain anything beyond a type now, but will in the future as more +// errors are implemented. +class RTCError { + public: + RTCError() : type_(RTCErrorType::NONE) {} + explicit RTCError(RTCErrorType type) : type_(type) {} + + RTCErrorType type() const { return type_; } + void set_type(RTCErrorType type) { type_ = type; } + + private: + RTCErrorType type_; +}; + // Outputs the error as a friendly string. // Update this method when adding a new error type. -std::ostream& operator<<(std::ostream& stream, RtcError error); +std::ostream& operator<<(std::ostream& stream, RTCErrorType error); class PeerConnectionInterface : public rtc::RefCountInterface { public: @@ -306,6 +324,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { } } + bool operator==(const RTCConfiguration& o) const; + bool operator!=(const RTCConfiguration& o) const; + bool dscp() { return media_config.enable_dscp; } void set_dscp(bool enable) { media_config.enable_dscp = enable; } @@ -379,6 +400,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // If true, ICE role is redetermined when peerconnection sets a local // transport description that indicates an ICE restart. bool redetermine_role_on_ice_restart = true; + // + // Don't forget to update operator== if adding something. + // }; struct RTCOfferAnswerOptions { @@ -556,15 +580,33 @@ class PeerConnectionInterface : public rtc::RefCountInterface { virtual PeerConnectionInterface::RTCConfiguration GetConfiguration() { return PeerConnectionInterface::RTCConfiguration(); } + // Sets the PeerConnection's global configuration to |config|. + // + // The members of |config| that may be changed are |type|, |servers|, + // |ice_candidate_pool_size| and |prune_turn_ports| (though the candidate + // pool size can't be changed after the first call to SetLocalDescription). + // Note that this means the BUNDLE and RTCP-multiplexing policies cannot be + // changed with this method. + // // Any changes to STUN/TURN servers or ICE candidate policy will affect the // next gathering phase, and cause the next call to createOffer to generate - // new ICE credentials. Note that the BUNDLE and RTCP-multiplexing policies - // cannot be changed with this method. + // new ICE credentials, as described in JSEP. This also occurs when + // |prune_turn_ports| changes, for the same reasoning. + // + // If an error occurs, returns false and populates |error| if non-null: + // - INVALID_MODIFICATION if |config| contains a modified parameter other + // than one of the parameters listed above. + // - INVALID_RANGE if |ice_candidate_pool_size| is out of range. + // - SYNTAX_ERROR if parsing an ICE server URL failed. + // - INVALID_PARAMETER if a TURN server is missing |username| or |password|. + // - INTERNAL_ERROR if an unexpected error occurred. + // // TODO(deadbeef): Make this pure virtual once all Chrome subclasses of // PeerConnectionInterface implement it. virtual bool SetConfiguration( - const PeerConnectionInterface::RTCConfiguration& config) { + const PeerConnectionInterface::RTCConfiguration& config, + RTCError* error = nullptr) { return false; } // Provides a remote candidate to the ICE Agent. diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index c52df41f2b..8094bba5c9 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -321,6 +321,8 @@ using webrtc::NotifierInterface; using webrtc::ObserverInterface; using webrtc::PeerConnectionInterface; using webrtc::PeerConnectionObserver; +using webrtc::RTCError; +using webrtc::RTCErrorType; using webrtc::RtpReceiverInterface; using webrtc::RtpSenderInterface; using webrtc::SdpParseError; @@ -694,6 +696,17 @@ class PeerConnectionInterfaceTest : public testing::Test { CreatePeerConnection(PeerConnectionInterface::RTCConfiguration(), nullptr); } + // DTLS does not work in a loopback call, so is disabled for most of the + // tests in this file. + void CreatePeerConnectionWithoutDtls() { + FakeConstraints no_dtls_constraints; + no_dtls_constraints.AddMandatory( + webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, false); + + CreatePeerConnection(PeerConnectionInterface::RTCConfiguration(), + &no_dtls_constraints); + } + void CreatePeerConnection(webrtc::MediaConstraintsInterface* constraints) { CreatePeerConnection(PeerConnectionInterface::RTCConfiguration(), constraints); @@ -722,17 +735,6 @@ class PeerConnectionInterfaceTest : public testing::Test { new cricket::FakePortAllocator(rtc::Thread::Current(), nullptr)); port_allocator_ = port_allocator.get(); - // DTLS does not work in a loopback call, so is disabled for most of the - // tests in this file. We only create a FakeIdentityService if the test - // explicitly sets the constraint. - FakeConstraints default_constraints; - if (!constraints) { - constraints = &default_constraints; - - default_constraints.AddMandatory( - webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, false); - } - std::unique_ptr cert_generator; bool dtls; if (FindConstraint(constraints, @@ -898,7 +900,7 @@ class PeerConnectionInterfaceTest : public testing::Test { } void InitiateCall() { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create a local stream with audio&video tracks. AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label"); CreateOfferReceiveAnswer(); @@ -1105,7 +1107,7 @@ class PeerConnectionInterfaceTest : public testing::Test { } std::unique_ptr CreateOfferWithOneAudioStream() { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVoiceStream(kStreamLabel1); std::unique_ptr offer; EXPECT_TRUE(DoCreateOffer(&offer, nullptr)); @@ -1281,7 +1283,7 @@ TEST_F(PeerConnectionInterfaceTest, GetConfigurationAfterSetConfiguration) { } TEST_F(PeerConnectionInterfaceTest, AddStreams) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVideoStream(kStreamLabel1); AddVoiceStream(kStreamLabel2); ASSERT_EQ(2u, pc_->local_streams()->count()); @@ -1311,7 +1313,7 @@ TEST_F(PeerConnectionInterfaceTest, AddStreams) { // Test that the created offer includes streams we added. TEST_F(PeerConnectionInterfaceTest, AddedStreamsPresentInOffer) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddAudioVideoStream(kStreamLabel1, "audio_track", "video_track"); std::unique_ptr offer; ASSERT_TRUE(DoCreateOffer(&offer, nullptr)); @@ -1355,7 +1357,7 @@ TEST_F(PeerConnectionInterfaceTest, AddedStreamsPresentInOffer) { } TEST_F(PeerConnectionInterfaceTest, RemoveStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVideoStream(kStreamLabel1); ASSERT_EQ(1u, pc_->local_streams()->count()); pc_->RemoveStream(pc_->local_streams()->at(0)); @@ -1367,7 +1369,7 @@ TEST_F(PeerConnectionInterfaceTest, RemoveStream) { // and that the RtpSenders are created correctly. // Also tests that RemoveTrack removes the tracks from subsequent offers. TEST_F(PeerConnectionInterfaceTest, AddTrackRemoveTrack) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create a dummy stream, so tracks share a stream label. rtc::scoped_refptr stream( pc_factory_->CreateLocalMediaStream(kStreamLabel1)); @@ -1442,7 +1444,7 @@ TEST_F(PeerConnectionInterfaceTest, AddTrackRemoveTrack) { // Test creating senders without a stream specified, // expecting a random stream ID to be generated. TEST_F(PeerConnectionInterfaceTest, AddTrackWithoutStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create a dummy stream, so tracks share a stream label. rtc::scoped_refptr audio_track( pc_factory_->CreateAudioTrack("audio_track", nullptr)); @@ -1470,7 +1472,7 @@ TEST_F(PeerConnectionInterfaceTest, CreateOfferReceiveAnswer) { } TEST_F(PeerConnectionInterfaceTest, CreateOfferReceivePrAnswerAndAnswer) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVideoStream(kStreamLabel1); CreateOfferAsLocalDescription(); std::string offer; @@ -1480,7 +1482,7 @@ TEST_F(PeerConnectionInterfaceTest, CreateOfferReceivePrAnswerAndAnswer) { } TEST_F(PeerConnectionInterfaceTest, ReceiveOfferCreateAnswer) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVideoStream(kStreamLabel1); CreateOfferAsRemoteDescription(); @@ -1490,7 +1492,7 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveOfferCreateAnswer) { } TEST_F(PeerConnectionInterfaceTest, ReceiveOfferCreatePrAnswerAndAnswer) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVideoStream(kStreamLabel1); CreateOfferAsRemoteDescription(); @@ -1513,7 +1515,7 @@ TEST_F(PeerConnectionInterfaceTest, Renegotiate) { // Tests that after negotiating an audio only call, the respondent can perform a // renegotiation that removes the audio stream. TEST_F(PeerConnectionInterfaceTest, RenegotiateAudioOnly) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVoiceStream(kStreamLabel1); CreateOfferAsRemoteDescription(); CreateAnswerAsLocalDescription(); @@ -1526,7 +1528,7 @@ TEST_F(PeerConnectionInterfaceTest, RenegotiateAudioOnly) { // Test that candidates are generated and that we can parse our own candidates. TEST_F(PeerConnectionInterfaceTest, IceCandidates) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); EXPECT_FALSE(pc_->AddIceCandidate(observer_.last_candidate_.get())); // SetRemoteDescription takes ownership of offer. @@ -1549,7 +1551,7 @@ TEST_F(PeerConnectionInterfaceTest, IceCandidates) { // Test that CreateOffer and CreateAnswer will fail if the track labels are // not unique. TEST_F(PeerConnectionInterfaceTest, CreateOfferAnswerWithInvalidStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create a regular offer for the CreateAnswer test later. std::unique_ptr offer; EXPECT_TRUE(DoCreateOffer(&offer, nullptr)); @@ -1570,7 +1572,7 @@ TEST_F(PeerConnectionInterfaceTest, CreateOfferAnswerWithInvalidStream) { // Test that we will get different SSRCs for each tracks in the offer and answer // we created. TEST_F(PeerConnectionInterfaceTest, SsrcInOfferAnswer) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create a local stream with audio&video tracks having different labels. AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label"); @@ -1602,7 +1604,7 @@ TEST_F(PeerConnectionInterfaceTest, SsrcInOfferAnswer) { // the stream to a PeerConnection. // TODO(deadbeef): Remove this test once this behavior is no longer supported. TEST_F(PeerConnectionInterfaceTest, AddTrackAfterAddStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create audio stream and add to PeerConnection. AddVoiceStream(kStreamLabel1); MediaStreamInterface* stream = pc_->local_streams()->at(0); @@ -1626,7 +1628,7 @@ TEST_F(PeerConnectionInterfaceTest, AddTrackAfterAddStream) { // the stream to a PeerConnection. // TODO(deadbeef): Remove this test once this behavior is no longer supported. TEST_F(PeerConnectionInterfaceTest, RemoveTrackAfterAddStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); // Create audio/video stream and add to PeerConnection. AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label"); MediaStreamInterface* stream = pc_->local_streams()->at(0); @@ -1645,7 +1647,7 @@ TEST_F(PeerConnectionInterfaceTest, RemoveTrackAfterAddStream) { // Test creating a sender with a stream ID, and ensure the ID is populated // in the offer. TEST_F(PeerConnectionInterfaceTest, CreateSenderWithStream) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); pc_->CreateSender("video", kStreamLabel1); std::unique_ptr offer; @@ -2075,7 +2077,7 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveFireFoxOffer) { // limited set of audio codecs and receive an updated offer with more audio // codecs, where the added codecs are not supported. TEST_F(PeerConnectionInterfaceTest, ReceiveUpdatedAudioOfferWithBadCodecs) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddVoiceStream("audio_label"); CreateOfferAsLocalDescription(); @@ -2183,6 +2185,17 @@ TEST_F(PeerConnectionInterfaceTest, SetConfigurationChangesCandidateFilter) { EXPECT_EQ(cricket::CF_RELAY, port_allocator_->candidate_filter()); } +TEST_F(PeerConnectionInterfaceTest, SetConfigurationChangesPruneTurnPortsFlag) { + PeerConnectionInterface::RTCConfiguration config; + config.prune_turn_ports = false; + CreatePeerConnection(config, nullptr); + EXPECT_FALSE(port_allocator_->prune_turn_ports()); + + config.prune_turn_ports = true; + EXPECT_TRUE(pc_->SetConfiguration(config)); + EXPECT_TRUE(port_allocator_->prune_turn_ports()); +} + // Test that when SetConfiguration changes both the pool size and other // attributes, the pooled session is created with the updated attributes. TEST_F(PeerConnectionInterfaceTest, @@ -2203,7 +2216,8 @@ TEST_F(PeerConnectionInterfaceTest, EXPECT_EQ(1UL, session->stun_servers().size()); } -// Test that after SetLocalDescription, changing the pool size is not allowed. +// Test that after SetLocalDescription, changing the pool size is not allowed, +// and an invalid modification error is returned. TEST_F(PeerConnectionInterfaceTest, CantChangePoolSizeAfterSetLocalDescription) { CreatePeerConnection(); @@ -2220,7 +2234,91 @@ TEST_F(PeerConnectionInterfaceTest, // Set local answer; now it's too late. CreateAnswerAsLocalDescription(); config.ice_candidate_pool_size = 3; - EXPECT_FALSE(pc_->SetConfiguration(config)); + RTCError error; + EXPECT_FALSE(pc_->SetConfiguration(config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, error.type()); +} + +// Test that SetConfiguration returns an invalid modification error if +// modifying a field in the configuration that isn't allowed to be modified. +TEST_F(PeerConnectionInterfaceTest, + SetConfigurationReturnsInvalidModificationError) { + PeerConnectionInterface::RTCConfiguration config; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyBalanced; + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; + config.continual_gathering_policy = PeerConnectionInterface::GATHER_ONCE; + CreatePeerConnection(config, nullptr); + + PeerConnectionInterface::RTCConfiguration modified_config = config; + modified_config.bundle_policy = + PeerConnectionInterface::kBundlePolicyMaxBundle; + RTCError error; + EXPECT_FALSE(pc_->SetConfiguration(modified_config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, error.type()); + + modified_config = config; + modified_config.rtcp_mux_policy = + PeerConnectionInterface::kRtcpMuxPolicyRequire; + error.set_type(RTCErrorType::NONE); + EXPECT_FALSE(pc_->SetConfiguration(modified_config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, error.type()); + + modified_config = config; + modified_config.continual_gathering_policy = + PeerConnectionInterface::GATHER_CONTINUALLY; + error.set_type(RTCErrorType::NONE); + EXPECT_FALSE(pc_->SetConfiguration(modified_config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, error.type()); +} + +// Test that SetConfiguration returns a range error if the candidate pool size +// is negative or larger than allowed by the spec. +TEST_F(PeerConnectionInterfaceTest, + SetConfigurationReturnsRangeErrorForBadCandidatePoolSize) { + PeerConnectionInterface::RTCConfiguration config; + CreatePeerConnection(config, nullptr); + + config.ice_candidate_pool_size = -1; + RTCError error; + EXPECT_FALSE(pc_->SetConfiguration(config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_RANGE, error.type()); + + config.ice_candidate_pool_size = INT_MAX; + error.set_type(RTCErrorType::NONE); + EXPECT_FALSE(pc_->SetConfiguration(config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_RANGE, error.type()); +} + +// Test that SetConfiguration returns a syntax error if parsing an ICE server +// URL failed. +TEST_F(PeerConnectionInterfaceTest, + SetConfigurationReturnsSyntaxErrorFromBadIceUrls) { + PeerConnectionInterface::RTCConfiguration config; + CreatePeerConnection(config, nullptr); + + PeerConnectionInterface::IceServer bad_server; + bad_server.uri = "stunn:www.example.com"; + config.servers.push_back(bad_server); + RTCError error; + EXPECT_FALSE(pc_->SetConfiguration(config, &error)); + EXPECT_EQ(RTCErrorType::SYNTAX_ERROR, error.type()); +} + +// Test that SetConfiguration returns an invalid parameter error if a TURN +// IceServer is missing a username or password. +TEST_F(PeerConnectionInterfaceTest, + SetConfigurationReturnsInvalidParameterIfCredentialsMissing) { + PeerConnectionInterface::RTCConfiguration config; + CreatePeerConnection(config, nullptr); + + PeerConnectionInterface::IceServer bad_server; + bad_server.uri = "turn:www.example.com"; + // Missing password. + bad_server.username = "foo"; + config.servers.push_back(bad_server); + RTCError error; + EXPECT_FALSE(pc_->SetConfiguration(config, &error)); + EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.type()); } // Test that PeerConnection::Close changes the states to closed and all remote @@ -2255,7 +2353,7 @@ TEST_F(PeerConnectionInterfaceTest, CloseAndTestStreamsAndStates) { // Test that PeerConnection methods fails gracefully after // PeerConnection::Close has been called. TEST_F(PeerConnectionInterfaceTest, CloseAndTestMethods) { - CreatePeerConnection(); + CreatePeerConnectionWithoutDtls(); AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label"); CreateOfferAsRemoteDescription(); CreateAnswerAsLocalDescription(); @@ -3227,10 +3325,41 @@ TEST(CreateSessionOptionsTest, MediaConstraintsInAnswer) { EXPECT_TRUE(updated_answer_options.has_video()); } -TEST(RtcErrorTest, OstreamOperator) { +TEST(RTCErrorTypeTest, OstreamOperator) { std::ostringstream oss; - oss << webrtc::RtcError::NONE << ' ' - << webrtc::RtcError::INVALID_PARAMETER << ' ' - << webrtc::RtcError::INTERNAL_ERROR; + oss << webrtc::RTCErrorType::NONE << ' ' + << webrtc::RTCErrorType::INVALID_PARAMETER << ' ' + << webrtc::RTCErrorType::INTERNAL_ERROR; EXPECT_EQ("NONE INVALID_PARAMETER INTERNAL_ERROR", oss.str()); } + +// Tests a few random fields being different. +TEST(RTCConfigurationTest, ComparisonOperators) { + PeerConnectionInterface::RTCConfiguration a; + PeerConnectionInterface::RTCConfiguration b; + EXPECT_EQ(a, b); + + PeerConnectionInterface::RTCConfiguration c; + c.servers.push_back(PeerConnectionInterface::IceServer()); + EXPECT_NE(a, c); + + PeerConnectionInterface::RTCConfiguration d; + d.type = PeerConnectionInterface::kRelay; + EXPECT_NE(a, d); + + PeerConnectionInterface::RTCConfiguration e; + e.audio_jitter_buffer_max_packets = 5; + EXPECT_NE(a, e); + + PeerConnectionInterface::RTCConfiguration f; + f.ice_connection_receiving_timeout = 1337; + EXPECT_NE(a, f); + + PeerConnectionInterface::RTCConfiguration g; + g.disable_ipv6 = true; + EXPECT_NE(a, g); + + PeerConnectionInterface::RTCConfiguration h( + PeerConnectionInterface::RTCConfigurationType::kAggressive); + EXPECT_NE(a, h); +} diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h index 2110ee2a50..bc1cd5ebe6 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -72,9 +72,10 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) PROXY_METHOD2(void, SetRemoteDescription, SetSessionDescriptionObserver*, SessionDescriptionInterface*) PROXY_METHOD0(PeerConnectionInterface::RTCConfiguration, GetConfiguration); - PROXY_METHOD1(bool, + PROXY_METHOD2(bool, SetConfiguration, - const PeerConnectionInterface::RTCConfiguration&); + const PeerConnectionInterface::RTCConfiguration&, + RTCError*); PROXY_METHOD1(bool, AddIceCandidate, const IceCandidateInterface*) PROXY_METHOD1(bool, RemoveIceCandidates, diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index d66424014b..6d13e6198f 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -133,6 +133,20 @@ struct MediaConfig { // Enables periodic bandwidth probing in application-limited region. bool periodic_alr_bandwidth_probing = false; } video; + + bool operator==(const MediaConfig& o) const { + return enable_dscp == o.enable_dscp && + video.enable_cpu_overuse_detection == + o.video.enable_cpu_overuse_detection && + video.suspend_below_min_bitrate == + o.video.suspend_below_min_bitrate && + video.disable_prerenderer_smoothing == + o.video.disable_prerenderer_smoothing && + video.periodic_alr_bandwidth_probing == + o.video.periodic_alr_bandwidth_probing; + } + + bool operator!=(const MediaConfig& o) const { return !(*this == o); } }; // Options that can be applied to a VoiceMediaChannel or a VoiceMediaEngine.