commit 001fd2d5037cca62b717619827cde675ee35f470
author jiayl@webrtc.org <jiayl@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d> Thu May 29 15:31:11 2014 +0000
committer jiayl@webrtc.org <jiayl@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d> Thu May 29 15:31:11 2014 +0000
tree c690a5365be9fc794bfc563a2431f580520bda57
parent 9aa7d8df95535aa681a637ffe56ffc7c7757868b

Fire OnRenegotiationNeeded only for the first SCTP DataChannel.
Subsequent DataChannels do not need renegotiation since SCTP data streams are not negotiated through SDP.

BUG=2431
R=pthatcher@google.com, wu@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/12629004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6268 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/app/webrtc/peerconnection.cc

diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 37b2a3b..0839977 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -482,6 +482,8 @@
 PeerConnection::CreateDataChannel(
     const std::string& label,
     const DataChannelInit* config) {
+  bool first_datachannel = !mediastream_signaling_->HasDataChannels();
+
   talk_base::scoped_ptr<InternalDataChannelInit> internal_config;
   if (config) {
     internal_config.reset(new InternalDataChannelInit(*config));
@@ -491,7 +493,11 @@
   if (!channel.get())
     return NULL;
 
-  observer_->OnRenegotiationNeeded();
+  // Trigger the onRenegotiationNeeded event for every new RTP DataChannel, or
+  // the first SCTP DataChannel.
+  if (session_->data_channel_type() == cricket::DCT_RTP || first_datachannel) {
+    observer_->OnRenegotiationNeeded();
+  }
 
   return DataChannelProxy::Create(signaling_thread(), channel.get());
 }

talk/app/webrtc/peerconnectioninterface_unittest.cc

diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc
index a605b0d..5c9b826 100644
--- a/talk/app/webrtc/peerconnectioninterface_unittest.cc
+++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc
@@ -946,23 +946,28 @@
       pc_->CreateDataChannel("1", &config);
   EXPECT_TRUE(channel != NULL);
   EXPECT_TRUE(channel->reliable());
+  EXPECT_TRUE(observer_.renegotiation_needed_);
+  observer_.renegotiation_needed_ = false;
 
   config.ordered = false;
   channel = pc_->CreateDataChannel("2", &config);
   EXPECT_TRUE(channel != NULL);
   EXPECT_TRUE(channel->reliable());
+  EXPECT_FALSE(observer_.renegotiation_needed_);
 
   config.ordered = true;
   config.maxRetransmits = 0;
   channel = pc_->CreateDataChannel("3", &config);
   EXPECT_TRUE(channel != NULL);
   EXPECT_FALSE(channel->reliable());
+  EXPECT_FALSE(observer_.renegotiation_needed_);
 
   config.maxRetransmits = -1;
   config.maxRetransmitTime = 0;
   channel = pc_->CreateDataChannel("4", &config);
   EXPECT_TRUE(channel != NULL);
   EXPECT_FALSE(channel->reliable());
+  EXPECT_FALSE(observer_.renegotiation_needed_);
 }
 
 // This tests that no data channel is returned if both maxRetransmits and
@@ -1012,6 +1017,23 @@
   EXPECT_TRUE(channel == NULL);
 }
 
+// This test verifies that OnRenegotiationNeeded is fired for every new RTP
+// DataChannel.
+TEST_F(PeerConnectionInterfaceTest, RenegotiationNeededForNewRtpDataChannel) {
+  FakeConstraints constraints;
+  constraints.SetAllowRtpDataChannels();
+  CreatePeerConnection(&constraints);
+
+  scoped_refptr<DataChannelInterface> dc1  =
+      pc_->CreateDataChannel("test1", NULL);
+  EXPECT_TRUE(observer_.renegotiation_needed_);
+  observer_.renegotiation_needed_ = false;
+
+  scoped_refptr<DataChannelInterface> dc2  =
+      pc_->CreateDataChannel("test2", NULL);
+  EXPECT_TRUE(observer_.renegotiation_needed_);
+}
+
 // This test that a data channel closes when a PeerConnection is deleted/closed.
 TEST_F(PeerConnectionInterfaceTest, DataChannelCloseWhenPeerConnectionClose) {
   FakeConstraints constraints;