Added control channel in ChromotingConnection. Moved channel

initialization to OnInitiate() in JingleChromotingConnection.
BUG=None
TEST=Unittests.

Review URL: http://codereview.chromium.org/4055002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63908 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 142 insertions, 60 deletions

diff --git a/remoting/host/client_connection_unittest.cc b/remoting/host/client_connection_unittest.cc
index 317d7fa..fcb3df3 100644
--- a/remoting/host/client_connection_unittest.cc
+++ b/remoting/host/client_connection_unittest.cc
@@ -30,7 +30,7 @@ class ClientConnectionTest : public testing::Test {
     viewer_ = new ClientConnection(&message_loop_, &handler_);
     viewer_->Init(connection_);
     EXPECT_CALL(handler_, OnConnectionOpened(viewer_.get()));
-    connection_->get_state_change_callback()->Run(
+    connection_->state_change_callback()->Run(
         ChromotingConnection::CONNECTED);
     message_loop_.RunAllPending();
   }
@@ -62,11 +62,11 @@ TEST_F(ClientConnectionTest, SendUpdateStream) {
 
 TEST_F(ClientConnectionTest, StateChange) {
   EXPECT_CALL(handler_, OnConnectionClosed(viewer_.get()));
-  connection_->get_state_change_callback()->Run(ChromotingConnection::CLOSED);
+  connection_->state_change_callback()->Run(ChromotingConnection::CLOSED);
   message_loop_.RunAllPending();
 
   EXPECT_CALL(handler_, OnConnectionFailed(viewer_.get()));
-  connection_->get_state_change_callback()->Run(ChromotingConnection::FAILED);
+  connection_->state_change_callback()->Run(ChromotingConnection::FAILED);
   message_loop_.RunAllPending();
 }
 
diff --git a/remoting/protocol/chromoting_connection.h b/remoting/protocol/chromoting_connection.h
index 0f54104..cfc8d3d7 100644
--- a/remoting/protocol/chromoting_connection.h
+++ b/remoting/protocol/chromoting_connection.h
@@ -42,24 +42,28 @@ class ChromotingConnection
   virtual void SetStateChangeCallback(StateChangeCallback* callback) = 0;
 
   // Reliable PseudoTCP channels for this connection.
-  // TODO(sergeyu): Remove VideoChannel, and use RTP channels instead.
-  // TODO(sergeyu): Make it possible to create/destroy new channels on-fly?
+  virtual net::Socket* GetControlChannel() = 0;
   virtual net::Socket* GetEventChannel() = 0;
+
+  // TODO(sergeyu): Remove VideoChannel, and use RTP channels instead.
   virtual net::Socket* GetVideoChannel() = 0;
 
   // Unreliable channels for this connection.
   virtual net::Socket* GetVideoRtpChannel() = 0;
   virtual net::Socket* GetVideoRtcpChannel() = 0;
 
+  // TODO(sergeyu): Make it possible to create/destroy additional channels
+  // on-fly?
+
   // JID of the other side.
   virtual const std::string& jid() = 0;
 
-  // Message loop that must be used for to access the channels of this
-  // connection.
+  // Message loop that must be used to access the channels of this connection.
   virtual MessageLoop* message_loop() = 0;
 
-  // Configuration of the protocol requested by the client.
-  // Returned pointer is valid until connection is closed.
+  // Configuration of the protocol that was sent or received in the
+  // session-initiate jingle message. Returned pointer is valid until
+  // connection is closed.
   virtual const CandidateChromotocolConfig* candidate_config() = 0;
 
   // Protocol configuration. Can be called only after session has been accepted.
diff --git a/remoting/protocol/fake_connection.cc b/remoting/protocol/fake_connection.cc
index 9f40e65..e0bb28e 100644
--- a/remoting/protocol/fake_connection.cc
+++ b/remoting/protocol/fake_connection.cc
@@ -83,13 +83,18 @@ void FakeChromotingConnection::SetStateChangeCallback(
   callback_.reset(callback);
 }
 
-FakeSocket* FakeChromotingConnection::GetVideoChannel() {
-  return &video_channel_;
+FakeSocket* FakeChromotingConnection::GetControlChannel() {
+  return &control_channel_;
 }
+
 FakeSocket* FakeChromotingConnection::GetEventChannel() {
   return &event_channel_;
 }
 
+FakeSocket* FakeChromotingConnection::GetVideoChannel() {
+  return &video_channel_;
+}
+
 FakeSocket* FakeChromotingConnection::GetVideoRtpChannel() {
   return &video_rtp_channel_;
 }
diff --git a/remoting/protocol/fake_connection.h b/remoting/protocol/fake_connection.h
index 33b120e..b69ce6c 100644
--- a/remoting/protocol/fake_connection.h
+++ b/remoting/protocol/fake_connection.h
@@ -57,7 +57,7 @@ class FakeChromotingConnection : public ChromotingConnection {
   FakeChromotingConnection();
   virtual ~FakeChromotingConnection();
 
-  StateChangeCallback* get_state_change_callback() { return callback_.get(); }
+  StateChangeCallback* state_change_callback() { return callback_.get(); }
 
   void set_message_loop(MessageLoop* message_loop) {
     message_loop_ = message_loop;
@@ -67,8 +67,9 @@ class FakeChromotingConnection : public ChromotingConnection {
 
   virtual void SetStateChangeCallback(StateChangeCallback* callback);
 
-  virtual FakeSocket* GetVideoChannel();
+  virtual FakeSocket* GetControlChannel();
   virtual FakeSocket* GetEventChannel();
+  virtual FakeSocket* GetVideoChannel();
 
   virtual FakeSocket* GetVideoRtpChannel();
   virtual FakeSocket* GetVideoRtcpChannel();
@@ -87,8 +88,9 @@ class FakeChromotingConnection : public ChromotingConnection {
   scoped_ptr<const CandidateChromotocolConfig> candidate_config_;
   scoped_ptr<const ChromotocolConfig> config_;
   MessageLoop* message_loop_;
-  FakeSocket video_channel_;
+  FakeSocket control_channel_;
   FakeSocket event_channel_;
+  FakeSocket video_channel_;
   FakeSocket video_rtp_channel_;
   FakeSocket video_rtcp_channel_;
   std::string jid_;
diff --git a/remoting/protocol/jingle_chromoting_connection.cc b/remoting/protocol/jingle_chromoting_connection.cc
index 10d8709..fe1cb7d 100644
--- a/remoting/protocol/jingle_chromoting_connection.cc
+++ b/remoting/protocol/jingle_chromoting_connection.cc
@@ -21,12 +21,15 @@ using cricket::Session;
 namespace remoting {
 
 namespace {
+const char kControlChannelName[] = "control";
+const char kEventChannelName[] = "event";
 const char kVideoChannelName[] = "video";
 const char kVideoRtpChannelName[] = "videortp";
 const char kVideoRtcpChannelName[] = "videortcp";
-const char kEventChannelName[] = "event";
 }  // namespace
 
+const char JingleChromotingConnection::kChromotingContentName[] = "chromoting";
+
 JingleChromotingConnection::JingleChromotingConnection(
     JingleChromotingServer* server)
     : server_(server),
@@ -73,10 +76,9 @@ void JingleChromotingConnection::SetStateChangeCallback(
   state_change_callback_.reset(callback);
 }
 
-// TODO(sergeyu): Remove this method after we switch to RTP.
-net::Socket* JingleChromotingConnection::GetVideoChannel() {
+net::Socket* JingleChromotingConnection::GetControlChannel() {
   DCHECK_EQ(server_->message_loop(), MessageLoop::current());
-  return video_channel_adapter_.get();
+  return control_channel_adapter_.get();
 }
 
 net::Socket* JingleChromotingConnection::GetEventChannel() {
@@ -84,6 +86,12 @@ net::Socket* JingleChromotingConnection::GetEventChannel() {
   return event_channel_adapter_.get();
 }
 
+// TODO(sergeyu): Remove this method after we switch to RTP.
+net::Socket* JingleChromotingConnection::GetVideoChannel() {
+  DCHECK_EQ(server_->message_loop(), MessageLoop::current());
+  return video_channel_adapter_.get();
+}
+
 net::Socket* JingleChromotingConnection::GetVideoRtpChannel() {
   DCHECK_EQ(server_->message_loop(), MessageLoop::current());
   return video_rtp_channel_.get();
@@ -137,6 +145,14 @@ void JingleChromotingConnection::Close(Task* closed_task) {
   }
 
   if (!closed_) {
+    if (control_channel_adapter_.get())
+      control_channel_adapter_->Close(net::ERR_CONNECTION_CLOSED);
+
+    if (control_channel_) {
+      control_channel_->OnSessionTerminate(session_);
+      control_channel_ = NULL;
+    }
+
     if (event_channel_adapter_.get())
       event_channel_adapter_->Close(net::ERR_CONNECTION_CLOSED);
 
@@ -176,22 +192,19 @@ void JingleChromotingConnection::OnSessionState(
 
   switch (state) {
     case Session::STATE_SENTINITIATE:
-      OnInitiate(false);
-      break;
-
     case Session::STATE_RECEIVEDINITIATE:
-      OnInitiate(true);
+      OnInitiate();
       break;
 
     case Session::STATE_SENTACCEPT:
-      OnAccept(false);
-      break;
-
     case Session::STATE_RECEIVEDACCEPT:
-      OnAccept(true);
+      OnAccept();
       break;
 
+    case Session::STATE_SENTTERMINATE:
     case Session::STATE_RECEIVEDTERMINATE:
+    case Session::STATE_SENTREJECT:
+    case Session::STATE_RECEIVEDREJECT:
       OnTerminate();
       break;
 
@@ -201,50 +214,45 @@ void JingleChromotingConnection::OnSessionState(
       break;
 
     default:
+      // We don't care about other steates.
       break;
   }
 }
 
-void JingleChromotingConnection::OnInitiate(bool incoming) {
+void JingleChromotingConnection::OnInitiate() {
   jid_ = session_->remote_name();
-  if (incoming)
-    server_->AcceptConnection(this, session_);
-  SetState(CONNECTING);
-}
-
-void JingleChromotingConnection::OnAccept(bool incoming) {
-  const cricket::ContentInfo* content =
-      session_->remote_description()->FirstContentByType(
-          kChromotingXmlNamespace);
-  CHECK(content);
-
-  // Set config for outgoing connections.
-  if (incoming) {
-    const ChromotingContentDescription* content_description =
-        static_cast<const ChromotingContentDescription*>(content->description);
-    ChromotocolConfig* config = content_description->config()->GetFinalConfig();
 
-    // Terminate the session if the config we received is invalid.
-    if (!config || !candidate_config()->IsSupported(config)) {
-      LOG(ERROR) << "Terminating outgoing session after an "
-          "invalid session description has been received.";
-      session_->Terminate();
-      return;
-    }
-
-    set_config(config);
+  std::string content_name;
+  // If we initiate the connection, we get to specify the content name. When
+  // accepting one, the remote end specifies it.
+  if (session_->initiator()) {
+    content_name = kChromotingContentName;
+  } else {
+    const cricket::ContentInfo* content;
+    content = session_->remote_description()->FirstContentByType(
+        kChromotingXmlNamespace);
+    CHECK(content);
+    content_name = content->name;
   }
 
   // Create video RTP channels.
   video_rtp_channel_.reset(new TransportChannelSocketAdapter(
-      session_->CreateChannel(content->name, kVideoRtpChannelName)));
+      session_->CreateChannel(content_name, kVideoRtpChannelName)));
   video_rtcp_channel_.reset(new TransportChannelSocketAdapter(
-      session_->CreateChannel(content->name, kVideoRtcpChannelName)));
+      session_->CreateChannel(content_name, kVideoRtcpChannelName)));
+
+  // Create control channel.
+  // TODO(sergeyu): Don't use talk_base::Thread::Current() here.
+  control_channel_ =
+      new PseudoTcpChannel(talk_base::Thread::Current(), session_);
+  control_channel_->Connect(content_name, kControlChannelName);
+  control_channel_adapter_.reset(new StreamSocketAdapter(
+      control_channel_->GetStream()));
 
   // Create event channel.
   event_channel_ =
       new PseudoTcpChannel(talk_base::Thread::Current(), session_);
-  event_channel_->Connect(content->name, kEventChannelName);
+  event_channel_->Connect(content_name, kEventChannelName);
   event_channel_adapter_.reset(new StreamSocketAdapter(
       event_channel_->GetStream()));
 
@@ -252,14 +260,51 @@ void JingleChromotingConnection::OnAccept(bool incoming) {
   // TODO(sergeyu): Remove video channel when we are ready to switch to RTP.
   video_channel_ =
       new PseudoTcpChannel(talk_base::Thread::Current(), session_);
-  video_channel_->Connect(content->name, kVideoChannelName);
+  video_channel_->Connect(content_name, kVideoChannelName);
   video_channel_adapter_.reset(new StreamSocketAdapter(
       video_channel_->GetStream()));
 
+  if (!session_->initiator())
+    server_->AcceptConnection(this, session_);
+
+  SetState(CONNECTING);
+}
+
+void JingleChromotingConnection::OnAccept() {
+  // Set config for outgoing connections.
+  if (session_->initiator()) {
+    const cricket::ContentInfo* content =
+        session_->remote_description()->FirstContentByType(
+            kChromotingXmlNamespace);
+    CHECK(content);
+
+    const ChromotingContentDescription* content_description =
+        static_cast<const ChromotingContentDescription*>(content->description);
+    ChromotocolConfig* config = content_description->config()->GetFinalConfig();
+
+    // Terminate the session if the config we received is invalid.
+    if (!config || !candidate_config()->IsSupported(config)) {
+      // TODO(sergeyu): Inform the user that the host is misbehaving?
+      LOG(ERROR) << "Terminating outgoing session after an "
+          "invalid session description has been received.";
+      session_->Terminate();
+      return;
+    }
+
+    set_config(config);
+  }
+
   SetState(CONNECTED);
 }
 
 void JingleChromotingConnection::OnTerminate() {
+  if (control_channel_adapter_.get())
+    control_channel_adapter_->Close(net::ERR_CONNECTION_ABORTED);
+  if (control_channel_) {
+    control_channel_->OnSessionTerminate(session_);
+    control_channel_ = NULL;
+  }
+
   if (event_channel_adapter_.get())
     event_channel_adapter_->Close(net::ERR_CONNECTION_ABORTED);
   if (event_channel_) {
diff --git a/remoting/protocol/jingle_chromoting_connection.h b/remoting/protocol/jingle_chromoting_connection.h
index 3279a20..736d11e 100644
--- a/remoting/protocol/jingle_chromoting_connection.h
+++ b/remoting/protocol/jingle_chromoting_connection.h
@@ -31,13 +31,17 @@ class TransportChannelSocketAdapter;
 class JingleChromotingConnection : public ChromotingConnection,
                                    public sigslot::has_slots<> {
  public:
+  static const char kChromotingContentName[];
+
   explicit JingleChromotingConnection(JingleChromotingServer* client);
 
   // ChromotingConnection interface.
   virtual void SetStateChangeCallback(StateChangeCallback* callback);
 
-  virtual net::Socket* GetVideoChannel();
+  virtual net::Socket* GetControlChannel();
   virtual net::Socket* GetEventChannel();
+  virtual net::Socket* GetVideoChannel();
+
   virtual net::Socket* GetVideoRtpChannel();
   virtual net::Socket* GetVideoRtcpChannel();
 
@@ -67,8 +71,8 @@ class JingleChromotingConnection : public ChromotingConnection,
   void OnSessionState(cricket::BaseSession* session,
                       cricket::BaseSession::State state);
 
-  void OnInitiate(bool incoming);
-  void OnAccept(bool incoming);
+  void OnInitiate();
+  void OnAccept();
   void OnTerminate();
 
   void SetState(State new_state);
@@ -91,6 +95,8 @@ class JingleChromotingConnection : public ChromotingConnection,
   scoped_ptr<const CandidateChromotocolConfig> candidate_config_;
   scoped_ptr<const ChromotocolConfig> config_;
 
+  cricket::PseudoTcpChannel* control_channel_;
+  scoped_ptr<StreamSocketAdapter> control_channel_adapter_;
   cricket::PseudoTcpChannel* event_channel_;
   scoped_ptr<StreamSocketAdapter> event_channel_adapter_;
   cricket::PseudoTcpChannel* video_channel_;
diff --git a/remoting/protocol/jingle_chromoting_connection_unittest.cc b/remoting/protocol/jingle_chromoting_connection_unittest.cc
index abd00f9..a369124 100644
--- a/remoting/protocol/jingle_chromoting_connection_unittest.cc
+++ b/remoting/protocol/jingle_chromoting_connection_unittest.cc
@@ -199,6 +199,7 @@ class JingleChromotingConnectionTest : public testing::Test {
 class ChannelTesterBase : public base::RefCountedThreadSafe<ChannelTesterBase> {
  public:
   enum ChannelType {
+    CONTROL,
     EVENT,
     VIDEO,
     VIDEO_RTP,
@@ -246,6 +247,8 @@ class ChannelTesterBase : public base::RefCountedThreadSafe<ChannelTesterBase> {
   net::Socket* SelectChannel(ChromotingConnection* connection,
                              ChannelType channel) {
     switch (channel) {
+      case CONTROL:
+        return connection->GetControlChannel();
       case EVENT:
         return connection->GetEventChannel();
       case VIDEO:
@@ -542,6 +545,22 @@ TEST_F(JingleChromotingConnectionTest, Connect) {
   InitiateConnection();
 }
 
+// Verify that data can be transmitted over the event channel.
+TEST_F(JingleChromotingConnectionTest, TestControlChannel) {
+  CreateServerPair();
+  InitiateConnection();
+  scoped_refptr<TCPChannelTester> tester =
+      new TCPChannelTester(thread_.message_loop(), host_connection_,
+                           client_connection_);
+  tester->Start(ChannelTesterBase::CONTROL);
+  tester->WaitFinished();
+  tester->CheckResults();
+
+  // Connections must be closed while |tester| still exists.
+  CloseConnections();
+}
+
+
 // Verify that data can be transmitted over the video channel.
 TEST_F(JingleChromotingConnectionTest, TestVideoChannel) {
   CreateServerPair();
diff --git a/remoting/protocol/jingle_chromoting_server.cc b/remoting/protocol/jingle_chromoting_server.cc
index 09ed2b7..802689d 100644
--- a/remoting/protocol/jingle_chromoting_server.cc
+++ b/remoting/protocol/jingle_chromoting_server.cc
@@ -462,7 +462,8 @@ bool JingleChromotingServer::WriteContent(
 SessionDescription* JingleChromotingServer::CreateSessionDescription(
     const CandidateChromotocolConfig* config) {
   SessionDescription* desc = new SessionDescription();
-  desc->AddContent(kChromotingContentName, kChromotingXmlNamespace,
+  desc->AddContent(JingleChromotingConnection::kChromotingContentName,
+                   kChromotingXmlNamespace,
                    new ChromotingContentDescription(config));
   return desc;
 }