commit 9f8164c06054016978378b7a01a9180106d92771
author jiayl@webrtc.org <jiayl@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d> Fri May 30 21:53:17 2014 +0000
committer jiayl@webrtc.org <jiayl@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d> Fri May 30 21:53:17 2014 +0000
tree 226d4022b5e5bd82d33dfdd4d21f84154fefc885
parent 1fddd6185dfccabcddf669a7ec923467aebe1e84

Fix two bugs in DataChannel state transition.
1. OnStateChange should not be fired if state is not changed.
2. RemotePeerRequestClose should be a no-op if it's already closed.

TBR=pthatcher@webrtc.org
BUG=

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

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

talk/app/webrtc/datachannel.cc

diff --git a/talk/app/webrtc/datachannel.cc b/talk/app/webrtc/datachannel.cc
index ef20269..14caa41 100644
--- a/talk/app/webrtc/datachannel.cc
+++ b/talk/app/webrtc/datachannel.cc
@@ -370,6 +370,9 @@
 }
 
 void DataChannel::DoClose() {
+  if (state_ == kClosed)
+    return;
+
   receive_ssrc_set_ = false;
   send_ssrc_set_ = false;
   SetState(kClosing);
@@ -412,6 +415,9 @@
 }
 
 void DataChannel::SetState(DataState state) {
+  if (state_ == state)
+    return;
+
   state_ = state;
   if (observer_) {
     observer_->OnStateChange();

talk/app/webrtc/datachannel_unittest.cc

diff --git a/talk/app/webrtc/datachannel_unittest.cc b/talk/app/webrtc/datachannel_unittest.cc
index e132fb1..991ae0c 100644
--- a/talk/app/webrtc/datachannel_unittest.cc
+++ b/talk/app/webrtc/datachannel_unittest.cc
@@ -34,9 +34,13 @@
 
 class FakeDataChannelObserver : public webrtc::DataChannelObserver {
  public:
-  FakeDataChannelObserver() : messages_received_(0) {}
+  FakeDataChannelObserver()
+      : messages_received_(0), on_state_change_count_(0) {}
 
-  void OnStateChange() {}
+  void OnStateChange() {
+    ++on_state_change_count_;
+  }
+
   void OnMessage(const webrtc::DataBuffer& buffer) {
     ++messages_received_;
   }
@@ -45,8 +49,17 @@
     return messages_received_;
   }
 
+  void ResetOnStateChangeCount() {
+    on_state_change_count_ = 0;
+  }
+
+  size_t on_state_change_count() const {
+    return on_state_change_count_;
+  }
+
  private:
   size_t messages_received_;
+  size_t on_state_change_count_;
 };
 
 class SctpDataChannelTest : public testing::Test {
@@ -339,3 +352,27 @@
   EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
             webrtc_data_channel_->state());
 }
+
+// Tests that a already closed DataChannel does not fire onStateChange again.
+TEST_F(SctpDataChannelTest, ClosedDataChannelDoesNotFireOnStateChange) {
+  AddObserver();
+  webrtc_data_channel_->Close();
+  // OnStateChange called for kClosing and kClosed.
+  EXPECT_EQ(2U, observer_->on_state_change_count());
+
+  observer_->ResetOnStateChangeCount();
+  webrtc_data_channel_->RemotePeerRequestClose();
+  EXPECT_EQ(0U, observer_->on_state_change_count());
+}
+
+// Tests that RemotePeerRequestClose closes the local DataChannel.
+TEST_F(SctpDataChannelTest, RemotePeerRequestClose) {
+  AddObserver();
+  webrtc_data_channel_->RemotePeerRequestClose();
+
+  // OnStateChange called for kClosing and kClosed.
+  EXPECT_EQ(2U, observer_->on_state_change_count());
+  EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
+            webrtc_data_channel_->state());
+}
+