diff options
author | sergeyu <sergeyu@chromium.org> | 2015-12-17 10:17:03 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-17 18:17:46 +0000 |
commit | 06fa7c566663ee46f417da43c8de7746b1104ff0 (patch) | |
tree | 66fe79d10fd7d577c88ae963b4849bb919d0780d | |
parent | 59acb302907bf7ccd8ebd580d2c1418e9c7cd94a (diff) | |
download | chromium_src-06fa7c566663ee46f417da43c8de7746b1104ff0.zip chromium_src-06fa7c566663ee46f417da43c8de7746b1104ff0.tar.gz chromium_src-06fa7c566663ee46f417da43c8de7746b1104ff0.tar.bz2 |
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}
-rw-r--r-- | remoting/client/chromoting_client.cc | 103 | ||||
-rw-r--r-- | remoting/client/chromoting_client.h | 33 | ||||
-rw-r--r-- | remoting/client/client_user_interface.h | 1 | ||||
-rw-r--r-- | remoting/protocol/connection_to_host.h | 29 | ||||
-rw-r--r-- | remoting/protocol/connection_to_host_impl.cc | 104 | ||||
-rw-r--r-- | remoting/protocol/connection_to_host_impl.h | 37 | ||||
-rw-r--r-- | remoting/protocol/fake_connection_to_host.cc | 32 | ||||
-rw-r--r-- | remoting/protocol/fake_connection_to_host.h | 9 | ||||
-rw-r--r-- | remoting/test/test_chromoting_client.cc | 29 | ||||
-rw-r--r-- | remoting/test/test_chromoting_client.h | 7 | ||||
-rw-r--r-- | remoting/test/test_chromoting_client_unittest.cc | 31 |
11 files changed, 164 insertions, 251 deletions
diff --git a/remoting/client/chromoting_client.cc b/remoting/client/chromoting_client.cc index 303150f..075bf30 100644 --- a/remoting/client/chromoting_client.cc +++ b/remoting/client/chromoting_client.cc @@ -4,35 +4,30 @@ #include "remoting/client/chromoting_client.h" -#include "base/bind.h" #include "remoting/base/capabilities.h" #include "remoting/client/audio_decode_scheduler.h" #include "remoting/client/audio_player.h" #include "remoting/client/client_context.h" #include "remoting/client/client_user_interface.h" #include "remoting/client/video_renderer.h" -#include "remoting/proto/audio.pb.h" -#include "remoting/proto/video.pb.h" -#include "remoting/protocol/authentication_method.h" +#include "remoting/protocol/authenticator.h" #include "remoting/protocol/connection_to_host.h" #include "remoting/protocol/host_stub.h" -#include "remoting/protocol/negotiating_client_authenticator.h" +#include "remoting/protocol/ice_transport.h" +#include "remoting/protocol/jingle_session_manager.h" #include "remoting/protocol/session_config.h" #include "remoting/protocol/transport_context.h" namespace remoting { -using protocol::AuthenticationMethod; - ChromotingClient::ChromotingClient(ClientContext* client_context, ClientUserInterface* user_interface, VideoRenderer* video_renderer, scoped_ptr<AudioPlayer> audio_player) - : task_runner_(client_context->main_task_runner()), - user_interface_(user_interface), + : user_interface_(user_interface), video_renderer_(video_renderer), - connection_(new protocol::ConnectionToHostImpl()), - host_capabilities_received_(false) { + connection_(new protocol::ConnectionToHostImpl()) { + DCHECK(client_context->main_task_runner()->BelongsToCurrentThread()); if (audio_player) { audio_decode_scheduler_.reset(new AudioDecodeScheduler( client_context->main_task_runner(), @@ -41,11 +36,8 @@ ChromotingClient::ChromotingClient(ClientContext* client_context, } ChromotingClient::~ChromotingClient() { -} - -void ChromotingClient::set_protocol_config( - scoped_ptr<protocol::CandidateSessionConfig> config) { - connection_->set_candidate_config(config.Pass()); + if (signal_strategy_) + signal_strategy_->RemoveListener(this); } void ChromotingClient::SetConnectionToHostForTests( @@ -59,8 +51,10 @@ void ChromotingClient::Start( scoped_refptr<protocol::TransportContext> transport_context, const std::string& host_jid, const std::string& capabilities) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(!session_manager_); // Start must be called more than once. + host_jid_ = host_jid; local_capabilities_ = capabilities; connection_->set_client_stub(this); @@ -68,13 +62,38 @@ void ChromotingClient::Start( connection_->set_video_stub(video_renderer_->GetVideoStub()); connection_->set_audio_stub(audio_decode_scheduler_.get()); - connection_->Connect(signal_strategy, transport_context, authenticator.Pass(), - host_jid, this); + session_manager_.reset(new protocol::JingleSessionManager( + make_scoped_ptr(new protocol::IceTransportFactory(transport_context)), + signal_strategy)); + + if (!protocol_config_) + protocol_config_ = protocol::CandidateSessionConfig::CreateDefault(); + if (!audio_decode_scheduler_) + protocol_config_->DisableAudioChannel(); + session_manager_->set_protocol_config(protocol_config_.Pass()); + + authenticator_ = authenticator.Pass(); + + signal_strategy_ = signal_strategy; + signal_strategy_->AddListener(this); + + switch (signal_strategy_->GetState()) { + case SignalStrategy::CONNECTING: + // Nothing to do here. Just need to wait until |signal_strategy_| becomes + // connected. + break; + case SignalStrategy::CONNECTED: + StartConnection(); + break; + case SignalStrategy::DISCONNECTED: + signal_strategy_->Connect(); + break; + } } void ChromotingClient::SetCapabilities( const protocol::Capabilities& capabilities) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); // Only accept the first |protocol::Capabilities| message. if (host_capabilities_received_) { @@ -96,28 +115,28 @@ void ChromotingClient::SetCapabilities( void ChromotingClient::SetPairingResponse( const protocol::PairingResponse& pairing_response) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); user_interface_->SetPairingResponse(pairing_response); } void ChromotingClient::DeliverHostMessage( const protocol::ExtensionMessage& message) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); user_interface_->DeliverHostMessage(message); } void ChromotingClient::InjectClipboardEvent( const protocol::ClipboardEvent& event) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); user_interface_->GetClipboardStub()->InjectClipboardEvent(event); } void ChromotingClient::SetCursorShape( const protocol::CursorShapeInfo& cursor_shape) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); user_interface_->GetCursorShapeStub()->SetCursorShape(cursor_shape); } @@ -125,7 +144,7 @@ void ChromotingClient::SetCursorShape( void ChromotingClient::OnConnectionState( protocol::ConnectionToHost::State state, protocol::ErrorCode error) { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); VLOG(1) << "ChromotingClient::OnConnectionState(" << state << ")"; if (state == protocol::ConnectionToHost::AUTHENTICATED) { @@ -148,8 +167,38 @@ void ChromotingClient::OnRouteChanged(const std::string& channel_name, user_interface_->OnRouteChanged(channel_name, route); } +void ChromotingClient::OnSignalStrategyStateChange( + SignalStrategy::State state) { + DCHECK(thread_checker_.CalledOnValidThread()); + + if (state == SignalStrategy::CONNECTED) { + VLOG(1) << "Connected as: " << signal_strategy_->GetLocalJid(); + // After signaling has been connected we can try connecting to the host. + if (connection_ && + connection_->state() == protocol::ConnectionToHost::INITIALIZING) { + StartConnection(); + } + } else if (state == SignalStrategy::DISCONNECTED) { + VLOG(1) << "Signaling connection closed."; + connection_.reset(); + user_interface_->OnConnectionState(protocol::ConnectionToHost::CLOSED, + protocol::SIGNALING_ERROR); + } +} + +bool ChromotingClient::OnSignalStrategyIncomingStanza( + const buzz::XmlElement* stanza) { + return false; +} + +void ChromotingClient::StartConnection() { + DCHECK(thread_checker_.CalledOnValidThread()); + connection_->Connect( + session_manager_->Connect(host_jid_, authenticator_.Pass()), this); +} + void ChromotingClient::OnAuthenticated() { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); // Initialize the decoder. video_renderer_->OnSessionConfig(connection_->config()); @@ -158,7 +207,7 @@ void ChromotingClient::OnAuthenticated() { } void ChromotingClient::OnChannelsConnected() { - DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(thread_checker_.CalledOnValidThread()); // Negotiate capabilities with the host. VLOG(1) << "Client capabilities: " << local_capabilities_; diff --git a/remoting/client/chromoting_client.h b/remoting/client/chromoting_client.h index 93daab2..6c179f4 100644 --- a/remoting/client/chromoting_client.h +++ b/remoting/client/chromoting_client.h @@ -18,6 +18,7 @@ #include "remoting/protocol/input_stub.h" #include "remoting/protocol/performance_tracker.h" #include "remoting/protocol/video_stub.h" +#include "remoting/signaling/signal_strategy.h" namespace base { class SingleThreadTaskRunner; @@ -37,9 +38,9 @@ class ClientUserInterface; class FrameConsumerProxy; class FrameProducer; class VideoRenderer; -class SignalStrategy; -class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, +class ChromotingClient : public SignalStrategy::Listener, + public protocol::ConnectionToHost::HostEventCallback, public protocol::ClientStub { public: // |client_context|, |user_interface| and |video_renderer| must outlive the @@ -52,7 +53,10 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, ~ChromotingClient() override; - void set_protocol_config(scoped_ptr<protocol::CandidateSessionConfig> config); + void set_protocol_config( + scoped_ptr<protocol::CandidateSessionConfig> config) { + protocol_config_ = config.Pass(); + } // Used to set fake/mock objects for tests which use the ChromotingClient. void SetConnectionToHostForTests( @@ -96,17 +100,32 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, const protocol::TransportRoute& route) override; private: + // SignalStrategy::StatusObserver interface. + void OnSignalStrategyStateChange(SignalStrategy::State state) override; + bool OnSignalStrategyIncomingStanza(const buzz::XmlElement* stanza) override; + + // Starts connection once |signal_strategy_| is connected. + void StartConnection(); + // Called when the connection is authenticated. void OnAuthenticated(); // Called when all channels are connected. void OnChannelsConnected(); + base::ThreadChecker thread_checker_; + + scoped_ptr<protocol::CandidateSessionConfig> protocol_config_; + // The following are not owned by this class. - scoped_refptr<base::SingleThreadTaskRunner> task_runner_; - ClientUserInterface* user_interface_; - VideoRenderer* video_renderer_; + ClientUserInterface* user_interface_ = nullptr; + VideoRenderer* video_renderer_ = nullptr; + SignalStrategy* signal_strategy_ = nullptr; + + std::string host_jid_; + scoped_ptr<protocol::Authenticator> authenticator_; + scoped_ptr<protocol::SessionManager> session_manager_; scoped_ptr<protocol::ConnectionToHost> connection_; scoped_ptr<AudioDecodeScheduler> audio_decode_scheduler_; @@ -117,7 +136,7 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, std::string host_capabilities_; // True if |protocol::Capabilities| message has been received. - bool host_capabilities_received_; + bool host_capabilities_received_ = false; // Record the statistics of the connection. protocol::PerformanceTracker perf_tracker_; diff --git a/remoting/client/client_user_interface.h b/remoting/client/client_user_interface.h index 51a097e..59ad3ef 100644 --- a/remoting/client/client_user_interface.h +++ b/remoting/client/client_user_interface.h @@ -17,6 +17,7 @@ namespace remoting { namespace protocol { class ClipboardStub; class CursorShapeStub; +class ExtensionMessage; class PairingResponse; } // namespace protocol 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_; diff --git a/remoting/test/test_chromoting_client.cc b/remoting/test/test_chromoting_client.cc index da19a6d..2e678e4 100644 --- a/remoting/test/test_chromoting_client.cc +++ b/remoting/test/test_chromoting_client.cc @@ -107,17 +107,19 @@ void TestChromotingClient::StartConnection( test_connection_to_host_.Pass()); } - XmppSignalStrategy::XmppServerConfig xmpp_server_config; - xmpp_server_config.host = kXmppHostName; - xmpp_server_config.port = kXmppPortNumber; - xmpp_server_config.use_tls = true; - xmpp_server_config.username = connection_setup_info.user_name; - xmpp_server_config.auth_token = connection_setup_info.access_token; - - // Set up the signal strategy. This must outlive the client object. - signal_strategy_.reset( - new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(), - request_context_getter, xmpp_server_config)); + if (!signal_strategy_) { + XmppSignalStrategy::XmppServerConfig xmpp_server_config; + xmpp_server_config.host = kXmppHostName; + xmpp_server_config.port = kXmppPortNumber; + xmpp_server_config.use_tls = true; + xmpp_server_config.username = connection_setup_info.user_name; + xmpp_server_config.auth_token = connection_setup_info.access_token; + + // Set up the signal strategy. This must outlive the client object. + signal_strategy_.reset( + new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(), + request_context_getter, xmpp_server_config)); + } protocol::NetworkSettings network_settings( protocol::NetworkSettings::NAT_TRAVERSAL_FULL); @@ -187,6 +189,11 @@ void TestChromotingClient::RemoveRemoteConnectionObserver( connection_observers_.RemoveObserver(observer); } +void TestChromotingClient::SetSignalStrategyForTests( + scoped_ptr<SignalStrategy> signal_strategy) { + signal_strategy_ = signal_strategy.Pass(); +} + void TestChromotingClient::SetConnectionToHostForTests( scoped_ptr<protocol::ConnectionToHost> connection_to_host) { test_connection_to_host_ = connection_to_host.Pass(); diff --git a/remoting/test/test_chromoting_client.h b/remoting/test/test_chromoting_client.h index cb0c730..ce9f9ac 100644 --- a/remoting/test/test_chromoting_client.h +++ b/remoting/test/test_chromoting_client.h @@ -67,7 +67,8 @@ class TestChromotingClient : public ClientUserInterface, // Unregisters an observerer from notifications for remote connection events. void RemoveRemoteConnectionObserver(RemoteConnectionObserver* observer); - // Used to set a fake/mock connection object for TestChromotingClient tests. + // Used to set a fake/mock dependencies for tests. + void SetSignalStrategyForTests(scoped_ptr<SignalStrategy> signal_strategy); void SetConnectionToHostForTests( scoped_ptr<protocol::ConnectionToHost> connection_to_host); @@ -114,8 +115,8 @@ class TestChromotingClient : public ClientUserInterface, // Processes video packets from the host. scoped_ptr<VideoRenderer> video_renderer_; - // Used to establish an XMPP connection with the google talk service. - scoped_ptr<XmppSignalStrategy> signal_strategy_; + // SignalStrategy used for connection signaling. + scoped_ptr<SignalStrategy> signal_strategy_; DISALLOW_COPY_AND_ASSIGN(TestChromotingClient); }; diff --git a/remoting/test/test_chromoting_client_unittest.cc b/remoting/test/test_chromoting_client_unittest.cc index 3fe411d..6d2522d 100644 --- a/remoting/test/test_chromoting_client_unittest.cc +++ b/remoting/test/test_chromoting_client_unittest.cc @@ -6,15 +6,11 @@ #include "base/message_loop/message_loop.h" #include "remoting/protocol/fake_connection_to_host.h" +#include "remoting/signaling/fake_signal_strategy.h" #include "remoting/test/connection_setup_info.h" #include "remoting/test/test_chromoting_client.h" #include "testing/gtest/include/gtest/gtest.h" -namespace { -const char kTestUserName[] = "test_user@faux_address.com"; -const char kAccessToken[] = "faux_access_token"; -} - namespace remoting { namespace test { @@ -36,13 +32,14 @@ class TestChromotingClientTest : public ::testing::Test, void TearDown() override; // Used for result verification. - bool is_connected_to_host_; - protocol::ConnectionToHost::State connection_state_; - protocol::ErrorCode error_code_; + bool is_connected_to_host_ = false; + protocol::ConnectionToHost::State connection_state_ = + protocol::ConnectionToHost::INITIALIZING; + protocol::ErrorCode error_code_ = protocol::OK; // Used for simulating different conditions for the TestChromotingClient. ConnectionSetupInfo connection_setup_info_; - FakeConnectionToHost* fake_connection_to_host_; + FakeConnectionToHost* fake_connection_to_host_ = nullptr; scoped_ptr<TestChromotingClient> test_chromoting_client_; @@ -57,15 +54,8 @@ class TestChromotingClientTest : public ::testing::Test, DISALLOW_COPY_AND_ASSIGN(TestChromotingClientTest); }; -TestChromotingClientTest::TestChromotingClientTest() - : is_connected_to_host_(false), - connection_state_(protocol::ConnectionToHost::INITIALIZING), - error_code_(protocol::OK), - fake_connection_to_host_(nullptr) { -} - -TestChromotingClientTest::~TestChromotingClientTest() { -} +TestChromotingClientTest::TestChromotingClientTest() {} +TestChromotingClientTest::~TestChromotingClientTest() {} void TestChromotingClientTest::SetUp() { test_chromoting_client_.reset(new TestChromotingClient()); @@ -75,11 +65,12 @@ void TestChromotingClientTest::SetUp() { // keep the ptr around so we can use it to simulate state changes. It will // remain valid until |test_chromoting_client_| is destroyed. fake_connection_to_host_ = new FakeConnectionToHost(); + test_chromoting_client_->SetSignalStrategyForTests(make_scoped_ptr( + new FakeSignalStrategy("test_user@faux_address.com/123"))); test_chromoting_client_->SetConnectionToHostForTests( make_scoped_ptr(fake_connection_to_host_)); - connection_setup_info_.access_token = kAccessToken; - connection_setup_info_.user_name = kTestUserName; + connection_setup_info_.host_jid = "test_host@faux_address.com/321"; connection_setup_info_.auth_methods.push_back( protocol::AuthenticationMethod::ThirdParty()); } |