Made the ChromotingHost class not ref-counted.

ChromotingHost becomes a non thread-safe class should should live on the network thread. The CL removes ChromotingHost::Shutdown() allowing ChromotingHost to be destroyed synchronously closing all existing connection.

Review URL: https://chromiumcodereview.appspot.com/14348042

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

1 files changed, 35 insertions, 106 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 0d3810b..0a365e1 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -76,14 +76,14 @@ ChromotingHost::ChromotingHost(
       network_task_runner_(network_task_runner),
       ui_task_runner_(ui_task_runner),
       signal_strategy_(signal_strategy),
-      state_(kInitial),
+      started_(false),
       protocol_config_(protocol::CandidateSessionConfig::CreateDefault()),
       login_backoff_(&kDefaultBackoffPolicy),
       authenticating_client_(false),
       reject_authenticating_client_(false),
       weak_factory_(this) {
-  DCHECK(signal_strategy);
   DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(signal_strategy);
 
   if (!desktop_environment_factory_->SupportsAudioCapture()) {
     protocol::CandidateSessionConfig::DisableAudioChannel(
@@ -92,74 +92,41 @@ ChromotingHost::ChromotingHost(
 }
 
 ChromotingHost::~ChromotingHost() {
-  DCHECK(clients_.empty());
+  DCHECK(CalledOnValidThread());
+
+  // Disconnect all of the clients.
+  while (!clients_.empty()) {
+    clients_.front()->DisconnectSession();
+  }
+
+  // Destroy the session manager to make sure that |signal_strategy_| does not
+  // have any listeners registered.
+  session_manager_.reset();
+
+  // Notify observers.
+  if (started_)
+    FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnShutdown());
 }
 
 void ChromotingHost::Start(const std::string& xmpp_login) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
+  DCHECK(!started_);
 
   LOG(INFO) << "Starting host";
-
-  // Make sure this object is not started.
-  if (state_ != kInitial)
-    return;
-  state_ = kStarted;
-
-  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
-                    OnStart(xmpp_login));
+  started_ = true;
+  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnStart(xmpp_login));
 
   // Start the SessionManager, supplying this ChromotingHost as the listener.
   session_manager_->Init(signal_strategy_, this);
 }
 
-// This method is called when we need to destroy the host process.
-void ChromotingHost::Shutdown(const base::Closure& shutdown_task) {
-  if (!network_task_runner_->BelongsToCurrentThread()) {
-    network_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::Shutdown, this, shutdown_task));
-    return;
-  }
-
-  switch (state_) {
-    case kInitial:
-    case kStopped:
-      // Nothing to do if we are not started.
-      state_ = kStopped;
-      if (!shutdown_task.is_null())
-        network_task_runner_->PostTask(FROM_HERE, shutdown_task);
-      break;
-
-    case kStopping:
-      // We are already stopping. Just save the task.
-      if (!shutdown_task.is_null())
-        shutdown_tasks_.push_back(shutdown_task);
-      break;
-
-    case kStarted:
-      if (!shutdown_task.is_null())
-        shutdown_tasks_.push_back(shutdown_task);
-      state_ = kStopping;
-
-      // Disconnect all of the clients.
-      while (!clients_.empty()) {
-        clients_.front()->DisconnectSession();
-      }
-
-      // Run the remaining shutdown tasks.
-      if (state_ == kStopping)
-        ShutdownFinish();
-
-      break;
-  }
-}
-
 void ChromotingHost::AddStatusObserver(HostStatusObserver* observer) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   status_observers_.AddObserver(observer);
 }
 
 void ChromotingHost::RemoveStatusObserver(HostStatusObserver* observer) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   status_observers_.RemoveObserver(observer);
 }
 
@@ -170,7 +137,7 @@ void ChromotingHost::RejectAuthenticatingClient() {
 
 void ChromotingHost::SetAuthenticatorFactory(
     scoped_ptr<protocol::AuthenticatorFactory> authenticator_factory) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   session_manager_->set_authenticator_factory(authenticator_factory.Pass());
 }
 
@@ -182,7 +149,7 @@ void ChromotingHost::SetMaximumSessionDuration(
 ////////////////////////////////////////////////////////////////////////////
 // protocol::ClientSession::EventHandler implementation.
 void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   login_backoff_.Reset();
 
@@ -215,7 +182,7 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   // Notify observers.
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
@@ -223,7 +190,7 @@ void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   // Notify observers.
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
@@ -231,7 +198,7 @@ void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
 }
 
 void ChromotingHost::OnSessionClosed(ClientSession* client) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client);
   CHECK(it != clients_.end());
@@ -243,28 +210,25 @@ void ChromotingHost::OnSessionClosed(ClientSession* client) {
 
   clients_.erase(it);
   delete client;
-
-  if (state_ == kStopping && clients_.empty())
-    ShutdownFinish();
 }
 
 void ChromotingHost::OnSessionSequenceNumber(ClientSession* session,
                                              int64 sequence_number) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 }
 
 void ChromotingHost::OnSessionRouteChange(
     ClientSession* session,
     const std::string& channel_name,
     const protocol::TransportRoute& route) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
                     OnClientRouteChange(session->client_jid(), channel_name,
                                         route));
 }
 
 void ChromotingHost::OnSessionManagerReady() {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   // Don't need to do anything here, just wait for incoming
   // connections.
 }
@@ -272,9 +236,9 @@ void ChromotingHost::OnSessionManagerReady() {
 void ChromotingHost::OnIncomingSession(
       protocol::Session* session,
       protocol::SessionManager::IncomingSessionResponse* response) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
-  if (state_ != kStarted) {
+  if (!started_) {
     *response = protocol::SessionManager::DECLINE;
     return;
   }
@@ -323,18 +287,14 @@ void ChromotingHost::OnIncomingSession(
 
 void ChromotingHost::set_protocol_config(
     scoped_ptr<protocol::CandidateSessionConfig> config) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   DCHECK(config.get());
-  DCHECK_EQ(state_, kInitial);
+  DCHECK(!started_);
   protocol_config_ = config.Pass();
 }
 
 void ChromotingHost::DisconnectAllClients() {
-  if (!network_task_runner_->BelongsToCurrentThread()) {
-    network_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::DisconnectAllClients, this));
-    return;
-  }
+  DCHECK(CalledOnValidThread());
 
   while (!clients_.empty()) {
     size_t size = clients_.size();
@@ -343,35 +303,4 @@ void ChromotingHost::DisconnectAllClients() {
   }
 }
 
-void ChromotingHost::ShutdownFinish() {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, kStopping);
-
-  state_ = kStopped;
-
-  // Destroy session manager.
-  session_manager_.reset();
-
-  // Clear |desktop_environment_factory_| and |signal_strategy_| to
-  // ensure we don't try to touch them after running shutdown tasks
-  desktop_environment_factory_ = NULL;
-  signal_strategy_ = NULL;
-
-  // Keep reference to |this|, so that we don't get destroyed while
-  // sending notifications.
-  scoped_refptr<ChromotingHost> self(this);
-
-  // Notify observers.
-  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
-                    OnShutdown());
-
-  for (std::vector<base::Closure>::iterator it = shutdown_tasks_.begin();
-       it != shutdown_tasks_.end(); ++it) {
-    it->Run();
-  }
-  shutdown_tasks_.clear();
-
-  weak_factory_.InvalidateWeakPtrs();
-}
-
 }  // namespace remoting