Closes the DataChannel when the send buffer is full or on transport errors.

As stated in the spec.

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

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

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

talk/app/webrtc/datachannel.cc

@@ -164,17 +164,23 @@ bool DataChannel::Send(const DataBuffer& buffer) {
   // If the queue is non-empty, we're waiting for SignalReadyToSend,
   // so just add to the end of the queue and keep waiting.
   if (!queued_send_data_.empty()) {
-    return QueueSendData(buffer);
+    if (!QueueSendData(buffer)) {
+      if (data_channel_type_ == cricket::DCT_RTP) {
+        return false;
+      }
+      Close();
+    }
+    return true;
   }
 
   cricket::SendDataResult send_result;
   if (!InternalSendWithoutQueueing(buffer, &send_result)) {
-    if (send_result == cricket::SDR_BLOCK) {
-      return QueueSendData(buffer);
+    if (data_channel_type_ == cricket::DCT_RTP) {
+      return false;
+    }
+    if (send_result != cricket::SDR_BLOCK || !QueueSendData(buffer)) {
+      Close();
     }
-    // Fail for other results.
-    // TODO(jiayl): We should close the data channel in this case.
-    return false;
   }
   return true;
 }
@@ -325,9 +331,8 @@ void DataChannel::OnDataReceived(cricket::DataChannel* channel,
     observer_->OnMessage(*buffer.get());
   } else {
     if (queued_received_data_.size() > kMaxQueuedReceivedDataPackets) {
-      // TODO(jiayl): We should close the data channel in this case.
       LOG(LS_ERROR)
-          << "Queued received data exceeds the max number of packes.";
+          << "Queued received data exceeds the max number of packets.";
       ClearQueuedReceivedData();
     }
     queued_received_data_.push(buffer.release());
@@ -522,8 +527,8 @@ bool DataChannel::InternalSendWithoutQueueing(
 }
 
 bool DataChannel::QueueSendData(const DataBuffer& buffer) {
-  if (queued_send_data_.size() > kMaxQueuedSendDataPackets) {
-    LOG(LS_ERROR) << "Can't buffer any more data in the data channel.";
+  if (queued_send_data_.size() >= kMaxQueuedSendDataPackets) {
+    LOG(LS_ERROR) << "Can't buffer any more data for the data channel.";
     return false;
   }
   queued_send_data_.push_back(new DataBuffer(buffer));

talk/app/webrtc/datachannel_unittest.cc

@@ -29,14 +29,24 @@
 #include "talk/app/webrtc/sctputils.h"
 #include "talk/app/webrtc/test/fakedatachannelprovider.h"
 #include "talk/base/gunit.h"
-#include "testing/base/public/gmock.h"
 
 using webrtc::DataChannel;
 
 class FakeDataChannelObserver : public webrtc::DataChannelObserver {
  public:
-  MOCK_METHOD0(OnStateChange, void());
-  MOCK_METHOD1(OnMessage, void(const webrtc::DataBuffer& buffer));
+  FakeDataChannelObserver() : messages_received_(0) {}
+
+  void OnStateChange() {}
+  void OnMessage(const webrtc::DataBuffer& buffer) {
+    ++messages_received_;
+  }
+
+  size_t messages_received() const {
+    return messages_received_;
+  }
+
+ private:
+  size_t messages_received_;
 };
 
 class SctpDataChannelTest : public testing::Test {
@@ -233,12 +243,13 @@ TEST_F(SctpDataChannelTest, ReceiveDataWithInvalidSsrc) {
   SetChannelReady();
 
   AddObserver();
-  EXPECT_CALL(*(observer_.get()), OnMessage(testing::_)).Times(0);
 
   cricket::ReceiveDataParams params;
   params.ssrc = 0;
   webrtc::DataBuffer buffer("abcd");
   webrtc_data_channel_->OnDataReceived(NULL, params, buffer.data);
+
+  EXPECT_EQ(0U, observer_->messages_received());
 }
 
 // Tests that the incoming messages with right ssrcs are acceted.
@@ -247,13 +258,13 @@ TEST_F(SctpDataChannelTest, ReceiveDataWithValidSsrc) {
   SetChannelReady();
 
   AddObserver();
-  EXPECT_CALL(*(observer_.get()), OnMessage(testing::_)).Times(1);
 
   cricket::ReceiveDataParams params;
   params.ssrc = 1;
   webrtc::DataBuffer buffer("abcd");
 
   webrtc_data_channel_->OnDataReceived(NULL, params, buffer.data);
+  EXPECT_EQ(1U, observer_->messages_received());
 }
 
 // Tests that no CONTROL message is sent if the datachannel is negotiated and
@@ -302,3 +313,29 @@ TEST_F(SctpDataChannelTest, OpenAckRoleInitialization) {
   webrtc::InternalDataChannelInit init2(base);
   EXPECT_EQ(webrtc::InternalDataChannelInit::kNone, init2.open_handshake_role);
 }
+
+// Tests that the DataChannel is closed if the sending buffer is full.
+TEST_F(SctpDataChannelTest, ClosedWhenSendBufferFull) {
+  SetChannelReady();
+  webrtc::DataBuffer buffer("abcd");
+  provider_.set_send_blocked(true);
+
+  for (size_t i = 0; i < 101; ++i) {
+    EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
+  }
+
+  EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
+            webrtc_data_channel_->state());
+}
+
+// Tests that the DataChannel is closed on transport errors.
+TEST_F(SctpDataChannelTest, ClosedOnTransportError) {
+  SetChannelReady();
+  webrtc::DataBuffer buffer("abcd");
+  provider_.set_transport_error();
+
+  EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
+
+  EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
+            webrtc_data_channel_->state());
+}

talk/app/webrtc/test/fakedatachannelprovider.h

@@ -32,7 +32,8 @@ class FakeDataChannelProvider : public webrtc::DataChannelProviderInterface {
   FakeDataChannelProvider()
       : send_blocked_(false),
         transport_available_(false),
-        ready_to_send_(false) {}
+        ready_to_send_(false),
+        transport_error_(false) {}
   virtual ~FakeDataChannelProvider() {}
 
   virtual bool SendData(const cricket::SendDataParams& params,
@@ -43,6 +44,12 @@ class FakeDataChannelProvider : public webrtc::DataChannelProviderInterface {
       *result = cricket::SDR_BLOCK;
       return false;
     }
+
+    if (transport_error_) {
+      *result = cricket::SDR_ERROR;
+      return false;
+    }
+
     last_send_data_params_ = params;
     return true;
   }
@@ -115,6 +122,10 @@ class FakeDataChannelProvider : public webrtc::DataChannelProviderInterface {
     }
   }
 
+  void set_transport_error() {
+    transport_error_ = true;
+  }
+
   cricket::SendDataParams last_send_data_params() const {
     return last_send_data_params_;
   }
@@ -136,6 +147,7 @@ class FakeDataChannelProvider : public webrtc::DataChannelProviderInterface {
   bool send_blocked_;
   bool transport_available_;
   bool ready_to_send_;
+  bool transport_error_;
   std::set<webrtc::DataChannel*> connected_channels_;
   std::set<uint32> send_ssrcs_;
   std::set<uint32> recv_ssrcs_;

talk/libjingle_tests.gyp

@@ -393,7 +393,7 @@
       ],
       # TODO(ronghuawu): Reenable below unit tests that require gmock.
       'sources': [
-        # 'app/webrtc/datachannel_unittest.cc',
+        'app/webrtc/datachannel_unittest.cc',
         'app/webrtc/dtmfsender_unittest.cc',
         'app/webrtc/jsepsessiondescription_unittest.cc',
         'app/webrtc/localaudiosource_unittest.cc',