commit 6de92f9255b8746ad1be920fa508c19cb71dea63
author deadbeef <deadbeef@webrtc.org> Mon Dec 12 18:49:32 2016 -0800
committer Commit bot <commit-bot@chromium.org> Tue Dec 13 02:49:40 2016 +0000
tree 68e86fd9681b11a1e08fd9cb091a721ab0552666
parent 25ed435afee393ef94cb0cd357ff594407b0453d

Don't allow changing ICE pool size after SetLocalDescription.

This was the decision at IETF 97
(see: https://github.com/rtcweb-wg/jsep/issues/381). It's simpler to not
allow this (since there's no real need for it) rather than try to decide
complex rules for it.

BUG=webrtc:6864

Review-Url: https://codereview.webrtc.org/2566833002
Cr-Commit-Position: refs/heads/master@{#15559}

webrtc/api/peerconnection.cc

diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 692ca2e..05a940e 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -1288,6 +1288,14 @@
 
 bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) {
   TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration");
+
+  if (session_->local_description() &&
+      configuration.ice_candidate_pool_size !=
+          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_) {
@@ -1295,6 +1303,7 @@
             RTC_FROM_HERE,
             rtc::Bind(&PeerConnection::ReconfigurePortAllocator_n, this,
                       configuration))) {
+      LOG(LS_ERROR) << "Failed to apply configuration to PortAllocator.";
       return false;
     }
   }
@@ -2386,10 +2395,9 @@
       ConvertIceTransportTypeToCandidateFilter(configuration.type));
   // Call this last since it may create pooled allocator sessions using the
   // candidate filter set above.
-  port_allocator_->SetConfiguration(stun_servers, turn_servers,
-                                    configuration.ice_candidate_pool_size,
-                                    configuration.prune_turn_ports);
-  return true;
+  return port_allocator_->SetConfiguration(
+      stun_servers, turn_servers, configuration.ice_candidate_pool_size,
+      configuration.prune_turn_ports);
 }
 
 bool PeerConnection::StartRtcEventLog_w(rtc::PlatformFile file,

webrtc/api/peerconnectioninterface_unittest.cc

diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 1fb3a5f..7b33c0c 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -2203,6 +2203,26 @@
   EXPECT_EQ(1UL, session->stun_servers().size());
 }
 
+// Test that after SetLocalDescription, changing the pool size is not allowed.
+TEST_F(PeerConnectionInterfaceTest,
+       CantChangePoolSizeAfterSetLocalDescription) {
+  CreatePeerConnection();
+  // Start by setting a size of 1.
+  PeerConnectionInterface::RTCConfiguration config;
+  config.ice_candidate_pool_size = 1;
+  EXPECT_TRUE(pc_->SetConfiguration(config));
+
+  // Set remote offer; can still change pool size at this point.
+  CreateOfferAsRemoteDescription();
+  config.ice_candidate_pool_size = 2;
+  EXPECT_TRUE(pc_->SetConfiguration(config));
+
+  // Set local answer; now it's too late.
+  CreateAnswerAsLocalDescription();
+  config.ice_candidate_pool_size = 3;
+  EXPECT_FALSE(pc_->SetConfiguration(config));
+}
+
 // Test that PeerConnection::Close changes the states to closed and all remote
 // tracks change state to ended.
 TEST_F(PeerConnectionInterfaceTest, CloseAndTestStreamsAndStates) {