Simplify ConnectionToHost interface.

Previously ConnectionToHost was responsible for monitoring state of
the signaling connection and creation of JingleSessionManager. Because
of this it was harder to test and the corresponding logic would need
to be duplicated between all implementations.
Now ChromotingClient monitors state of the signaling connection,
creates JingleSessionManager with a Session and then passes the Session
object to ConnectionToHost.

Review URL: https://codereview.chromium.org/1520323007

Cr-Commit-Position: refs/heads/master@{#365854}

5 files changed, 28 insertions, 183 deletions

diff --git a/remoting/protocol/connection_to_host.h b/remoting/protocol/connection_to_host.h
index f5c8c82..b41df20 100644
--- a/remoting/protocol/connection_to_host.h
+++ b/remoting/protocol/connection_to_host.h
@@ -7,27 +7,20 @@
 
 #include <string>
 
-#include "base/callback_forward.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "remoting/protocol/errors.h"
 
 namespace remoting {
-
-class SignalStrategy;
-
 namespace protocol {
 
 class AudioStub;
 class Authenticator;
-class CandidateSessionConfig;
 class ClientStub;
 class ClipboardStub;
-class ExtensionMessage;
 class HostStub;
 class InputStub;
+class Session;
 class SessionConfig;
-class TransportContext;
 struct TransportRoute;
 class VideoStub;
 
@@ -69,11 +62,6 @@ class ConnectionToHost {
 
   virtual ~ConnectionToHost() {}
 
-  // Allows to set a custom protocol configuration (e.g. for tests). Cannot be
-  // called after Connect().
-  virtual void set_candidate_config(
-      scoped_ptr<CandidateSessionConfig> config) = 0;
-
   // Set the stubs which will handle messages from the host.
   // The caller must ensure that stubs out-live the connection.
   // Unless otherwise specified, all stubs must be set before Connect()
@@ -84,17 +72,10 @@ class ConnectionToHost {
   // If no audio stub is specified then audio will not be requested.
   virtual void set_audio_stub(AudioStub* audio_stub) = 0;
 
-  // Initiates a connection to the host specified by |host_jid|.
-  // |signal_strategy| is used to signal to the host, and must outlive the
-  // connection. Data channels will be negotiated over |transport_factory|.
-  // |authenticator| will be used to authenticate the session and data channels.
-  // |event_callback| will be notified of changes in the state of the connection
-  // and must outlive the ConnectionToHost.
-  // Caller must set stubs (see below) before calling Connect.
-  virtual void Connect(SignalStrategy* signal_strategy,
-                       scoped_refptr<TransportContext> transport_context,
-                       scoped_ptr<Authenticator> authenticator,
-                       const std::string& host_jid,
+  // Initiates a connection using |session|. |event_callback| will be notified
+  // of changes in the state of the connection and must outlive the
+  // ConnectionToHost. Caller must set stubs (see below) before calling Connect.
+  virtual void Connect(scoped_ptr<Session> session,
                        HostEventCallback* event_callback) = 0;
 
   // Returns the session configuration that was negotiated with the host.
diff --git a/remoting/protocol/connection_to_host_impl.cc b/remoting/protocol/connection_to_host_impl.cc
index c17b41e..96f84c9 100644
--- a/remoting/protocol/connection_to_host_impl.cc
+++ b/remoting/protocol/connection_to_host_impl.cc
@@ -11,7 +11,6 @@
 #include "remoting/protocol/audio_reader.h"
 #include "remoting/protocol/audio_stub.h"
 #include "remoting/protocol/auth_util.h"
-#include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/client_control_dispatcher.h"
 #include "remoting/protocol/client_event_dispatcher.h"
 #include "remoting/protocol/client_stub.h"
@@ -19,35 +18,14 @@
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/errors.h"
 #include "remoting/protocol/ice_transport.h"
-#include "remoting/protocol/jingle_session_manager.h"
 #include "remoting/protocol/transport.h"
-#include "remoting/protocol/transport_context.h"
 #include "remoting/protocol/video_stub.h"
 
 namespace remoting {
 namespace protocol {
 
-ConnectionToHostImpl::ConnectionToHostImpl()
-    : event_callback_(nullptr),
-      client_stub_(nullptr),
-      clipboard_stub_(nullptr),
-      audio_stub_(nullptr),
-      signal_strategy_(nullptr),
-      state_(INITIALIZING),
-      error_(OK) {}
-
-ConnectionToHostImpl::~ConnectionToHostImpl() {
-  CloseChannels();
-
-  if (session_.get())
-    session_.reset();
-
-  if (session_manager_.get())
-    session_manager_.reset();
-
-  if (signal_strategy_)
-    signal_strategy_->RemoveListener(this);
-}
+ConnectionToHostImpl::ConnectionToHostImpl() {}
+ConnectionToHostImpl::~ConnectionToHostImpl() {}
 
 #define RETURN_STRING_LITERAL(x) \
 case x: \
@@ -66,59 +44,18 @@ const char* ConnectionToHost::StateToString(State state) {
   return nullptr;
 }
 
-void ConnectionToHostImpl::Connect(
-    SignalStrategy* signal_strategy,
-    scoped_refptr<TransportContext> transport_context,
-    scoped_ptr<Authenticator> authenticator,
-    const std::string& host_jid,
-    HostEventCallback* event_callback) {
+void ConnectionToHostImpl::Connect(scoped_ptr<Session> session,
+                                   HostEventCallback* event_callback) {
   DCHECK(client_stub_);
   DCHECK(clipboard_stub_);
   DCHECK(monitored_video_stub_);
 
-  // Initialize default |candidate_config_| if set_candidate_config() wasn't
-  // called.
-  if (!candidate_config_)
-    candidate_config_ = CandidateSessionConfig::CreateDefault();
-  if (!audio_stub_)
-    candidate_config_->DisableAudioChannel();
+  session_ = session.Pass();
+  session_->SetEventHandler(this);
 
-  signal_strategy_ = signal_strategy;
   event_callback_ = event_callback;
-  authenticator_ = authenticator.Pass();
-
-  // Save jid of the host. The actual connection is created later after
-  // |signal_strategy_| is connected.
-  host_jid_ = host_jid;
-
-  signal_strategy_->AddListener(this);
-
-  session_manager_.reset(new JingleSessionManager(
-      make_scoped_ptr(new IceTransportFactory(transport_context)),
-      signal_strategy));
-  session_manager_->set_protocol_config(candidate_config_->Clone());
 
   SetState(CONNECTING, OK);
-
-  switch (signal_strategy_->GetState()) {
-    case SignalStrategy::CONNECTING:
-      // Nothing to do here. Just need to wait until |signal_strategy_| becomes
-      // connected.
-      break;
-    case SignalStrategy::CONNECTED:
-      StartSession();
-      break;
-    case SignalStrategy::DISCONNECTED:
-      signal_strategy_->Connect();
-      break;
-  }
-}
-
-void ConnectionToHostImpl::set_candidate_config(
-    scoped_ptr<CandidateSessionConfig> config) {
-  DCHECK_EQ(state_, INITIALIZING);
-
-  candidate_config_ = config.Pass();
 }
 
 const SessionConfig& ConnectionToHostImpl::config() {
@@ -159,35 +96,6 @@ void ConnectionToHostImpl::set_audio_stub(AudioStub* audio_stub) {
   audio_stub_ = audio_stub;
 }
 
-void ConnectionToHostImpl::StartSession() {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(state_, CONNECTING);
-
-  session_ = session_manager_->Connect(host_jid_, authenticator_.Pass());
-  session_->SetEventHandler(this);
-}
-
-void ConnectionToHostImpl::OnSignalStrategyStateChange(
-    SignalStrategy::State state) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(event_callback_);
-
-  if (state == SignalStrategy::CONNECTED) {
-    VLOG(1) << "Connected as: " << signal_strategy_->GetLocalJid();
-    // After signaling has been connected we can try connecting to the host.
-    if (state_ == CONNECTING)
-      StartSession();
-  } else if (state == SignalStrategy::DISCONNECTED) {
-    VLOG(1) << "Connection closed.";
-    CloseOnError(SIGNALING_ERROR);
-  }
-}
-
-bool ConnectionToHostImpl::OnSignalStrategyIncomingStanza(
-    const buzz::XmlElement* stanza) {
-  return false;
-}
-
 void ConnectionToHostImpl::OnSessionStateChange(Session::State state) {
   DCHECK(CalledOnValidThread());
   DCHECK(event_callback_);
diff --git a/remoting/protocol/connection_to_host_impl.h b/remoting/protocol/connection_to_host_impl.h
index 93699d6..1b48379 100644
--- a/remoting/protocol/connection_to_host_impl.h
+++ b/remoting/protocol/connection_to_host_impl.h
@@ -23,13 +23,8 @@
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/session_config.h"
 #include "remoting/protocol/session_manager.h"
-#include "remoting/signaling/signal_strategy.h"
 
 namespace remoting {
-
-class XmppProxy;
-class VideoPacket;
-
 namespace protocol {
 
 class AudioReader;
@@ -38,7 +33,6 @@ class ClientEventDispatcher;
 class ClientVideoDispatcher;
 
 class ConnectionToHostImpl : public ConnectionToHost,
-                             public SignalStrategy::Listener,
                              public Session::EventHandler,
                              public ChannelDispatcherBase::EventHandler,
                              public base::NonThreadSafe {
@@ -47,15 +41,11 @@ class ConnectionToHostImpl : public ConnectionToHost,
   ~ConnectionToHostImpl() override;
 
   // ConnectionToHost interface.
-  void set_candidate_config(scoped_ptr<CandidateSessionConfig> config) override;
   void set_client_stub(ClientStub* client_stub) override;
   void set_clipboard_stub(ClipboardStub* clipboard_stub) override;
   void set_video_stub(VideoStub* video_stub) override;
   void set_audio_stub(AudioStub* audio_stub) override;
-  void Connect(SignalStrategy* signal_strategy,
-               scoped_refptr<TransportContext> transport_context,
-               scoped_ptr<Authenticator> authenticator,
-               const std::string& host_jid,
+  void Connect(scoped_ptr<Session> session,
                HostEventCallback* event_callback) override;
   const SessionConfig& config() override;
   ClipboardStub* clipboard_forwarder() override;
@@ -64,12 +54,6 @@ class ConnectionToHostImpl : public ConnectionToHost,
   State state() const override;
 
  private:
-  void StartSession();
-
-  // SignalStrategy::StatusObserver interface.
-  void OnSignalStrategyStateChange(SignalStrategy::State state) override;
-  bool OnSignalStrategyIncomingStanza(const buzz::XmlElement* stanza) override;
-
   // Session::EventHandler interface.
   void OnSessionStateChange(Session::State state) override;
   void OnSessionRouteChange(const std::string& channel_name,
@@ -92,20 +76,13 @@ class ConnectionToHostImpl : public ConnectionToHost,
 
   void SetState(State state, ErrorCode error);
 
-  std::string host_jid_;
-  std::string host_public_key_;
-  scoped_ptr<Authenticator> authenticator_;
-
-  HostEventCallback* event_callback_;
-
-  scoped_ptr<CandidateSessionConfig> candidate_config_;
+  HostEventCallback* event_callback_ = nullptr;
 
   // Stub for incoming messages.
-  ClientStub* client_stub_;
-  ClipboardStub* clipboard_stub_;
-  AudioStub* audio_stub_;
+  ClientStub* client_stub_ = nullptr;
+  ClipboardStub* clipboard_stub_ = nullptr;
+  AudioStub* audio_stub_ = nullptr;
 
-  SignalStrategy* signal_strategy_;
   scoped_ptr<SessionManager> session_manager_;
   scoped_ptr<Session> session_;
   scoped_ptr<MonitoredVideoStub> monitored_video_stub_;
@@ -118,8 +95,8 @@ class ConnectionToHostImpl : public ConnectionToHost,
   InputFilter event_forwarder_;
 
   // Internal state of the connection.
-  State state_;
-  ErrorCode error_;
+  State state_ = INITIALIZING;
+  ErrorCode error_ = OK;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ConnectionToHostImpl);
diff --git a/remoting/protocol/fake_connection_to_host.cc b/remoting/protocol/fake_connection_to_host.cc
index 865b61c..f818730 100644
--- a/remoting/protocol/fake_connection_to_host.cc
+++ b/remoting/protocol/fake_connection_to_host.cc
@@ -11,36 +11,20 @@ namespace remoting {
 namespace test {
 
 FakeConnectionToHost::FakeConnectionToHost()
-    : state_(INITIALIZING),
-      session_config_(protocol::SessionConfig::ForTest()) {
-}
+    : session_config_(protocol::SessionConfig::ForTest()) {}
+FakeConnectionToHost::~FakeConnectionToHost() {}
 
-FakeConnectionToHost::~FakeConnectionToHost() {
-}
-
-void FakeConnectionToHost::set_candidate_config(
-    scoped_ptr<protocol::CandidateSessionConfig> config) {
-}
-
-void FakeConnectionToHost::set_client_stub(protocol::ClientStub* client_stub) {
-}
+void FakeConnectionToHost::set_client_stub(protocol::ClientStub* client_stub) {}
 
 void FakeConnectionToHost::set_clipboard_stub(
-    protocol::ClipboardStub* clipboard_stub) {
-}
+    protocol::ClipboardStub* clipboard_stub) {}
 
-void FakeConnectionToHost::set_video_stub(protocol::VideoStub* video_stub) {
-}
+void FakeConnectionToHost::set_video_stub(protocol::VideoStub* video_stub) {}
 
-void FakeConnectionToHost::set_audio_stub(protocol::AudioStub* audio_stub) {
-}
+void FakeConnectionToHost::set_audio_stub(protocol::AudioStub* audio_stub) {}
 
-void FakeConnectionToHost::Connect(
-    SignalStrategy* signal_strategy,
-    scoped_refptr<protocol::TransportContext> transport_context,
-    scoped_ptr<protocol::Authenticator> authenticator,
-    const std::string& host_jid,
-    HostEventCallback* event_callback) {
+void FakeConnectionToHost::Connect(scoped_ptr<protocol::Session> session,
+                                   HostEventCallback* event_callback) {
   DCHECK(event_callback);
 
   event_callback_ = event_callback;
diff --git a/remoting/protocol/fake_connection_to_host.h b/remoting/protocol/fake_connection_to_host.h
index 85ea24a..508d1ea 100644
--- a/remoting/protocol/fake_connection_to_host.h
+++ b/remoting/protocol/fake_connection_to_host.h
@@ -19,16 +19,11 @@ class FakeConnectionToHost : public protocol::ConnectionToHost {
   ~FakeConnectionToHost() override;
 
   // ConnectionToHost interface.
-  void set_candidate_config(
-      scoped_ptr<protocol::CandidateSessionConfig> config) override;
   void set_client_stub(protocol::ClientStub* client_stub) override;
   void set_clipboard_stub(protocol::ClipboardStub* clipboard_stub) override;
   void set_video_stub(protocol::VideoStub* video_stub) override;
   void set_audio_stub(protocol::AudioStub* audio_stub) override;
-  void Connect(SignalStrategy* signal_strategy,
-               scoped_refptr<protocol::TransportContext> transport_context,
-               scoped_ptr<protocol::Authenticator> authenticator,
-               const std::string& host_jid,
+  void Connect(scoped_ptr<protocol::Session> session,
                HostEventCallback* event_callback) override;
   const protocol::SessionConfig& config() override;
   protocol::ClipboardStub* clipboard_forwarder() override;
@@ -47,7 +42,7 @@ class FakeConnectionToHost : public protocol::ConnectionToHost {
  private:
   void SetState(State state, protocol::ErrorCode error);
 
-  State state_;
+  State state_ = INITIALIZING;
 
   HostEventCallback* event_callback_;