Made the ChromotingHost class not ref-counted.

remoting/host/chromoting_host.cc

Index: remoting/host/chromoting_host.cc
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 9f7de5613804b7f97250d119805a05965f50761a..da4a256d1a82fb3c453af7fe9d428477f8a7fa60 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -82,8 +82,8 @@ ChromotingHost::ChromotingHost(
       authenticating_client_(false),
       reject_authenticating_client_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
-  DCHECK(signal_strategy);
   DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(signal_strategy);
 
   if (!desktop_environment_factory_->SupportsAudioCapture()) {
     protocol::CandidateSessionConfig::DisableAudioChannel(
@@ -96,7 +96,7 @@ ChromotingHost::~ChromotingHost() {
 }
 
 void ChromotingHost::Start(const std::string& xmpp_login) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());

[Wez, 2013/04/02 19:58:33]

Can we also DCHECK that Start() is only called from the Initial state?  Do we
still need the separate Stopped state?

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

 
   LOG(INFO) << "Starting host";
 
@@ -113,31 +113,21 @@ void ChromotingHost::Start(const std::string& xmpp_login) {
 }
 
 // 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;
-  }
+void ChromotingHost::Shutdown() {
+  DCHECK(CalledOnValidThread());

[Wez, 2013/04/02 19:58:33]

nit: Can we also DCHECK that Shutdown is never called from the Initial or
Stopped states?

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

 
   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);
+      // We are already stopping.
       break;
 
     case kStarted:
-      if (!shutdown_task.is_null())
-        shutdown_tasks_.push_back(shutdown_task);
       state_ = kStopping;
 
       // Disconnect all of the clients.
@@ -146,20 +136,22 @@ void ChromotingHost::Shutdown(const base::Closure& shutdown_task) {
       }
 
       // Run the remaining shutdown tasks.
-      if (state_ == kStopping)
+      if (state_ == kStopping) {

[Wez, 2013/04/02 19:58:33]

How can we ever end up not in the kStopping state at this point?  Even if
DisconnectSession() triggers another Shutdown() we'll ignore that via the
kStopping case.

[alexeypa (please no reviews), 2013/04/08 23:28:17]

Done.

+        state_ = kStopped;
         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 +162,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 +174,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 +207,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 +215,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 +223,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 +235,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,7 +261,7 @@ void ChromotingHost::OnSessionManagerReady() {
 void ChromotingHost::OnIncomingSession(
       protocol::Session* session,
       protocol::SessionManager::IncomingSessionResponse* response) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
 
   if (state_ != kStarted) {
     *response = protocol::SessionManager::DECLINE;
@@ -323,18 +312,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);
   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();
@@ -344,10 +329,7 @@ void ChromotingHost::DisconnectAllClients() {
 }
 
 void ChromotingHost::ShutdownFinish() {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, kStopping);
-
-  state_ = kStopped;
+  DCHECK(CalledOnValidThread());
 
   // Destroy session manager.
   session_manager_.reset();
@@ -357,19 +339,8 @@ void ChromotingHost::ShutdownFinish() {
   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();
+  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnShutdown());
 
   weak_factory_.InvalidateWeakPtrs();
 }