diff options
author | sergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-01-04 01:10:11 +0000 |
---|---|---|
committer | sergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-01-04 01:10:11 +0000 |
commit | 988dfc3c6ce0ee36d607a957c2447fa45488c745 (patch) | |
tree | 03ae5fcdfeda3fd64376545372beea4b8e644aab /remoting | |
parent | 7362b51c76a0b6c0f7ed21382333ef4aed1b4a88 (diff) | |
download | chromium_src-988dfc3c6ce0ee36d607a957c2447fa45488c745.zip chromium_src-988dfc3c6ce0ee36d607a957c2447fa45488c745.tar.gz chromium_src-988dfc3c6ce0ee36d607a957c2447fa45488c745.tar.bz2 |
Move signaling connection creation out of ChromotingHost.
Also removed signaling-related events from ChromotingHostObserver interface
and changes host observers to listen for notifications from SignalStrategy
directly.
BUG=107276
Review URL: http://codereview.chromium.org/9004050
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@116256 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'remoting')
-rw-r--r-- | remoting/host/chromoting_host.cc | 64 | ||||
-rw-r--r-- | remoting/host/chromoting_host.h | 31 | ||||
-rw-r--r-- | remoting/host/chromoting_host_unittest.cc | 9 | ||||
-rw-r--r-- | remoting/host/heartbeat_sender.cc | 75 | ||||
-rw-r--r-- | remoting/host/heartbeat_sender.h | 35 | ||||
-rw-r--r-- | remoting/host/heartbeat_sender_unittest.cc | 64 | ||||
-rw-r--r-- | remoting/host/host_status_observer.h | 6 | ||||
-rw-r--r-- | remoting/host/it2me_host_user_interface.cc | 9 | ||||
-rw-r--r-- | remoting/host/it2me_host_user_interface.h | 4 | ||||
-rw-r--r-- | remoting/host/log_to_server.cc | 34 | ||||
-rw-r--r-- | remoting/host/log_to_server.h | 26 | ||||
-rw-r--r-- | remoting/host/log_to_server_unittest.cc | 28 | ||||
-rw-r--r-- | remoting/host/plugin/host_script_object.cc | 35 | ||||
-rw-r--r-- | remoting/host/plugin/host_script_object.h | 8 | ||||
-rw-r--r-- | remoting/host/register_support_host_request.cc | 82 | ||||
-rw-r--r-- | remoting/host/register_support_host_request.h | 49 | ||||
-rw-r--r-- | remoting/host/register_support_host_request_unittest.cc | 46 | ||||
-rw-r--r-- | remoting/host/remoting_me2me_host.cc | 61 | ||||
-rw-r--r-- | remoting/host/simple_host_process.cc | 91 |
19 files changed, 361 insertions, 396 deletions
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc index 3633d23..b16a83ce 100644 --- a/remoting/host/chromoting_host.cc +++ b/remoting/host/chromoting_host.cc @@ -18,7 +18,6 @@ #include "remoting/host/event_executor.h" #include "remoting/host/host_config.h" #include "remoting/host/screen_recorder.h" -#include "remoting/jingle_glue/xmpp_signal_strategy.h" #include "remoting/protocol/connection_to_client.h" #include "remoting/protocol/client_stub.h" #include "remoting/protocol/host_stub.h" @@ -32,16 +31,9 @@ using remoting::protocol::InputStub; namespace remoting { -// static -ChromotingHost* ChromotingHost::Create(ChromotingHostContext* context, - MutableHostConfig* config, - DesktopEnvironment* environment, - bool allow_nat_traversal) { - return new ChromotingHost(context, config, environment, allow_nat_traversal); -} - ChromotingHost::ChromotingHost(ChromotingHostContext* context, MutableHostConfig* config, + SignalStrategy* signal_strategy, DesktopEnvironment* environment, bool allow_nat_traversal) : context_(context), @@ -49,10 +41,13 @@ ChromotingHost::ChromotingHost(ChromotingHostContext* context, config_(config), allow_nat_traversal_(allow_nat_traversal), have_shared_secret_(false), + signal_strategy_(signal_strategy), stopping_recorders_(0), state_(kInitial), protocol_config_(protocol::CandidateSessionConfig::CreateDefault()), is_it2me_(false) { + DCHECK(context_); + DCHECK(signal_strategy); DCHECK(desktop_environment_); desktop_environment_->set_host(this); } @@ -69,7 +64,6 @@ void ChromotingHost::Start() { } LOG(INFO) << "Starting host"; - DCHECK(!signal_strategy_.get()); // Make sure this object is not started. if (state_ != kInitial) @@ -82,29 +76,10 @@ void ChromotingHost::Start() { return; } - // Use an XMPP connection to the Talk network for session signalling. - std::string xmpp_login; - std::string xmpp_auth_token; - std::string xmpp_auth_service; - if (!config_->GetString(kXmppLoginConfigPath, &xmpp_login) || - !config_->GetString(kXmppAuthTokenConfigPath, &xmpp_auth_token) || - !config_->GetString(kXmppAuthServiceConfigPath, &xmpp_auth_service)) { - LOG(ERROR) << "XMPP credentials are not defined in the config."; - return; - } - - // Create and start XMPP connection. - signal_strategy_.reset( - new XmppSignalStrategy(context_->jingle_thread(), xmpp_login, - xmpp_auth_token, xmpp_auth_service)); - signal_strategy_->AddListener(this); - signal_strategy_->Connect(); - // Create and start session manager. session_manager_.reset( new protocol::JingleSessionManager(context_->network_message_loop())); - session_manager_->Init(signal_strategy_.get(), - this, allow_nat_traversal_); + session_manager_->Init(signal_strategy_, this, allow_nat_traversal_); } // This method is called when we need to destroy the host process. @@ -145,13 +120,6 @@ void ChromotingHost::Shutdown(const base::Closure& shutdown_task) { have_shared_secret_ = false; } - // Stop XMPP connection synchronously. - if (signal_strategy_.get()) { - signal_strategy_->Disconnect(); - signal_strategy_->RemoveListener(this); - signal_strategy_.reset(); - } - if (recorder_.get()) { StopScreenRecorder(); } else { @@ -254,28 +222,6 @@ void ChromotingHost::OnSessionSequenceNumber(ClientSession* session, recorder_->UpdateSequenceNumber(sequence_number); } -//////////////////////////////////////////////////////////////////////////// -// SignalStrategy::StatusObserver implementations -void ChromotingHost::OnSignalStrategyStateChange( - SignalStrategy::State state) { - DCHECK(context_->network_message_loop()->BelongsToCurrentThread()); - - if (state == SignalStrategy::CONNECTED) { - LOG(INFO) << "Host connected as " << signal_strategy_->GetLocalJid(); - - for (StatusObserverList::iterator it = status_observers_.begin(); - it != status_observers_.end(); ++it) { - (*it)->OnSignallingConnected(signal_strategy_.get()); - } - } else if (state == SignalStrategy::DISCONNECTED) { - LOG(INFO) << "Host disconnected from talk network."; - for (StatusObserverList::iterator it = status_observers_.begin(); - it != status_observers_.end(); ++it) { - (*it)->OnSignallingDisconnected(); - } - } -} - void ChromotingHost::OnSessionManagerReady() { DCHECK(context_->network_message_loop()->BelongsToCurrentThread()); // Don't need to do anything here, just wait for incoming diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h index 5fede90..c9e1d7f 100644 --- a/remoting/host/chromoting_host.h +++ b/remoting/host/chromoting_host.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -62,17 +62,15 @@ class ScreenRecorder; // incoming connection. class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>, public ClientSession::EventHandler, - public SignalStrategy::Listener, public protocol::SessionManager::Listener { public: - // Factory methods that must be used to create ChromotingHost - // instances. It does NOT take ownership of |context|, and - // |environment|, but they should not be deleted until returned host - // is destroyed. - static ChromotingHost* Create(ChromotingHostContext* context, - MutableHostConfig* config, - DesktopEnvironment* environment, - bool allow_nat_traversal); + // The caller must ensure that |context|, |signal_strategy| and + // |environment| out-live the host. + ChromotingHost(ChromotingHostContext* context, + MutableHostConfig* config, + SignalStrategy* signal_strategy, + DesktopEnvironment* environment, + bool allow_nat_traversal); // Asynchronously start the host process. // @@ -98,11 +96,6 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>, void SetSharedSecret(const std::string& shared_secret); //////////////////////////////////////////////////////////////////////////// - // SignalStrategy::Listener interface. - virtual void OnSignalStrategyStateChange( - SignalStrategy::State state) OVERRIDE; - - //////////////////////////////////////////////////////////////////////////// // ClientSession::EventHandler implementation. virtual void OnSessionAuthenticated(ClientSession* client) OVERRIDE; virtual void OnSessionAuthenticationFailed(ClientSession* client) OVERRIDE; @@ -152,11 +145,6 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>, kStopped, }; - // Caller keeps ownership of |context| and |environment|. - ChromotingHost(ChromotingHostContext* context, - MutableHostConfig* config, - DesktopEnvironment* environment, - bool allow_nat_traversal); virtual ~ChromotingHost(); // Creates encoder for the specified configuration. @@ -196,8 +184,7 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>, bool have_shared_secret_; // Connection objects. - scoped_ptr<SignalStrategy> signal_strategy_; - std::string local_jid_; + SignalStrategy* signal_strategy_; scoped_ptr<protocol::SessionManager> session_manager_; // StatusObserverList is thread-safe and can be used on any thread. diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc index 7ce925b..9e210a6 100644 --- a/remoting/host/chromoting_host_unittest.cc +++ b/remoting/host/chromoting_host_unittest.cc @@ -1,10 +1,11 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include "base/bind.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop_proxy.h" +#include "remoting/jingle_glue/mock_objects.h" #include "remoting/host/capturer_fake.h" #include "remoting/host/chromoting_host.h" #include "remoting/host/chromoting_host_context.h" @@ -92,8 +93,9 @@ class ChromotingHostTest : public testing::Test { desktop_environment_.reset( new DesktopEnvironment(&context_, capturer, event_executor_)); - host_ = ChromotingHost::Create( - &context_, config_,desktop_environment_.get(), false); + host_ = new ChromotingHost( + &context_, config_, &signal_strategy_, + desktop_environment_.get(), false); disconnect_window_ = new MockDisconnectWindow(); continue_window_ = new MockContinueWindow(); @@ -218,6 +220,7 @@ class ChromotingHostTest : public testing::Test { MessageLoop message_loop_; scoped_refptr<base::MessageLoopProxy> message_loop_proxy_; MockConnectionToClientEventHandler handler_; + MockSignalStrategy signal_strategy_; scoped_ptr<DesktopEnvironment> desktop_environment_; scoped_ptr<It2MeHostUserInterface> it2me_host_user_interface_; scoped_refptr<ChromotingHost> host_; diff --git a/remoting/host/heartbeat_sender.cc b/remoting/host/heartbeat_sender.cc index 2a7d8ac0..27ac108 100644 --- a/remoting/host/heartbeat_sender.cc +++ b/remoting/host/heartbeat_sender.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -34,68 +34,61 @@ const char kSetIntervalTag[] = "set-interval"; const int64 kDefaultHeartbeatIntervalMs = 5 * 60 * 1000; // 5 minutes. } -HeartbeatSender::HeartbeatSender(base::MessageLoopProxy* message_loop, - MutableHostConfig* config) - +HeartbeatSender::HeartbeatSender() : state_(CREATED), - message_loop_(message_loop), - config_(config), + signal_strategy_(NULL), interval_ms_(kDefaultHeartbeatIntervalMs) { - DCHECK(config_); } HeartbeatSender::~HeartbeatSender() { - DCHECK(state_ == CREATED || state_ == INITIALIZED || state_ == STOPPED); + if (signal_strategy_) + signal_strategy_->RemoveListener(this); } -bool HeartbeatSender::Init() { +bool HeartbeatSender::Init(SignalStrategy* signal_strategy, + MutableHostConfig* config) { DCHECK(state_ == CREATED); - if (!config_->GetString(kHostIdConfigPath, &host_id_)) { + if (!config->GetString(kHostIdConfigPath, &host_id_)) { LOG(ERROR) << "host_id is not defined in the config."; return false; } - if (!key_pair_.Load(config_)) { + if (!key_pair_.Load(config)) { return false; } - state_ = INITIALIZED; - - return true; -} - -void HeartbeatSender::OnSignallingConnected(SignalStrategy* signal_strategy) { - DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK(state_ == INITIALIZED || state_ == STOPPED); - state_ = STARTED; + DCHECK(signal_strategy); + signal_strategy_ = signal_strategy; + signal_strategy_->AddListener(this); - full_jid_ = signal_strategy->GetLocalJid(); + state_ = INITIALIZED; - iq_sender_.reset(new IqSender(signal_strategy)); + // Update the state if the |signal_strategy_| is already connected. + OnSignalStrategyStateChange(signal_strategy_->GetState()); - DoSendStanza(); - timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(interval_ms_), this, - &HeartbeatSender::DoSendStanza); + return true; } -void HeartbeatSender::OnSignallingDisconnected() { - DCHECK(message_loop_->BelongsToCurrentThread()); - state_ = STOPPED; - request_.reset(); - iq_sender_.reset(); - timer_.Stop(); +void HeartbeatSender::OnSignalStrategyStateChange(SignalStrategy::State state) { + if (state == SignalStrategy::CONNECTED) { + DCHECK(state_ == INITIALIZED || state_ == STOPPED); + state_ = STARTED; + + iq_sender_.reset(new IqSender(signal_strategy_)); + + DoSendStanza(); + timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(interval_ms_), + this, &HeartbeatSender::DoSendStanza); + } else if (state == SignalStrategy::DISCONNECTED) { + state_ = STOPPED; + request_.reset(); + iq_sender_.reset(); + timer_.Stop(); + } } -// Ignore any notifications other than signalling -// connected/disconnected events. -void HeartbeatSender::OnAccessDenied() { } -void HeartbeatSender::OnClientAuthenticated(const std::string& jid) { } -void HeartbeatSender::OnClientDisconnected(const std::string& jid) { } -void HeartbeatSender::OnShutdown() { } - void HeartbeatSender::DoSendStanza() { - DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK_EQ(state_, STARTED); VLOG(1) << "Sending heartbeat stanza to " << kChromotingBotJid; @@ -106,8 +99,6 @@ void HeartbeatSender::DoSendStanza() { } void HeartbeatSender::ProcessResponse(const XmlElement* response) { - DCHECK(message_loop_->BelongsToCurrentThread()); - std::string type = response->Attr(buzz::QN_TYPE); if (type == buzz::STR_ERROR) { LOG(ERROR) << "Received error in response to heartbeat: " @@ -167,7 +158,7 @@ XmlElement* HeartbeatSender::CreateSignature() { signature_tag->AddAttr( QName(kChromotingXmlNamespace, kSignatureTimeAttr), time_str); - std::string message = full_jid_ + ' ' + time_str; + std::string message = signal_strategy_->GetLocalJid() + ' ' + time_str; std::string signature(key_pair_.GetSignature(message)); signature_tag->AddText(signature); diff --git a/remoting/host/heartbeat_sender.h b/remoting/host/heartbeat_sender.h index 63c9576..f50e156 100644 --- a/remoting/host/heartbeat_sender.h +++ b/remoting/host/heartbeat_sender.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,12 +8,12 @@ #include <string> #include "base/compiler_specific.h" +#include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/timer.h" #include "remoting/host/host_key_pair.h" -#include "remoting/host/host_status_observer.h" -#include "base/gtest_prod_util.h" +#include "remoting/jingle_glue/signal_strategy.h" namespace base { class MessageLoopProxy; @@ -62,25 +62,20 @@ class MutableHostConfig; // optional. Host uses default heartbeat interval if it doesn't find // set-interval tag in the result Iq stanza it receives from the // server. -// -// TODO(sergeyu): Is it enough to sign JID and nothing else? -class HeartbeatSender : public HostStatusObserver { +class HeartbeatSender : public SignalStrategy::Listener { public: - HeartbeatSender(base::MessageLoopProxy* main_loop, - MutableHostConfig* config); + HeartbeatSender(); virtual ~HeartbeatSender(); - // Initializes heart-beating for |jingle_client_| with |config_|. Returns - // false if the config is invalid (e.g. private key cannot be parsed). - bool Init(); + // Initializes the HeartbeatSender. Returns false if the |config| is + // invalid (e.g. private key cannot be parsed). SignalStrategy must + // outlive this object. Heartbeats will start when the supplied + // SignalStrategy enters the CONNECTED state. + bool Init(SignalStrategy* signal_strategy, MutableHostConfig* config); - // HostStatusObserver implementation. - virtual void OnSignallingConnected(SignalStrategy* signal_strategy) OVERRIDE; - virtual void OnSignallingDisconnected() OVERRIDE; - virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE; - virtual void OnClientDisconnected(const std::string& jid) OVERRIDE; - virtual void OnAccessDenied() OVERRIDE; - virtual void OnShutdown() OVERRIDE; + // SignalStrategy::Listener interface. + virtual void OnSignalStrategyStateChange( + SignalStrategy::State state) OVERRIDE; private: FRIEND_TEST_ALL_PREFIXES(HeartbeatSenderTest, DoSendStanza); @@ -104,11 +99,9 @@ class HeartbeatSender : public HostStatusObserver { buzz::XmlElement* CreateSignature(); State state_; - scoped_refptr<base::MessageLoopProxy> message_loop_; - scoped_refptr<MutableHostConfig> config_; + SignalStrategy* signal_strategy_; std::string host_id_; HostKeyPair key_pair_; - std::string full_jid_; scoped_ptr<IqSender> iq_sender_; scoped_ptr<IqRequest> request_; int interval_ms_; diff --git a/remoting/host/heartbeat_sender_unittest.cc b/remoting/host/heartbeat_sender_unittest.cc index b1de98b..854a6a6 100644 --- a/remoting/host/heartbeat_sender_unittest.cc +++ b/remoting/host/heartbeat_sender_unittest.cc @@ -1,9 +1,11 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include "remoting/host/heartbeat_sender.h" +#include <set> + #include "base/memory/ref_counted.h" #include "base/message_loop.h" #include "base/message_loop_proxy.h" @@ -39,30 +41,49 @@ const int64 kTestTime = 123123123; const char kStanzaId[] = "123"; } // namespace +ACTION_P(AddListener, list) { + list->insert(arg0); +} +ACTION_P(RemoveListener, list) { + EXPECT_TRUE(list->find(arg0) != list->end()); + list->erase(arg0); +} + class HeartbeatSenderTest : public testing::Test { protected: - virtual void SetUp() { + virtual void SetUp() OVERRIDE { config_ = new InMemoryHostConfig(); config_->SetString(kHostIdConfigPath, kHostId); config_->SetString(kPrivateKeyConfigPath, kTestHostKeyPair); + + EXPECT_CALL(signal_strategy_, GetState()) + .WillOnce(Return(SignalStrategy::DISCONNECTED)); + EXPECT_CALL(signal_strategy_, AddListener(NotNull())) + .WillRepeatedly(AddListener(&signal_strategy_listeners_)); + EXPECT_CALL(signal_strategy_, RemoveListener(NotNull())) + .WillRepeatedly(RemoveListener(&signal_strategy_listeners_)); + EXPECT_CALL(signal_strategy_, GetLocalJid()) + .WillRepeatedly(Return(kTestJid)); + + heartbeat_sender_.reset(new HeartbeatSender()); + ASSERT_TRUE(heartbeat_sender_->Init(&signal_strategy_, config_)); + } + + virtual void TearDown() OVERRIDE { + heartbeat_sender_.reset(); + EXPECT_TRUE(signal_strategy_listeners_.empty()); } - MockSignalStrategy signal_strategy_; MessageLoop message_loop_; + MockSignalStrategy signal_strategy_; + std::set<SignalStrategy::Listener*> signal_strategy_listeners_; scoped_refptr<InMemoryHostConfig> config_; + scoped_ptr<HeartbeatSender> heartbeat_sender_; }; // Call Start() followed by Stop(), and makes sure an Iq stanza is // being sent. TEST_F(HeartbeatSenderTest, DoSendStanza) { - SignalStrategy::Listener* listener; - EXPECT_CALL(signal_strategy_, AddListener(NotNull())) - .WillOnce(SaveArg<0>(&listener)); - - scoped_ptr<HeartbeatSender> heartbeat_sender( - new HeartbeatSender(base::MessageLoopProxy::current(), config_)); - ASSERT_TRUE(heartbeat_sender->Init()); - XmlElement* sent_iq = NULL; EXPECT_CALL(signal_strategy_, GetLocalJid()) .WillRepeatedly(Return(kTestJid)); @@ -71,7 +92,7 @@ TEST_F(HeartbeatSenderTest, DoSendStanza) { EXPECT_CALL(signal_strategy_, SendStanza(NotNull())) .WillOnce(DoAll(SaveArg<0>(&sent_iq), Return(true))); - heartbeat_sender->OnSignallingConnected(&signal_strategy_); + heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED); message_loop_.RunAllPending(); scoped_ptr<XmlElement> stanza(sent_iq); @@ -81,23 +102,15 @@ TEST_F(HeartbeatSenderTest, DoSendStanza) { std::string(kChromotingBotJid)); EXPECT_EQ(stanza->Attr(buzz::QName("", "type")), "set"); - EXPECT_CALL(signal_strategy_, RemoveListener(listener)); - - heartbeat_sender->OnSignallingDisconnected(); + heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED); message_loop_.RunAllPending(); } // Validate format of the heartbeat stanza. TEST_F(HeartbeatSenderTest, CreateHeartbeatMessage) { - scoped_ptr<HeartbeatSender> heartbeat_sender( - new HeartbeatSender(base::MessageLoopProxy::current(), - config_)); - ASSERT_TRUE(heartbeat_sender->Init()); - int64 start_time = static_cast<int64>(base::Time::Now().ToDoubleT()); - heartbeat_sender->full_jid_ = kTestJid; - scoped_ptr<XmlElement> stanza(heartbeat_sender->CreateHeartbeatMessage()); + scoped_ptr<XmlElement> stanza(heartbeat_sender_->CreateHeartbeatMessage()); ASSERT_TRUE(stanza.get() != NULL); EXPECT_TRUE(QName(kChromotingXmlNamespace, "heartbeat") == @@ -141,12 +154,9 @@ TEST_F(HeartbeatSenderTest, ProcessResponse) { const int kTestInterval = 123; set_interval->AddText(base::IntToString(kTestInterval)); - scoped_ptr<HeartbeatSender> heartbeat_sender( - new HeartbeatSender(base::MessageLoopProxy::current(), - config_)); - heartbeat_sender->ProcessResponse(response.get()); + heartbeat_sender_->ProcessResponse(response.get()); - EXPECT_EQ(kTestInterval * 1000, heartbeat_sender->interval_ms_); + EXPECT_EQ(kTestInterval * 1000, heartbeat_sender_->interval_ms_); } } // namespace remoting diff --git a/remoting/host/host_status_observer.h b/remoting/host/host_status_observer.h index cb5574d..c11f4c7 100644 --- a/remoting/host/host_status_observer.h +++ b/remoting/host/host_status_observer.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -17,10 +17,6 @@ class HostStatusObserver { HostStatusObserver() { } virtual ~HostStatusObserver() { } - // Called when status of the signalling channel changes. - virtual void OnSignallingConnected(SignalStrategy* signal_strategy) = 0; - virtual void OnSignallingDisconnected() = 0; - // Called when an unauthorized user attempts to connect to the host. virtual void OnAccessDenied() = 0; diff --git a/remoting/host/it2me_host_user_interface.cc b/remoting/host/it2me_host_user_interface.cc index 857eaca..3e7da8f 100644 --- a/remoting/host/it2me_host_user_interface.cc +++ b/remoting/host/it2me_host_user_interface.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -52,13 +52,6 @@ void It2MeHostUserInterface::InitFrom(DisconnectWindow* disconnect_window, local_input_monitor_.reset(monitor); } -void It2MeHostUserInterface::OnSignallingConnected( - SignalStrategy* signal_strategy) { -} - -void It2MeHostUserInterface::OnSignallingDisconnected() { -} - void It2MeHostUserInterface::OnClientAuthenticated(const std::string& jid) { // There should not be more than one concurrent authenticated connection. DCHECK(authenticated_jid_.empty()); diff --git a/remoting/host/it2me_host_user_interface.h b/remoting/host/it2me_host_user_interface.h index 5450175..eba9ec0 100644 --- a/remoting/host/it2me_host_user_interface.h +++ b/remoting/host/it2me_host_user_interface.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -42,8 +42,6 @@ class It2MeHostUserInterface : public HostStatusObserver { // HostStatusObserver implementation. These methods will be called from the // network thread. - virtual void OnSignallingConnected(SignalStrategy* signal_strategy) OVERRIDE; - virtual void OnSignallingDisconnected() OVERRIDE; virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE; virtual void OnClientDisconnected(const std::string& jid) OVERRIDE; virtual void OnAccessDenied() OVERRIDE; diff --git a/remoting/host/log_to_server.cc b/remoting/host/log_to_server.cc index 082cb13..3d4dffc 100644 --- a/remoting/host/log_to_server.cc +++ b/remoting/host/log_to_server.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -7,6 +7,7 @@ #include "base/bind.h" #include "base/message_loop_proxy.h" #include "remoting/base/constants.h" +#include "remoting/host/chromoting_host.h" #include "remoting/host/server_log_entry.h" #include "remoting/jingle_glue/iq_sender.h" #include "remoting/jingle_glue/jingle_thread.h" @@ -23,11 +24,13 @@ namespace { const char kLogCommand[] = "log"; } // namespace -LogToServer::LogToServer(base::MessageLoopProxy* message_loop) - : message_loop_(message_loop) { +LogToServer::LogToServer(SignalStrategy* signal_strategy) + : signal_strategy_(signal_strategy) { + signal_strategy_->AddListener(this); } LogToServer::~LogToServer() { + signal_strategy_->RemoveListener(this); } void LogToServer::LogSessionStateChange(bool connected) { @@ -37,18 +40,13 @@ void LogToServer::LogSessionStateChange(bool connected) { Log(*entry.get()); } -void LogToServer::OnSignallingConnected(SignalStrategy* signal_strategy) { - DCHECK(message_loop_->BelongsToCurrentThread()); - iq_sender_.reset(new IqSender(signal_strategy)); - SendPendingEntries(); -} - -void LogToServer::OnSignallingDisconnected() { - DCHECK(message_loop_->BelongsToCurrentThread()); - iq_sender_.reset(); -} - -void LogToServer::OnAccessDenied() { +void LogToServer::OnSignalStrategyStateChange(SignalStrategy::State state) { + if (state == SignalStrategy::CONNECTED) { + iq_sender_.reset(new IqSender(signal_strategy_)); + SendPendingEntries(); + } else if (state == SignalStrategy::DISCONNECTED) { + iq_sender_.reset(); + } } void LogToServer::OnClientAuthenticated(const std::string& jid) { @@ -59,18 +57,18 @@ void LogToServer::OnClientDisconnected(const std::string& jid) { LogSessionStateChange(false); } +void LogToServer::OnAccessDenied() { +} + void LogToServer::OnShutdown() { } void LogToServer::Log(const ServerLogEntry& entry) { - DCHECK(message_loop_->BelongsToCurrentThread()); pending_entries_.push_back(entry); SendPendingEntries(); } void LogToServer::SendPendingEntries() { - DCHECK(message_loop_->BelongsToCurrentThread()); - if (iq_sender_ == NULL) { return; } diff --git a/remoting/host/log_to_server.h b/remoting/host/log_to_server.h index 1cf27cf..9dea354 100644 --- a/remoting/host/log_to_server.h +++ b/remoting/host/log_to_server.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -11,6 +11,7 @@ #include "base/memory/scoped_ptr.h" #include "remoting/host/host_status_observer.h" #include "remoting/host/server_log_entry.h" +#include "remoting/jingle_glue/signal_strategy.h" namespace base { class MessageLoopProxy; @@ -22,25 +23,25 @@ class XmlElement; namespace remoting { +class ChromotingHost; class IqSender; -/** - * A class that sends log entries to a server. - * - * The class is not thread-safe. - */ -class LogToServer : public HostStatusObserver { +// LogToServer sends log entries to a server. +class LogToServer : public HostStatusObserver, + public SignalStrategy::Listener { public: - explicit LogToServer(base::MessageLoopProxy* message_loop); + explicit LogToServer(SignalStrategy* signal_strategy); virtual ~LogToServer(); // Logs a session state change. // Currently, this is either connection or disconnection. void LogSessionStateChange(bool connected); - // HostStatusObserver implementation. - virtual void OnSignallingConnected(SignalStrategy* signal_strategy) OVERRIDE; - virtual void OnSignallingDisconnected() OVERRIDE; + // SignalStrategy::Listener interface. + virtual void OnSignalStrategyStateChange( + SignalStrategy::State state) OVERRIDE; + + // HostStatusObserver interface. virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE; virtual void OnClientDisconnected(const std::string& jid) OVERRIDE; virtual void OnAccessDenied() OVERRIDE; @@ -50,7 +51,8 @@ class LogToServer : public HostStatusObserver { void Log(const ServerLogEntry& entry); void SendPendingEntries(); - scoped_refptr<base::MessageLoopProxy> message_loop_; + scoped_refptr<ChromotingHost> host_; + SignalStrategy* signal_strategy_; scoped_ptr<IqSender> iq_sender_; std::deque<ServerLogEntry> pending_entries_; diff --git a/remoting/host/log_to_server_unittest.cc b/remoting/host/log_to_server_unittest.cc index 0749163..60e7c2f5 100644 --- a/remoting/host/log_to_server_unittest.cc +++ b/remoting/host/log_to_server_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -31,14 +31,16 @@ class LogToServerTest : public testing::Test { LogToServerTest() {} virtual void SetUp() OVERRIDE { message_loop_proxy_ = base::MessageLoopProxy::current(); - log_to_server_.reset(new LogToServer(message_loop_proxy_)); + EXPECT_CALL(signal_strategy_, AddListener(_)); + log_to_server_.reset(new LogToServer(&signal_strategy_)); + EXPECT_CALL(signal_strategy_, RemoveListener(_)); } protected: - scoped_refptr<base::MessageLoopProxy> message_loop_proxy_; - scoped_ptr<LogToServer> log_to_server_; MessageLoop message_loop_; + scoped_refptr<base::MessageLoopProxy> message_loop_proxy_; MockSignalStrategy signal_strategy_; + scoped_ptr<LogToServer> log_to_server_; }; TEST_F(LogToServerTest, SendNow) { @@ -54,13 +56,14 @@ TEST_F(LogToServerTest, SendNow) { .WillOnce(QuitMainMessageLoop(&message_loop_)) .RetiresOnSaturation(); } - log_to_server_->OnSignallingConnected(&signal_strategy_); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED); log_to_server_->OnClientAuthenticated("client@domain.com/5678"); - log_to_server_->OnSignallingDisconnected(); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED); message_loop_.Run(); } TEST_F(LogToServerTest, SendLater) { + log_to_server_->OnClientAuthenticated("client@domain.com/5678"); { InSequence s; EXPECT_CALL(signal_strategy_, GetLocalJid()) @@ -73,13 +76,14 @@ TEST_F(LogToServerTest, SendLater) { .WillOnce(QuitMainMessageLoop(&message_loop_)) .RetiresOnSaturation(); } - log_to_server_->OnClientAuthenticated("client@domain.com/5678"); - log_to_server_->OnSignallingConnected(&signal_strategy_); - log_to_server_->OnSignallingDisconnected(); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED); message_loop_.Run(); } TEST_F(LogToServerTest, SendTwoEntriesLater) { + log_to_server_->OnClientAuthenticated("client@domain.com/5678"); + log_to_server_->OnClientAuthenticated("client2@domain.com/6789"); { InSequence s; EXPECT_CALL(signal_strategy_, GetLocalJid()) @@ -92,10 +96,8 @@ TEST_F(LogToServerTest, SendTwoEntriesLater) { .WillOnce(QuitMainMessageLoop(&message_loop_)) .RetiresOnSaturation(); } - log_to_server_->OnClientAuthenticated("client@domain.com/5678"); - log_to_server_->OnClientAuthenticated("client2@domain.com/6789"); - log_to_server_->OnSignallingConnected(&signal_strategy_); - log_to_server_->OnSignallingDisconnected(); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED); + log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED); message_loop_.Run(); } diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc index 9f3a559..50661f1 100644 --- a/remoting/host/plugin/host_script_object.cc +++ b/remoting/host/plugin/host_script_object.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -10,6 +10,7 @@ #include "base/sys_string_conversions.h" #include "base/threading/platform_thread.h" #include "base/utf_string_conversions.h" +#include "remoting/jingle_glue/xmpp_signal_strategy.h" #include "remoting/base/auth_token_util.h" #include "remoting/host/chromoting_host.h" #include "remoting/host/chromoting_host_context.h" @@ -337,13 +338,6 @@ bool HostNPScriptObject::Enumerate(std::vector<std::string>* values) { return true; } -void HostNPScriptObject::OnSignallingConnected( - SignalStrategy* signal_strategy) { -} - -void HostNPScriptObject::OnSignallingDisconnected() { -} - void HostNPScriptObject::OnAccessDenied() { DCHECK(host_context_.network_message_loop()->BelongsToCurrentThread()); @@ -475,11 +469,7 @@ void HostNPScriptObject::FinishConnect( return; } - // Store the supplied user ID and token to the Host configuration. scoped_refptr<MutableHostConfig> host_config = new InMemoryHostConfig(); - host_config->SetString(kXmppLoginConfigPath, uid); - host_config->SetString(kXmppAuthTokenConfigPath, auth_token); - host_config->SetString(kXmppAuthServiceConfigPath, auth_service); // Generate a key pair for the Host to use. // TODO(wez): Move this to the worker thread. @@ -487,10 +477,16 @@ void HostNPScriptObject::FinishConnect( host_key_pair.Generate(); host_key_pair.Save(host_config); + // Create and start XMPP connection. + scoped_ptr<SignalStrategy> signal_strategy( + new XmppSignalStrategy(host_context_.jingle_thread(), uid, + auth_token, auth_service)); + // Request registration of the host for support. scoped_ptr<RegisterSupportHostRequest> register_request( new RegisterSupportHostRequest()); if (!register_request->Init( + signal_strategy.get(), host_config.get(), base::Bind(&HostNPScriptObject::OnReceivedSupportID, base::Unretained(this)))) { @@ -507,17 +503,17 @@ void HostNPScriptObject::FinishConnect( // Beyond this point nothing can fail, so save the config and request. host_config_ = host_config; + signal_strategy_.reset(signal_strategy.release()); register_request_.reset(register_request.release()); // Create the Host. LOG(INFO) << "NAT state: " << nat_traversal_enabled_; - host_ = ChromotingHost::Create( - &host_context_, host_config_, desktop_environment_.get(), - nat_traversal_enabled_); + host_ = new ChromotingHost( + &host_context_, host_config_, signal_strategy_.get(), + desktop_environment_.get(), nat_traversal_enabled_); host_->AddStatusObserver(this); - host_->AddStatusObserver(register_request_.get()); if (enable_log_to_server_) { - log_to_server_.reset(new LogToServer(host_context_.network_message_loop())); + log_to_server_.reset(new LogToServer(signal_strategy_.get())); host_->AddStatusObserver(log_to_server_.get()); } host_->set_it2me(true); @@ -531,6 +527,11 @@ void HostNPScriptObject::FinishConnect( host_->SetUiStrings(ui_strings_); } + // Post a task to start XMPP connection. + host_context_.network_message_loop()->PostTask( + FROM_HERE, base::Bind(&SignalStrategy::Connect, + base::Unretained(signal_strategy_.get()))); + // Start the Host. host_->Start(); diff --git a/remoting/host/plugin/host_script_object.h b/remoting/host/plugin/host_script_object.h index a87b712..b40bfd3 100644 --- a/remoting/host/plugin/host_script_object.h +++ b/remoting/host/plugin/host_script_object.h @@ -68,9 +68,6 @@ class HostNPScriptObject : public HostStatusObserver { bool Enumerate(std::vector<std::string>* values); // remoting::HostStatusObserver implementation. - virtual void OnSignallingConnected( - remoting::SignalStrategy* signal_strategy) OVERRIDE; - virtual void OnSignallingDisconnected() OVERRIDE; virtual void OnAccessDenied() OVERRIDE; virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE; virtual void OnClientDisconnected(const std::string& jid) OVERRIDE; @@ -177,10 +174,11 @@ class HostNPScriptObject : public HostStatusObserver { base::PlatformThreadId np_thread_id_; scoped_refptr<PluginMessageLoopProxy> plugin_message_loop_proxy_; + ChromotingHostContext host_context_; + scoped_refptr<MutableHostConfig> host_config_; + scoped_ptr<SignalStrategy> signal_strategy_; scoped_ptr<RegisterSupportHostRequest> register_request_; scoped_ptr<LogToServer> log_to_server_; - scoped_refptr<MutableHostConfig> host_config_; - ChromotingHostContext host_context_; scoped_ptr<DesktopEnvironment> desktop_environment_; scoped_ptr<It2MeHostUserInterface> it2me_host_user_interface_; diff --git a/remoting/host/register_support_host_request.cc b/remoting/host/register_support_host_request.cc index 5668f27..63caadf 100644 --- a/remoting/host/register_support_host_request.cc +++ b/remoting/host/register_support_host_request.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -36,58 +36,49 @@ const char kSupportIdLifetimeTag[] = "support-id-lifetime"; } RegisterSupportHostRequest::RegisterSupportHostRequest() - : message_loop_(NULL) { + : signal_strategy_(NULL) { } RegisterSupportHostRequest::~RegisterSupportHostRequest() { + if (signal_strategy_) + signal_strategy_->RemoveListener(this); } -bool RegisterSupportHostRequest::Init(HostConfig* config, +bool RegisterSupportHostRequest::Init(SignalStrategy* signal_strategy, + HostConfig* config, const RegisterCallback& callback) { - callback_ = callback; - if (!key_pair_.Load(config)) { return false; } - return true; -} -void RegisterSupportHostRequest::OnSignallingConnected( - SignalStrategy* signal_strategy) { - DCHECK(!callback_.is_null()); - - message_loop_ = MessageLoop::current(); + callback_ = callback; + signal_strategy_ = signal_strategy; + signal_strategy_->AddListener(this); + iq_sender_.reset(new IqSender(signal_strategy_)); - iq_sender_.reset(new IqSender(signal_strategy)); - request_.reset(iq_sender_->SendIq( - buzz::STR_SET, kChromotingBotJid, - CreateRegistrationRequest(signal_strategy->GetLocalJid()), - base::Bind(&RegisterSupportHostRequest::ProcessResponse, - base::Unretained(this)))); + return true; } -void RegisterSupportHostRequest::OnSignallingDisconnected() { - if (!message_loop_) { - // We will reach here with |message_loop_| NULL if the Host's - // XMPP connection attempt fails. - CHECK(!callback_.is_null()); - DCHECK(!request_.get()); - callback_.Run(false, std::string(), base::TimeDelta()); - return; +void RegisterSupportHostRequest::OnSignalStrategyStateChange( + SignalStrategy::State state) { + if (state == SignalStrategy::CONNECTED) { + DCHECK(!callback_.is_null()); + + request_.reset(iq_sender_->SendIq( + buzz::STR_SET, kChromotingBotJid, + CreateRegistrationRequest(signal_strategy_->GetLocalJid()), + base::Bind(&RegisterSupportHostRequest::ProcessResponse, + base::Unretained(this)))); + } else if (state == SignalStrategy::DISCONNECTED) { + // We will reach here if signaling fails to connect. + CallCallback(false, std::string(), base::TimeDelta()); } - DCHECK_EQ(message_loop_, MessageLoop::current()); - request_.reset(); - iq_sender_.reset(); } -// Ignore any notifications other than signalling -// connected/disconnected events. -void RegisterSupportHostRequest::OnAccessDenied() { } -void RegisterSupportHostRequest::OnClientAuthenticated( - const std::string& jid) { } -void RegisterSupportHostRequest::OnClientDisconnected( - const std::string& jid) { } -void RegisterSupportHostRequest::OnShutdown() { } +bool RegisterSupportHostRequest::OnSignalStrategyIncomingStanza( + const buzz::XmlElement* stanza) { + return false; +} XmlElement* RegisterSupportHostRequest::CreateRegistrationRequest( const std::string& jid) { @@ -176,13 +167,24 @@ bool RegisterSupportHostRequest::ParseResponse(const XmlElement* response, return true; } - void RegisterSupportHostRequest::ProcessResponse(const XmlElement* response) { - DCHECK_EQ(message_loop_, MessageLoop::current()); std::string support_id; base::TimeDelta lifetime; bool success = ParseResponse(response, &support_id, &lifetime); - callback_.Run(success, support_id, lifetime); + CallCallback(success, support_id, lifetime); +} + +void RegisterSupportHostRequest::CallCallback( + bool success, const std::string& support_id, base::TimeDelta lifetime) { + // Cleanup state before calling the callback. + request_.reset(); + iq_sender_.reset(); + signal_strategy_->RemoveListener(this); + signal_strategy_ = NULL; + + RegisterCallback callback = callback_; + callback_.Reset(); + callback.Run(success, support_id, lifetime); } } // namespace remoting diff --git a/remoting/host/register_support_host_request.h b/remoting/host/register_support_host_request.h index 84dab32..16e8a61 100644 --- a/remoting/host/register_support_host_request.h +++ b/remoting/host/register_support_host_request.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -10,8 +10,8 @@ #include "base/callback.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "remoting/jingle_glue/signal_strategy.h" #include "remoting/host/host_key_pair.h" -#include "remoting/host/host_status_observer.h" #include "testing/gtest/include/gtest/gtest_prod.h" class MessageLoop; @@ -29,12 +29,11 @@ namespace remoting { class IqRequest; class IqSender; -// RegisterSupportHostRequest sends support host registeration request -// to the Chromoting Bot. It listens to the status of the host using -// HostStatusObserver interface and sends the request when signalling -// channel is connected. When a response is received from the bot, it -// calls the callback specified in the Init() method. -class RegisterSupportHostRequest : public HostStatusObserver { +// RegisterSupportHostRequest sends a request to register the host for +// a SupportID, as soon as the associated SignalStrategy becomes +// connected. When a response is received from the bot, it calls the +// callback specified in the Init() method. +class RegisterSupportHostRequest : public SignalStrategy::Listener { public: // First parameter is set to true on success. Second parameter is // the new SessionID received from the bot. Third parameter is the @@ -45,23 +44,22 @@ class RegisterSupportHostRequest : public HostStatusObserver { RegisterSupportHostRequest(); virtual ~RegisterSupportHostRequest(); - // Provide the configuration to use to register the host, and a - // callback to invoke when a registration response is received. - // |callback| is called when registration response is received from - // the server. Ownership of |callback| is given to the request - // object. Caller must ensure that the callback object is valid - // while signalling connection exists. Returns false on falure - // (e.g. config is invalid). Callback is never called if the bot - // malfunctions and doesn't respond to the request. - bool Init(HostConfig* config, const RegisterCallback& callback); + // Initializes the registration to use the |signal_startegy| and to + // notify |callback| upon completion or failure. Returns false on + // falure (e.g. config is invalid). Callback is never called if the + // bot malfunctions and doesn't respond to the request. + // + // TODO(sergeyu): This class should have timeout for the bot + // response. + bool Init(SignalStrategy* signal_strategy, + HostConfig* config, + const RegisterCallback& callback); // HostStatusObserver implementation. - virtual void OnSignallingConnected(SignalStrategy* signal_strategy) OVERRIDE; - virtual void OnSignallingDisconnected() OVERRIDE; - virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE; - virtual void OnClientDisconnected(const std::string& jid) OVERRIDE; - virtual void OnAccessDenied() OVERRIDE; - virtual void OnShutdown() OVERRIDE; + virtual void OnSignalStrategyStateChange( + SignalStrategy::State state) OVERRIDE; + virtual bool OnSignalStrategyIncomingStanza( + const buzz::XmlElement* stanza) OVERRIDE; private: void DoSend(); @@ -74,7 +72,10 @@ class RegisterSupportHostRequest : public HostStatusObserver { bool ParseResponse(const buzz::XmlElement* response, std::string* support_id, base::TimeDelta* lifetime); - MessageLoop* message_loop_; + void CallCallback( + bool success, const std::string& support_id, base::TimeDelta lifetime); + + SignalStrategy* signal_strategy_; RegisterCallback callback_; scoped_ptr<IqSender> iq_sender_; scoped_ptr<IqRequest> request_; diff --git a/remoting/host/register_support_host_request_unittest.cc b/remoting/host/register_support_host_request_unittest.cc index dc3258d..92eaef2 100644 --- a/remoting/host/register_support_host_request_unittest.cc +++ b/remoting/host/register_support_host_request_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -7,6 +7,7 @@ #include "base/bind.h" #include "base/memory/ref_counted.h" #include "base/message_loop.h" +#include "base/observer_list.h" #include "base/string_number_conversions.h" #include "remoting/base/constants.h" #include "remoting/host/host_key_pair.h" @@ -37,6 +38,13 @@ const char kSupportId[] = "AB4RF3"; const char kSupportIdLifetime[] = "300"; const char kStanzaId[] = "123"; +ACTION_P(AddListener, list) { + list->AddObserver(arg0); +} +ACTION_P(RemoveListener, list) { + list->RemoveObserver(arg0); +} + class MockCallback { public: MOCK_METHOD3(OnResponse, void(bool result, const std::string& support_id, @@ -51,37 +59,40 @@ class RegisterSupportHostRequestTest : public testing::Test { virtual void SetUp() { config_ = new InMemoryHostConfig(); config_->SetString(kPrivateKeyConfigPath, kTestHostKeyPair); + + EXPECT_CALL(signal_strategy_, AddListener(NotNull())) + .WillRepeatedly(AddListener(&signal_strategy_listeners_)); + EXPECT_CALL(signal_strategy_, RemoveListener(NotNull())) + .WillRepeatedly(RemoveListener(&signal_strategy_listeners_)); + EXPECT_CALL(signal_strategy_, GetLocalJid()) + .WillRepeatedly(Return(kTestJid)); } - MockSignalStrategy signal_strategy_; MessageLoop message_loop_; + MockSignalStrategy signal_strategy_; + ObserverList<SignalStrategy::Listener, true> signal_strategy_listeners_; scoped_refptr<InMemoryHostConfig> config_; MockCallback callback_; }; + TEST_F(RegisterSupportHostRequestTest, Send) { // |iq_request| is freed by RegisterSupportHostRequest. int64 start_time = static_cast<int64>(base::Time::Now().ToDoubleT()); - SignalStrategy::Listener* listener; - EXPECT_CALL(signal_strategy_, AddListener(NotNull())) - .WillOnce(SaveArg<0>(&listener)); - scoped_ptr<RegisterSupportHostRequest> request( new RegisterSupportHostRequest()); ASSERT_TRUE(request->Init( - config_, base::Bind(&MockCallback::OnResponse, - base::Unretained(&callback_)))); + &signal_strategy_, config_, base::Bind(&MockCallback::OnResponse, + base::Unretained(&callback_)))); XmlElement* sent_iq = NULL; EXPECT_CALL(signal_strategy_, GetNextId()) .WillOnce(Return(kStanzaId)); - EXPECT_CALL(signal_strategy_, GetLocalJid()) - .WillRepeatedly(Return(kTestJid)); EXPECT_CALL(signal_strategy_, SendStanza(NotNull())) .WillOnce(DoAll(SaveArg<0>(&sent_iq), Return(true))); - request->OnSignallingConnected(&signal_strategy_); + request->OnSignalStrategyStateChange(SignalStrategy::CONNECTED); message_loop_.RunAllPending(); // Verify format of the query. @@ -136,10 +147,17 @@ TEST_F(RegisterSupportHostRequestTest, Send) { support_id_lifetime->AddText(kSupportIdLifetime); result->AddElement(support_id_lifetime); - EXPECT_TRUE(listener->OnSignalStrategyIncomingStanza(response.get())); - message_loop_.RunAllPending(); + int consumed = 0; + ObserverListBase<SignalStrategy::Listener>::Iterator it( + signal_strategy_listeners_); + SignalStrategy::Listener* listener; + while ((listener = it.GetNext()) != NULL) { + if (listener->OnSignalStrategyIncomingStanza(response.get())) + consumed++; + } + EXPECT_EQ(1, consumed); - EXPECT_CALL(signal_strategy_, RemoveListener(listener)); + message_loop_.RunAllPending(); } } // namespace remoting diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc index 8a10c7b..c37fc71 100644 --- a/remoting/host/remoting_me2me_host.cc +++ b/remoting/host/remoting_me2me_host.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // @@ -29,6 +29,7 @@ #include "remoting/host/heartbeat_sender.h" #include "remoting/host/host_config.h" #include "remoting/host/json_host_config.h" +#include "remoting/jingle_glue/xmpp_signal_strategy.h" #if defined(TOOLKIT_USES_GTK) #include "ui/gfx/gtk_util.h" @@ -91,22 +92,50 @@ class HostProcess { return 1; } + // Use an XMPP connection to the Talk network for session signalling. + std::string xmpp_login; + std::string xmpp_auth_token; + if (!auth_config_->GetString(kXmppLoginConfigPath, &xmpp_login) || + !auth_config_->GetString(kXmppAuthTokenConfigPath, &xmpp_auth_token)) { + LOG(ERROR) << "XMPP credentials are not defined in the config."; + return 1; + } + + std::string xmpp_auth_service; + if (!auth_config_->GetString(remoting::kXmppAuthServiceConfigPath, + &xmpp_auth_service)) { + // For the me2me host, we assume we use the ClientLogin token for + // chromiumsync because we do not have an HTTP stack with which we can + // easily request an OAuth2 access token even if we had a RefreshToken for + // the account. + xmpp_auth_service = remoting::kChromotingTokenDefaultServiceName; + } + + // Create and start XMPP connection. + scoped_ptr<SignalStrategy> signal_strategy( + new XmppSignalStrategy(context.jingle_thread(), xmpp_login, + xmpp_auth_token, xmpp_auth_service)); + // Create the DesktopEnvironment and ChromotingHost. scoped_ptr<DesktopEnvironment> desktop_environment( DesktopEnvironment::Create(&context)); - host_ = ChromotingHost::Create( - &context, host_config_, desktop_environment.get(), false); + host_ = new ChromotingHost( + &context, host_config_, signal_strategy.get(), + desktop_environment.get(), false); // Initialize HeartbeatSender. scoped_ptr<remoting::HeartbeatSender> heartbeat_sender( - new remoting::HeartbeatSender(context.network_message_loop(), - host_config_)); - if (!heartbeat_sender->Init()) { + new remoting::HeartbeatSender()); + if (!heartbeat_sender->Init(signal_strategy.get(), host_config_)) { context.Stop(); return 1; } - host_->AddStatusObserver(heartbeat_sender.get()); + + // Post a task to start XMPP connection. + context.network_message_loop()->PostTask( + FROM_HERE, base::Bind(&remoting::SignalStrategy::Connect, + base::Unretained(signal_strategy.get()))); // Run the ChromotingHost until the shutdown task is executed. host_->Start(); @@ -134,13 +163,13 @@ class HostProcess { bool LoadConfig(base::MessageLoopProxy* message_loop_proxy) { host_config_ = new remoting::JsonHostConfig(host_config_path_, message_loop_proxy); - scoped_refptr<remoting::JsonHostConfig> auth_config = + auth_config_ = new remoting::JsonHostConfig(auth_config_path_, message_loop_proxy); std::string failed_path; if (!host_config_->Read()) { failed_path = host_config_path_.value(); - } else if (!auth_config->Read()) { + } else if (!auth_config_->Read()) { failed_path = auth_config_path_.value(); } if (!failed_path.empty()) { @@ -148,25 +177,13 @@ class HostProcess { return false; } - // Copy the needed keys from |auth_config| into |host_config|. - std::string value; - auth_config->GetString(kXmppAuthTokenConfigPath, &value); - host_config_->SetString(kXmppAuthTokenConfigPath, value); - auth_config->GetString(kXmppLoginConfigPath, &value); - host_config_->SetString(kXmppLoginConfigPath, value); - - // For the Me2Me host, we assume we always use the ClientLogin token for - // chromiumsync because we do not have an HTTP stack with which we can - // easily request an OAuth2 access token even if we had a RefreshToken for - // the account. - host_config_->SetString(kXmppAuthServiceConfigPath, - kChromotingTokenDefaultServiceName); return true; } FilePath auth_config_path_; FilePath host_config_path_; + scoped_refptr<remoting::JsonHostConfig> auth_config_; scoped_refptr<remoting::JsonHostConfig> host_config_; scoped_refptr<ChromotingHost> host_; diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc index 3ab83ee..9926a36 100644 --- a/remoting/host/simple_host_process.cc +++ b/remoting/host/simple_host_process.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // @@ -42,6 +42,7 @@ #include "remoting/host/log_to_server.h" #include "remoting/host/json_host_config.h" #include "remoting/host/register_support_host_request.h" +#include "remoting/jingle_glue/xmpp_signal_strategy.h" #include "remoting/proto/video.pb.h" #if defined(TOOLKIT_USES_GTK) @@ -56,8 +57,6 @@ HMODULE g_hModule = NULL; using remoting::ChromotingHost; using remoting::DesktopEnvironment; -using remoting::kChromotingTokenDefaultServiceName; -using remoting::kXmppAuthServiceConfigPath; using remoting::It2MeHostUserInterface; using remoting::protocol::CandidateSessionConfig; using remoting::protocol::ChannelConfig; @@ -98,10 +97,6 @@ class SimpleHost { // It needs to be a UI message loop to keep runloops spinning on the Mac. MessageLoop message_loop(MessageLoop::TYPE_UI); - remoting::ChromotingHostContext context( - base::MessageLoopProxy::current()); - context.Start(); - base::Thread file_io_thread("FileIO"); file_io_thread.Start(); @@ -112,36 +107,37 @@ class SimpleHost { if (!config->Read()) { LOG(ERROR) << "Failed to read configuration file " << config_path.value(); - context.Stop(); return 1; } - // For the simple host, we assume we always use the ClientLogin token for - // chromiumsync because we do not have an HTTP stack with which we can - // easily request an OAuth2 access token even if we had a RefreshToken for - // the account. - config->SetString(kXmppAuthServiceConfigPath, - kChromotingTokenDefaultServiceName); - - // Initialize AccessVerifier. - // TODO(jamiewalch): For the IT2Me case, the access verifier is passed to - // RegisterSupportHostRequest::Init, so transferring ownership of it to the - // ChromotingHost could cause a crash condition if SetIT2MeAccessCode is - // called after the ChromotingHost is destroyed (for example, at shutdown). - // Fix this. - scoped_ptr<remoting::RegisterSupportHostRequest> register_request; - scoped_ptr<remoting::HeartbeatSender> heartbeat_sender; - scoped_ptr<remoting::LogToServer> log_to_server; - if (is_it2me_) { - register_request.reset(new remoting::RegisterSupportHostRequest()); - if (!register_request->Init( - config, base::Bind(&SimpleHost::SetIT2MeAccessCode, - base::Unretained(this)))) { - return 1; - } + remoting::ChromotingHostContext context( + base::MessageLoopProxy::current()); + context.Start(); + + // Use an XMPP connection to the Talk network for session signalling. + std::string xmpp_login; + std::string xmpp_auth_token; + if (!config->GetString(remoting::kXmppLoginConfigPath, &xmpp_login) || + !config->GetString(remoting::kXmppAuthTokenConfigPath, + &xmpp_auth_token)) { + LOG(ERROR) << "XMPP credentials are not defined in the config."; + return 1; + } + std::string xmpp_auth_service; + if (!config->GetString(remoting::kXmppAuthServiceConfigPath, + &xmpp_auth_service)) { + // For the simple host, we assume we always use the ClientLogin token for + // chromiumsync because we do not have an HTTP stack with which we can + // easily request an OAuth2 access token even if we had a RefreshToken for + // the account. + xmpp_auth_service = remoting::kChromotingTokenDefaultServiceName; } - log_to_server.reset(new remoting::LogToServer( - context.network_message_loop())); + + // Create and start XMPP connection. + scoped_ptr<remoting::SignalStrategy> signal_strategy( + new remoting::XmppSignalStrategy(context.jingle_thread(), xmpp_login, + xmpp_auth_token, xmpp_auth_service)); + // Construct a chromoting host. scoped_ptr<DesktopEnvironment> desktop_environment; @@ -157,10 +153,14 @@ class SimpleHost { desktop_environment.reset(DesktopEnvironment::Create(&context)); } - host_ = ChromotingHost::Create( - &context, config, desktop_environment.get(), false); + host_ = new ChromotingHost(&context, config, signal_strategy.get(), + desktop_environment.get(), false); host_->set_it2me(is_it2me_); + scoped_ptr<remoting::LogToServer> log_to_server; + log_to_server.reset(new remoting::LogToServer(signal_strategy.get())); + host_->AddStatusObserver(log_to_server.get()); + scoped_ptr<It2MeHostUserInterface> it2me_host_user_interface; if (is_it2me_) { it2me_host_user_interface.reset(new It2MeHostUserInterface(host_, @@ -173,18 +173,27 @@ class SimpleHost { host_->set_protocol_config(protocol_config_.release()); } + scoped_ptr<remoting::RegisterSupportHostRequest> register_request; + scoped_ptr<remoting::HeartbeatSender> heartbeat_sender; if (is_it2me_) { - host_->AddStatusObserver(register_request.get()); + register_request.reset(new remoting::RegisterSupportHostRequest()); + if (!register_request->Init(signal_strategy.get(), config, + base::Bind(&SimpleHost::SetIT2MeAccessCode, + base::Unretained(this)))) { + return 1; + } } else { // Initialize HeartbeatSender. heartbeat_sender.reset( - new remoting::HeartbeatSender(context.network_message_loop(), - config)); - if (!heartbeat_sender->Init()) + new remoting::HeartbeatSender()); + if (!heartbeat_sender->Init(signal_strategy.get(), config)) return 1; - host_->AddStatusObserver(heartbeat_sender.get()); } - host_->AddStatusObserver(log_to_server.get()); + + // Post a task to start XMPP connection. + context.network_message_loop()->PostTask( + FROM_HERE, base::Bind(&remoting::SignalStrategy::Connect, + base::Unretained(signal_strategy.get()))); // Let the chromoting host run until the shutdown task is executed. host_->Start(); |